Merge r236142 - "DidFirstVisuallyNonEmptyLayout" callback does not ge…

…t 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. Source/WebCore: 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): Tools: Add API test coverage. * TestWebKitAPI/Tests/WebKit/LayoutMilestonesWithAllContentInFrame.cpp: (TestWebKitAPI::didFinishNavigation): (TestWebKitAPI::TEST):
WebKit · Sep 19, 2018 · 67023d6 · 67023d6
1 parent 5be08a8
commit 67023d6
Show file tree

Hide file tree

Showing 6 changed files with 104 additions and 12 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,20 @@
+2018-09-18  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-15  Rob Buis  <rbuis@igalia.com>
 
         XMLHttpRequest::createResponseBlob() should create a Blob with type for empty response

diff --git 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
@@ -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
@@ -180,6 +180,7 @@ class FrameView final : public ScrollView {
     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
@@ -1,3 +1,17 @@
+2018-09-18  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-18  Claudio Saavedra  <csaavedra@igalia.com>
 
         [WPE] Implement mouse event modifiers

diff --git 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