Changeset 206922 in webkit


Ignore:
Timestamp:
Oct 7, 2016 11:55:09 AM (8 years ago)
Author:
akling@apple.com
Message:

[WK2] didRemoveFrameFromHierarchy callback doesn't fire for subframes when evicting from PageCache.
<​https://webkit.org/b/163098>
<rdar://problem/28663488>

Reviewed by Antti Koivisto.

Source/WebCore:

Fix a bug where WK2 didRemoveFrameFromHierarchy callbacks wouldn't fire for subframes that were getting
kicked out of PageCache. The problem was happening because CachedFrame would disconnect the Frame from
its Page just before calling FrameLoader::detachViewsAndDocumentLoader() where the callbacks are fired.
Without a Page, the WebFrame on WK2 side can't find its WebPage, and so it can't fire its callbacks.

The fix is just to switch the order of those two lines.

This bug was causing frequent DOM and window object leaks in some clients *cough* Safari *cough* that
were relying on didRemoveFrameFromHierarchy to release their isolated worlds.

Test: WebKit2.DidRemoveFrameFromHiearchyInPageCache

  • history/CachedFrame.cpp:

(WebCore::CachedFrame::destroy):

Tools:

Add an API test that puts a 10-subframe page into the page cache, then loads other pages
until the first page gets kicked out. The test succeeds if we receive didRemoveFrameFromHierarchy
callbacks for all the subframes.

  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/WebKit2/DidRemoveFrameFromHiearchyInPageCache.cpp: Added.

(TestWebKitAPI::didFinishLoadForFrame):
(TestWebKitAPI::setPageLoaderClient):
(TestWebKitAPI::didReceivePageMessageFromInjectedBundle):
(TestWebKitAPI::setInjectedBundleClient):
(TestWebKitAPI::TEST):

  • TestWebKitAPI/Tests/WebKit2/DidRemoveFrameFromHiearchyInPageCache_Bundle.cpp: Added.

(TestWebKitAPI::didRemoveFrameFromHierarchyCallback):
(TestWebKitAPI::DidRemoveFrameFromHiearchyInPageCacheTest::DidRemoveFrameFromHiearchyInPageCacheTest):
(TestWebKitAPI::DidRemoveFrameFromHiearchyInPageCacheTest::didCreatePage):

  • TestWebKitAPI/Tests/WebKit2/many-iframes.html: Added.
Location:
trunk
Files:
3 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r206921 r206922  
     12016-10-07  Andreas Kling  <akling@apple.com>
     2
     3        [WK2] didRemoveFrameFromHierarchy callback doesn't fire for subframes when evicting from PageCache.
     4        <https://webkit.org/b/163098>
     5        <rdar://problem/28663488>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        Fix a bug where WK2 didRemoveFrameFromHierarchy callbacks wouldn't fire for subframes that were getting
     10        kicked out of PageCache. The problem was happening because CachedFrame would disconnect the Frame from
     11        its Page just before calling FrameLoader::detachViewsAndDocumentLoader() where the callbacks are fired.
     12        Without a Page, the WebFrame on WK2 side can't find its WebPage, and so it can't fire its callbacks.
     13
     14        The fix is just to switch the order of those two lines.
     15
     16        This bug was causing frequent DOM and window object leaks in some clients *cough* Safari *cough* that
     17        were relying on didRemoveFrameFromHierarchy to release their isolated worlds.
     18
     19        Test: WebKit2.DidRemoveFrameFromHiearchyInPageCache
     20
     21        * history/CachedFrame.cpp:
     22        (WebCore::CachedFrame::destroy):
     23
    1242016-10-07  Nan Wang  <n_wang@apple.com>
    225

  • trunk/Source/WebCore/history/CachedFrame.cpp

    r206006 r206922  
    249249
    250250    if (!m_isMainFrame) {
     251        m_view->frame().loader().detachViewsAndDocumentLoader();
    251252        m_view->frame().detachFromPage();
    252         m_view->frame().loader().detachViewsAndDocumentLoader();
    253253    }
    254254   

  • trunk/Tools/ChangeLog

    r206918 r206922  
     12016-10-07  Andreas Kling  <akling@apple.com>
     2
     3        [WK2] didRemoveFrameFromHierarchy callback doesn't fire for subframes when evicting from PageCache.
     4        <https://webkit.org/b/163098>
     5        <rdar://problem/28663488>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        Add an API test that puts a 10-subframe page into the page cache, then loads other pages
     10        until the first page gets kicked out. The test succeeds if we receive didRemoveFrameFromHierarchy
     11        callbacks for all the subframes.
     12
     13        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     14        * TestWebKitAPI/Tests/WebKit2/DidRemoveFrameFromHiearchyInPageCache.cpp: Added.
     15        (TestWebKitAPI::didFinishLoadForFrame):
     16        (TestWebKitAPI::setPageLoaderClient):
     17        (TestWebKitAPI::didReceivePageMessageFromInjectedBundle):
     18        (TestWebKitAPI::setInjectedBundleClient):
     19        (TestWebKitAPI::TEST):
     20        * TestWebKitAPI/Tests/WebKit2/DidRemoveFrameFromHiearchyInPageCache_Bundle.cpp: Added.
     21        (TestWebKitAPI::didRemoveFrameFromHierarchyCallback):
     22        (TestWebKitAPI::DidRemoveFrameFromHiearchyInPageCacheTest::DidRemoveFrameFromHiearchyInPageCacheTest):
     23        (TestWebKitAPI::DidRemoveFrameFromHiearchyInPageCacheTest::didCreatePage):
     24        * TestWebKitAPI/Tests/WebKit2/many-iframes.html: Added.
     25
    1262016-10-07  Emanuele Aina  <emanuele.aina@collabora.com>
    227

  • trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r206829 r206922  
    435435                A57A34F216AF6B2B00C2501F /* PageVisibilityStateWithWindowChanges.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = A57A34F116AF69E200C2501F /* PageVisibilityStateWithWindowChanges.html */; };
    436436                A5E2027515B21F6E00C13E14 /* WindowlessWebViewWithMedia.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = A5E2027015B2180600C13E14 /* WindowlessWebViewWithMedia.html */; };
     437                AD57AC201DA7465000FF1BDE /* DidRemoveFrameFromHiearchyInPageCache_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = AD57AC1E1DA7464D00FF1BDE /* DidRemoveFrameFromHiearchyInPageCache_Bundle.cpp */; };
     438                AD57AC211DA7465B00FF1BDE /* DidRemoveFrameFromHiearchyInPageCache.cpp in Sources */ = {isa = PBXBuildFile; fileRef = AD57AC1F1DA7464D00FF1BDE /* DidRemoveFrameFromHiearchyInPageCache.cpp */; };
     439                AD57AC221DA7466E00FF1BDE /* many-iframes.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = AD57AC1D1DA7463800FF1BDE /* many-iframes.html */; };
    437440                ADCEBBA61D9AE229002E283A /* Consume.cpp in Sources */ = {isa = PBXBuildFile; fileRef = ADCEBBA51D99D4CF002E283A /* Consume.cpp */; };
    438441                B55AD1D5179F3B3000AC1494 /* PreventImageLoadWithAutoResizing_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = B55AD1D3179F3ABF00AC1494 /* PreventImageLoadWithAutoResizing_Bundle.cpp */; };
    … …  
    555558                        dstSubfolderSpec = 7;
    556559                        files = (
     560                                AD57AC221DA7466E00FF1BDE /* many-iframes.html in Copy Resources */,
    557561                                F415086D1DA040C50044BE9B /* play-audio-on-click.html in Copy Resources */,
    558562                                F4F137921D9B683E002BEC57 /* large-video-test-now-playing.html in Copy Resources */,
    … …  
    10681072                A5E2027215B2181900C13E14 /* WindowlessWebViewWithMedia.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WindowlessWebViewWithMedia.mm; sourceTree = "<group>"; };
    10691073                A7A966DA140ECCC8005EF9B4 /* CheckedArithmeticOperations.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = CheckedArithmeticOperations.cpp; sourceTree = "<group>"; };
     1074                AD57AC1D1DA7463800FF1BDE /* many-iframes.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "many-iframes.html"; sourceTree = "<group>"; };
     1075                AD57AC1E1DA7464D00FF1BDE /* DidRemoveFrameFromHiearchyInPageCache_Bundle.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DidRemoveFrameFromHiearchyInPageCache_Bundle.cpp; sourceTree = "<group>"; };
     1076                AD57AC1F1DA7464D00FF1BDE /* DidRemoveFrameFromHiearchyInPageCache.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DidRemoveFrameFromHiearchyInPageCache.cpp; sourceTree = "<group>"; };
    10701077                ADCEBBA51D99D4CF002E283A /* Consume.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Consume.cpp; sourceTree = "<group>"; };
    10711078                B4039F9C15E6D8B3007255D6 /* MathExtras.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MathExtras.cpp; sourceTree = "<group>"; };
    … …  
    16291636                        isa = PBXGroup;
    16301637                        children = (
     1638                                AD57AC1E1DA7464D00FF1BDE /* DidRemoveFrameFromHiearchyInPageCache_Bundle.cpp */,
     1639                                AD57AC1F1DA7464D00FF1BDE /* DidRemoveFrameFromHiearchyInPageCache.cpp */,
    16311640                                0F139E741A423A4600F590F5 /* cocoa */,
    16321641                                C0C5D3BB14598B6F00A802A6 /* mac */,
    … …  
    18271836                                2DD7D3AE178227AC0026E1E3 /* lots-of-text-vertical-lr.html */,
    18281837                                930AD401150698B30067970F /* lots-of-text.html */,
     1838                                AD57AC1D1DA7463800FF1BDE /* many-iframes.html */,
    18291839                                51CD1C711B38D48400142CA5 /* modal-alerts-in-new-about-blank-window.html */,
    18301840                                7A1458FB1AD5C03500E06772 /* mouse-button-listener.html */,
    … …  
    24282438                                7CCE7EC71A411A7E00447C4C /* PageVisibilityStateWithWindowChanges.mm in Sources */,
    24292439                                7CCE7F091A411AE600447C4C /* ParentFrame.cpp in Sources */,
     2440                                AD57AC211DA7465B00FF1BDE /* DidRemoveFrameFromHiearchyInPageCache.cpp in Sources */,
    24302441                                7C83E0511D0A641800FEBCF3 /* ParsedContentRange.cpp in Sources */,
    24312442                                7CCE7F0A1A411AE600447C4C /* PasteboardNotifications.mm in Sources */,
    … …  
    25932604                                0F139E791A42457000F590F5 /* PlatformUtilitiesCocoa.mm in Sources */,
    25942605                                BC575BE0126F590D006F0F12 /* PlatformUtilitiesMac.mm in Sources */,
     2606                                AD57AC201DA7465000FF1BDE /* DidRemoveFrameFromHiearchyInPageCache_Bundle.cpp in Sources */,
    25952607                                B55AD1D5179F3B3000AC1494 /* PreventImageLoadWithAutoResizing_Bundle.cpp in Sources */,
    25962608                                C0BD669F131D3CFF00E18F2A /* ResponsivenessTimerDoesntFireEarly_Bundle.cpp in Sources */,
Note: See TracChangeset for help on using the changeset viewer.