Changeset 246057 in webkit


Ignore:
Timestamp:
Jun 4, 2019 12:38:17 AM (5 years ago)
Author:
commit-queue@webkit.org
Message:

JS wrapper of target in ResizeObserverEntry/ResizeObserver shouldn't get collected ahead
​https://bugs.webkit.org/show_bug.cgi?id=197457

Patch by Cathie Chen <cathiechen> on 2019-06-04
Reviewed by Ryosuke Niwa.

Source/WebCore:

Add JSCustomMarkFunction to make sure JS wrappers wouldn't be collected when JSResizeObserverEntry live.

For ResizeObserver, if targets are removed, it will get fired for the last time. We also need to keep these JS
wrappers live. So add these targets to a GCReachableRef list once they're observed.

Add element-leak.html to test the targets with entry.target.myEntry = entry could be released properly.

Tests: resize-observer/element-leak.html

resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive.html
resize-observer/resize-observer-keeps-js-wrapper-of-target-alive.html

  • Sources.txt:
  • WebCore.xcodeproj/project.pbxproj:
  • bindings/js/JSResizeObserverEntryCustom.cpp: Added.

(WebCore::JSResizeObserverEntry::visitAdditionalChildren):

  • page/ResizeObserver.cpp:

(WebCore::ResizeObserver::observe):
(WebCore::ResizeObserver::removeAllTargets):
(WebCore::ResizeObserver::removeObservation):
(WebCore::ResizeObserver::stop):

  • page/ResizeObserver.h:
  • page/ResizeObserverEntry.idl:

LayoutTests:

  • platform/win/TestExpectations:
  • resize-observer/element-leak-expected.txt: Added.
  • resize-observer/element-leak.html: Added.
  • resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive-expected.txt: Added.
  • resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive.html: Added.
  • resize-observer/resize-observer-keeps-js-wrapper-of-target-alive-expected.txt: Added.
  • resize-observer/resize-observer-keeps-js-wrapper-of-target-alive.html: Added.
  • resize-observer/resources/element-leak-frame.html: Added.
Location:
trunk
Files:
7 added
8 edited
1 copied

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r246049 r246057  
     12019-06-04  Cathie Chen  <cathiechen@igalia.com>
     2
     3        JS wrapper of target in ResizeObserverEntry/ResizeObserver shouldn't get collected ahead
     4        https://bugs.webkit.org/show_bug.cgi?id=197457
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        * platform/win/TestExpectations:
     9        * resize-observer/element-leak-expected.txt: Added.
     10        * resize-observer/element-leak.html: Added.
     11        * resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive-expected.txt: Added.
     12        * resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive.html: Added.
     13        * resize-observer/resize-observer-keeps-js-wrapper-of-target-alive-expected.txt: Added.
     14        * resize-observer/resize-observer-keeps-js-wrapper-of-target-alive.html: Added.
     15        * resize-observer/resources/element-leak-frame.html: Added.
     16
    1172019-06-03  Youenn Fablet  <youenn@apple.com>
    218

  • trunk/LayoutTests/platform/win/TestExpectations

    r246016 r246057  
    44024402
    44034403webkit.org/b/198112 http/tests/security/showModalDialog-sync-cross-origin-page-load2.html [ Skip ]
     4404
     4405# The removed elements couldn't be released properly in Win.
     4406# The relevant bug is https://bugs.webkit.org/show_bug.cgi?id=197908
     4407resize-observer/element-leak.html [ Skip ]

  • trunk/Source/WebCore/ChangeLog

    r246056 r246057  
     12019-06-04  Cathie Chen  <cathiechen@igalia.com>
     2
     3        JS wrapper of target in ResizeObserverEntry/ResizeObserver shouldn't get collected ahead
     4        https://bugs.webkit.org/show_bug.cgi?id=197457
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Add JSCustomMarkFunction to make sure JS wrappers wouldn't be collected when JSResizeObserverEntry live.
     9
     10        For ResizeObserver, if targets are removed, it will get fired for the last time. We also need to keep these JS
     11        wrappers live. So add these targets to a GCReachableRef list once they're observed.
     12
     13        Add element-leak.html to test the targets with `entry.target.myEntry = entry` could be released properly.
     14
     15        Tests: resize-observer/element-leak.html
     16               resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive.html
     17               resize-observer/resize-observer-keeps-js-wrapper-of-target-alive.html
     18
     19        * Sources.txt:
     20        * WebCore.xcodeproj/project.pbxproj:
     21        * bindings/js/JSResizeObserverEntryCustom.cpp: Added.
     22        (WebCore::JSResizeObserverEntry::visitAdditionalChildren):
     23        * page/ResizeObserver.cpp:
     24        (WebCore::ResizeObserver::observe):
     25        (WebCore::ResizeObserver::removeAllTargets):
     26        (WebCore::ResizeObserver::removeObservation):
     27        (WebCore::ResizeObserver::stop):
     28        * page/ResizeObserver.h:
     29        * page/ResizeObserverEntry.idl:
     30
    1312019-06-03  Andy Estes  <aestes@apple.com>
    232

  • trunk/Source/WebCore/Sources.txt

    r245945 r246057  
    534534bindings/js/JSRemoteDOMWindowBase.cpp
    535535bindings/js/JSRemoteDOMWindowCustom.cpp
     536bindings/js/JSResizeObserverEntryCustom.cpp
    536537bindings/js/JSSVGPathSegCustom.cpp
    537538bindings/js/JSSVGViewSpecCustom.cpp

  • trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

    r246002 r246057  
    87948794                585D6E011A1A792E00FA4F12 /* SimpleLineLayoutFlowContents.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SimpleLineLayoutFlowContents.cpp; sourceTree = "<group>"; };
    87958795                585D6E021A1A792E00FA4F12 /* SimpleLineLayoutFlowContents.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SimpleLineLayoutFlowContents.h; sourceTree = "<group>"; };
     8796                5884FE5622813E2D0040AFF6 /* JSResizeObserverEntryCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSResizeObserverEntryCustom.cpp; sourceTree = "<group>"; };
    87968797                589556EC18D4A44000764B03 /* BorderEdge.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = BorderEdge.h; sourceTree = "<group>"; };
    87978798                58AEE2F318D4BCCF0022E7FE /* BorderEdge.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = BorderEdge.cpp; sourceTree = "<group>"; };
    … …  
    2069120692                                833CF70F20DB3F5F00141BCC /* JSPerformanceObserverCustom.cpp */,
    2069220693                                A4A69B8BB91B49D0A804C31D /* JSPromiseRejectionEventCustom.cpp */,
     20694                                5884FE5622813E2D0040AFF6 /* JSResizeObserverEntryCustom.cpp */,
    2069320695                                83F572941FA1066F003837BE /* JSServiceWorkerClientCustom.cpp */,
    2069420696                                460D19441FCE21DD00C3DB85 /* JSServiceWorkerGlobalScopeCustom.cpp */,

  • trunk/Source/WebCore/bindings/js/JSResizeObserverEntryCustom.cpp

    r246056 r246057  
    2424 */
    2525
    26 // https://wicg.github.io/ResizeObserver/
     26#include "config.h"
     27#include "JSResizeObserverEntry.h"
    2728
    28 [
    29     Conditional=RESIZE_OBSERVER,
    30     ImplementationLacksVTable,
    31     EnabledBySetting=ResizeObserver
    32 ] interface ResizeObserverEntry {
    33     readonly attribute Element target;
    34     readonly attribute DOMRectReadOnly contentRect;
    35 };
     29#include "JSNodeCustom.h"
     30
     31namespace WebCore {
     32
     33void JSResizeObserverEntry::visitAdditionalChildren(JSC::SlotVisitor& visitor)
     34{
     35    visitor.addOpaqueRoot(root(wrapped().target()));
     36    visitor.addOpaqueRoot(wrapped().contentRect());
     37}
     38
     39}

  • trunk/Source/WebCore/page/ResizeObserver.cpp

    r244182 r246057  
    7070
    7171    m_observations.append(ResizeObservation::create(&target));
     72    m_pendingTargets.append(target);
    7273
    7374    if (m_document) {
    … …  
    141142        ASSERT_UNUSED(removed, removed);
    142143    }
     144    m_pendingTargets.clear();
     145    m_activeObservations.clear();
    143146    m_observations.clear();
    144147}
    … …  
    146149bool ResizeObserver::removeObservation(const Element& target)
    147150{
     151    m_pendingTargets.removeFirstMatching([&target](auto& pendingTarget) {
     152        return pendingTarget.ptr() == &target;
     153    });
     154
    148155    m_activeObservations.removeFirstMatching([&target](auto& observation) {
    149156        return observation->target() == &target;
    … …  
    174181    disconnect();
    175182    m_callback = nullptr;
    176     m_observations.clear();
    177     m_activeObservations.clear();
    178183}
    179184

  • trunk/Source/WebCore/page/ResizeObserver.h

    r244182 r246057  
    2929
    3030#include "ActiveDOMObject.h"
     31#include "GCReachableRef.h"
    3132#include "ResizeObservation.h"
    3233#include "ResizeObserverCallback.h"
    … …  
    8182
    8283    Vector<Ref<ResizeObservation>> m_activeObservations;
     84    Vector<GCReachableRef<Element>> m_pendingTargets;
    8385    bool m_hasSkippedObservations { false };
    8486};

  • trunk/Source/WebCore/page/ResizeObserverEntry.idl

    r243643 r246057  
    2929    Conditional=RESIZE_OBSERVER,
    3030    ImplementationLacksVTable,
    31     EnabledBySetting=ResizeObserver
     31    EnabledBySetting=ResizeObserver,
     32    JSCustomMarkFunction
    3233] interface ResizeObserverEntry {
    3334    readonly attribute Element target;
Note: See TracChangeset for help on using the changeset viewer.