Attachment 349943 Details for Bug 189681 – Patch

[patch] Patch

bug-189681-20180917145304.patch (text/plain), 6.22 KB, created by Chris Dumez on 2018-09-17 14:53:05 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-09-17 14:53:05 PDT

Size: 6.22 KB

patch

obsolete

>Subversion Revision: 236078
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c19c24da1025bf6e498de9b361de8d4370f988db..16cfda2fcc2e10d5f42bc5845d0190865c51999c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,20 @@
>+2018-09-17  Chris Dumez  <cdumez@apple.com>
>+
>+        "DidFirstNonVisuallyEmptyLayout" callback does not get called when restoring a page from PageCache
>+        https://bugs.webkit.org/show_bug.cgi?id=189681
>+        <rdar://problem/44526171>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The "DidFirstNonVisuallyEmptyLayout" callback was not getting called when restoring a page from PageCache
>+        because the FrameView is restored from PageCache and we would fail to restore its flags (such as
>+        m_firstVisuallyNonEmptyLayoutCallbackPending) when entering Page Cache. We now call FrameView::clear()
>+        to reset those flags when entering PageCache so that layout milestone events properly get sent again
>+        when restoring from Page Cache.
>+
>+        * history/CachedFrame.cpp:
>+        (WebCore::CachedFrame::CachedFrame):
>+
> 2018-09-17  Simon Fraser  <simon.fraser@apple.com>
> 
>         Add more ResourceLoading logging, particularly in MemoryCache code
>diff --git a/Source/WebCore/history/CachedFrame.cpp b/Source/WebCore/history/CachedFrame.cpp
>index 3b34388eb1d62783d24b91c56c2673f196d567c1..49b9f2d832e616c0aedc4e1f38f6b3246f0d5bc0 100644
>--- a/Source/WebCore/history/CachedFrame.cpp
>+++ b/Source/WebCore/history/CachedFrame.cpp
>@@ -154,6 +154,10 @@ CachedFrame::CachedFrame(Frame& frame)
> 
>     m_document->domWindow()->suspendForDocumentSuspension();
> 
>+    // Clear FrameView to reset flags such as 'firstVisuallyNonEmptyLayoutCallbackPending' so that the
>+    // 'DidFirstVisuallyNonEmptyLayout' callback gets called against when restoring from PageCache.
>+    m_view->clear();
>+
>     frame.loader().client().savePlatformDataToCachedFrame(this);
> 
>     // documentWillSuspendForPageCache() can set up a layout timer on the FrameView, so clear timers after that.
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 30321e7dd69ff333542c1f42ae3d32b8103624d9..21bc28ed25148a0af7b693366450aae7174f5542 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-09-17  Chris Dumez  <cdumez@apple.com>
>+
>+        "DidFirstNonVisuallyEmptyLayout" callback does not get called when restoring a page from PageCache
>+        https://bugs.webkit.org/show_bug.cgi?id=189681
>+        <rdar://problem/44526171>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKit/LayoutMilestonesWithAllContentInFrame.cpp:
>+        (TestWebKitAPI::didFinishNavigation):
>+        (TestWebKitAPI::TEST):
>+
> 2018-09-17  Chris Dumez  <cdumez@apple.com>
> 
>         PSON: window.open() with 'noopener' should only process-swap cross-site, not cross-origin
>diff --git a/Tools/TestWebKitAPI/Tests/WebKit/LayoutMilestonesWithAllContentInFrame.cpp b/Tools/TestWebKitAPI/Tests/WebKit/LayoutMilestonesWithAllContentInFrame.cpp
>index c39e4b383b823958ccd9a2d6016c8cc76a3f405f..7c5bb710bef3be5d02886654c6846a6e3235b6e7 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKit/LayoutMilestonesWithAllContentInFrame.cpp
>+++ b/Tools/TestWebKitAPI/Tests/WebKit/LayoutMilestonesWithAllContentInFrame.cpp
>@@ -35,14 +35,20 @@
> 
> namespace TestWebKitAPI {
> 
>-static bool testDone;
>+static bool didFirstVisuallyNonEmptyLayout;
>+static bool didNavigate;
> 
> static void renderingProgressDidChange(WKPageRef page, WKPageRenderingProgressEvents milestones, WKTypeRef, const void* clientInfo)
> {
>     // This test ensures that the DidFirstVisuallyNonEmptyLayout will be reached for the main frame
>     // even when all of the content is in a subframe.
>     if (milestones & WKPageRenderingProgressEventFirstVisuallyNonEmptyLayout)
>-        testDone = true;
>+        didFirstVisuallyNonEmptyLayout = true;
>+}
>+
>+static void didFinishNavigation(WKPageRef page, WKNavigationRef navigation, WKTypeRef userData, const void* clientInfo)
>+{
>+    didNavigate = true;
> }
> 
> TEST(WebKit, LayoutMilestonesWithAllContentInFrame)
>@@ -62,8 +68,53 @@ TEST(WebKit, LayoutMilestonesWithAllContentInFrame)
>     WKPageListenForLayoutMilestones(webView.page(), WKPageRenderingProgressEventFirstVisuallyNonEmptyLayout);
>     WKPageLoadURL(webView.page(), adoptWK(Util::createURLForResource("all-content-in-one-iframe", "html")).get());
> 
>-    Util::run(&testDone);
>-    EXPECT_TRUE(testDone);
>+    Util::run(&didFirstVisuallyNonEmptyLayout);
>+    EXPECT_TRUE(didFirstVisuallyNonEmptyLayout);
>+}
>+
>+TEST(WebKit, FirstVisuallyNonEmptyLayoutAfterPageCacheRestore)
>+{
>+    WKRetainPtr<WKContextRef> context(AdoptWK, WKContextCreate());
>+
>+    WKContextSetCacheModel(context.get(), kWKCacheModelPrimaryWebBrowser); // Enables the Page Cache.
>+
>+    PlatformWebView webView(context.get());
>+
>+    WKPageNavigationClientV3 loaderClient;
>+    memset(&loaderClient, 0, sizeof(loaderClient));
>+
>+    loaderClient.base.version = 3;
>+    loaderClient.base.clientInfo = &webView;
>+    loaderClient.renderingProgressDidChange = renderingProgressDidChange;
>+    loaderClient.didFinishNavigation = didFinishNavigation;
>+
>+    WKPageSetPageNavigationClient(webView.page(), &loaderClient.base);
>+
>+    WKPageListenForLayoutMilestones(webView.page(), WKPageRenderingProgressEventFirstVisuallyNonEmptyLayout);
>+    WKPageLoadURL(webView.page(), adoptWK(Util::createURLForResource("simple-tall", "html")).get());
>+
>+    Util::run(&didFirstVisuallyNonEmptyLayout);
>+    EXPECT_TRUE(didFirstVisuallyNonEmptyLayout);
>+    didFirstVisuallyNonEmptyLayout = false;
>+    Util::run(&didNavigate);
>+    EXPECT_TRUE(didNavigate);
>+    didNavigate = false;
>+
>+    WKPageLoadURL(webView.page(), adoptWK(Util::createURLForResource("large-red-square-image", "html")).get());
>+    Util::run(&didFirstVisuallyNonEmptyLayout);
>+    EXPECT_TRUE(didFirstVisuallyNonEmptyLayout);
>+    didFirstVisuallyNonEmptyLayout = false;
>+    Util::run(&didNavigate);
>+    EXPECT_TRUE(didNavigate);
>+    didNavigate = false;
>+
>+    WKPageGoBack(webView.page());
>+    Util::run(&didFirstVisuallyNonEmptyLayout);
>+    EXPECT_TRUE(didFirstVisuallyNonEmptyLayout);
>+    didFirstVisuallyNonEmptyLayout = false;
>+    Util::run(&didNavigate);
>+    EXPECT_TRUE(didNavigate);
>+    didNavigate = false;
> }
> 
> } // namespace TestWebKitAPI

Actions: View | Formatted Diff | Diff

Attachments on bug 189681: 349943 | 349944 | 349955 | 349969 | 349973 | 349974 | 350022