Attachment 350022 Details for Bug 189681 – Patch

[patch] Patch

bug-189681-20180918084316.patch (text/plain), 8.48 KB, created by Chris Dumez on 2018-09-18 08:43:17 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-09-18 08:43:17 PDT

Size: 8.48 KB

patch

obsolete

>Subversion Revision: 236078
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c19c24da1025bf6e498de9b361de8d4370f988db..1088a8875183afb1864ebe91aba97934965f4120 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,20 @@
>+2018-09-17  Chris Dumez  <cdumez@apple.com>
>+
>+        "DidFirstVisuallyNonEmptyLayout" 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 Alex Christensen and Zalan Bujtas.
>+
>+        The "DidFirstVisuallyNonEmptyLayout" 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 reset those flags that
>+        are related to layout miletones 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..146795a948a51dbea38143a4ffbef4700c53976f 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->resetLayoutMilestones();
>+
>     frame.loader().client().savePlatformDataToCachedFrame(this);
> 
>     // documentWillSuspendForPageCache() can set up a layout timer on the FrameView, so clear timers after that.
>diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
>index 99489ef371ea06640301b1aee11b5812c84eba87..802f39186507488507f77d0b61f8eaac43685f85 100644
>--- a/Source/WebCore/page/FrameView.cpp
>+++ b/Source/WebCore/page/FrameView.cpp
>@@ -267,7 +267,6 @@ void FrameView::reset()
>     m_isOverlapped = false;
>     m_contentIsOpaque = false;
>     m_updateEmbeddedObjectsTimer.stop();
>-    m_firstLayoutCallbackPending = false;
>     m_wasScrolledByUser = false;
>     m_delayedScrollEventTimer.stop();
>     m_shouldScrollToFocusedElement = false;
>@@ -279,18 +278,24 @@ void FrameView::reset()
>     m_lastPaintTime = MonotonicTime();
>     m_paintBehavior = PaintBehavior::Normal;
>     m_isPainting = false;
>-    m_visuallyNonEmptyCharacterCount = 0;
>-    m_visuallyNonEmptyPixelCount = 0;
>-    m_isVisuallyNonEmpty = false;
>-    m_firstVisuallyNonEmptyLayoutCallbackPending = true;
>-    m_renderTextCountForVisuallyNonEmptyCharacters = 0;
>-    m_renderedSignificantAmountOfText = false;
>-    m_significantRenderedTextMilestonePending = true;
>     m_needsDeferredScrollbarsUpdate = false;
>     m_maintainScrollPositionAnchor = nullptr;
>+    resetLayoutMilestones();
>     layoutContext().reset();
> }
> 
>+void FrameView::resetLayoutMilestones()
>+{
>+    m_firstLayoutCallbackPending = false;
>+    m_isVisuallyNonEmpty = false;
>+    m_firstVisuallyNonEmptyLayoutCallbackPending = true;
>+    m_significantRenderedTextMilestonePending = true;
>+    m_renderedSignificantAmountOfText = false;
>+    m_visuallyNonEmptyCharacterCount = 0;
>+    m_visuallyNonEmptyPixelCount = 0;
>+    m_renderTextCountForVisuallyNonEmptyCharacters = 0;
>+}
>+
> void FrameView::removeFromAXObjectCache()
> {
>     if (AXObjectCache* cache = axObjectCache()) {
>diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
>index f50cde6ae90b235d8138abd08de4acf85a0d7990..240e645737a4c78cf52d71d8d28570a03b84f9af 100644
>--- a/Source/WebCore/page/FrameView.h
>+++ b/Source/WebCore/page/FrameView.h
>@@ -180,6 +180,7 @@ public:
>     WEBCORE_EXPORT void recalculateScrollbarOverlayStyle();
> 
>     void clear();
>+    void resetLayoutMilestones();
> 
>     WEBCORE_EXPORT bool isTransparent() const;
>     WEBCORE_EXPORT void setTransparent(bool isTransparent);
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 30321e7dd69ff333542c1f42ae3d32b8103624d9..1da3f50c14833cc2f2b4a6c2db8d0ac9e629bc0f 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-09-17  Chris Dumez  <cdumez@apple.com>
>+
>+        "DidFirstVisuallyNonEmptyLayout" 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 Alex Christensen and Zalan Bujtas.
>+
>+        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