Changeset 165534 in webkit

Timestamp:

Mar 13, 2014, 8:41:42 AM (11 years ago)

Author:

Antti Koivisto

Message:

Ensure that layout milestones complete in all cases
​https://bugs.webkit.org/show_bug.cgi?id=130101

Reviewed by Darin Adler.

Milestones fail to complete in some testing scenarios.

• dom/Document.cpp:

(WebCore::Document::setParsing):

Check if we need to fire layout milestones if parsing finishes without pending layout.
Parsing status affects whether the document is considered non-empty and that affects
layout milestones.

Remove explicit layout scheduling here, layout timer will be active already if there
is a layout pending

• page/FrameView.cpp:

(WebCore::FrameView::performPostLayoutTasks):
(WebCore::FrameView::firePaintRelatedMilestonesIfNeeded):

Renamed for consistency

(WebCore::FrameView::fireLayoutRelatedMilestonesIfNeeded):

Factor layout milestone firing into a function.

• page/FrameView.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Document.cpp (modified) (1 diff)
• page/FrameView.cpp (modified) (5 diffs)
• page/FrameView.h (modified) (1 diff)
• rendering/RenderLayerCompositor.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-03-13  Antti Koivisto  <antti@apple.com>
+
+        Ensure that layout milestones complete in all cases
+        https://bugs.webkit.org/show_bug.cgi?id=130101
+        
+        Reviewed by Darin Adler.
+        
+        Milestones fail to complete in some testing scenarios.
+
+        * dom/Document.cpp:
+        (WebCore::Document::setParsing):
+        
+            Check if we need to fire layout milestones if parsing finishes without pending layout.
+            Parsing status affects whether the document is considered non-empty and that affects
+            layout milestones.
+
+            Remove explicit layout scheduling here, layout timer will be active already if there
+            is a layout pending
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::performPostLayoutTasks):
+        (WebCore::FrameView::firePaintRelatedMilestonesIfNeeded):
+        
+            Renamed for consistency
+
+        (WebCore::FrameView::fireLayoutRelatedMilestonesIfNeeded):
+        
+            Factor layout milestone firing into a function.
+
+        * page/FrameView.h:
+
 2014-03-12  Brian Burg  <bburg@apple.com>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -2487 +2487 @@
         m_sharedObjectPool = std::make_unique<DocumentSharedObjectPool>();
 
-    if (!m_bParsing && view())
-        view()->scheduleRelayout();
+    if (!m_bParsing && view() && !view()->needsLayout())
+        view()->fireLayoutRelatedMilestonesIfNeeded();
 
 #ifdef INSTRUMENT_LAYOUT_SCHEDULING

trunk/Source/WebCore/page/FrameView.cpp

@@ -2636 +2636 @@
     frame().selection().updateAndRevealSelection();
 
-    LayoutMilestones requestedMilestones = 0;
-    LayoutMilestones milestonesAchieved = 0;
-    Page* page = frame().page();
-    if (page)
-        requestedMilestones = page->requestedLayoutMilestones();
-
-    if (m_nestedLayoutCount <= 1 && frame().document()->documentElement()) {
-        if (m_firstLayoutCallbackPending) {
-            m_firstLayoutCallbackPending = false;
-            frame().loader().didFirstLayout();
-            if (requestedMilestones & DidFirstLayout)
-                milestonesAchieved |= DidFirstLayout;
-            if (frame().isMainFrame())
-                page->startCountingRelevantRepaintedObjects();
-        }
-        updateIsVisuallyNonEmpty();
-
-        // If the layout was done with pending sheets, we are not in fact visually non-empty yet.
-        if (m_isVisuallyNonEmpty && !frame().document()->didLayoutWithPendingStylesheets() && m_firstVisuallyNonEmptyLayoutCallbackPending) {
-            m_firstVisuallyNonEmptyLayoutCallbackPending = false;
-            if (requestedMilestones & DidFirstVisuallyNonEmptyLayout)
-                milestonesAchieved |= DidFirstVisuallyNonEmptyLayout;
-        }
-    }
+    if (m_nestedLayoutCount <= 1 && frame().document()->documentElement())
+        fireLayoutRelatedMilestonesIfNeeded();
 
 #if PLATFORM(IOS)
@@ -2669 +2647 @@
 #endif
 
-    if (milestonesAchieved && frame().isMainFrame())
-        frame().loader().didLayout(milestonesAchieved);
-
 #if ENABLE(FONT_LOAD_EVENTS)
     if (RuntimeEnabledFeatures::sharedFeatures().fontLoadEventsEnabled())
@@ -2692 +2667 @@
     }
 
-    if (page) {
-        if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
+    if (auto* page = frame().page()) {
+        if (auto* scrollingCoordinator = page->scrollingCoordinator())
             scrollingCoordinator->frameViewLayoutUpdated(this);
     }
@@ -3557 +3532 @@
         InspectorInstrumentation::didPaint(renderView, p, rect);
         // FIXME: should probably not fire milestones for snapshot painting. https://bugs.webkit.org/show_bug.cgi?id=117623
-        firePaintRelatedMilestones();
+        firePaintRelatedMilestonesIfNeeded();
     }
 }
@@ -4148 +4123 @@
 }
 
-void FrameView::firePaintRelatedMilestones()
+void FrameView::fireLayoutRelatedMilestonesIfNeeded()
+{
+    LayoutMilestones requestedMilestones = 0;
+    LayoutMilestones milestonesAchieved = 0;
+    Page* page = frame().page();
+    if (page)
+        requestedMilestones = page->requestedLayoutMilestones();
+
+    if (m_firstLayoutCallbackPending) {
+        m_firstLayoutCallbackPending = false;
+        frame().loader().didFirstLayout();
+        if (requestedMilestones & DidFirstLayout)
+            milestonesAchieved |= DidFirstLayout;
+        if (frame().isMainFrame())
+            page->startCountingRelevantRepaintedObjects();
+    }
+    updateIsVisuallyNonEmpty();
+
+    // If the layout was done with pending sheets, we are not in fact visually non-empty yet.
+    if (m_isVisuallyNonEmpty && !frame().document()->didLayoutWithPendingStylesheets() && m_firstVisuallyNonEmptyLayoutCallbackPending) {
+        m_firstVisuallyNonEmptyLayoutCallbackPending = false;
+        if (requestedMilestones & DidFirstVisuallyNonEmptyLayout)
+            milestonesAchieved |= DidFirstVisuallyNonEmptyLayout;
+    }
+
+    if (milestonesAchieved && frame().isMainFrame())
+        frame().loader().didLayout(milestonesAchieved);
+}
+
+void FrameView::firePaintRelatedMilestonesIfNeeded()
 {
     Page* page = frame().page();

trunk/Source/WebCore/page/FrameView.h

@@ -432 +432 @@
 
     void addPaintPendingMilestones(LayoutMilestones);
-    void firePaintRelatedMilestones();
+    void firePaintRelatedMilestonesIfNeeded();
+    void fireLayoutRelatedMilestonesIfNeeded();
     LayoutMilestones milestonesPendingPaint() const { return m_milestonesPendingPaint; }
 

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -3756 +3756 @@
         return;
 
-    m_renderView.frameView().firePaintRelatedMilestones();
+    m_renderView.frameView().firePaintRelatedMilestonesIfNeeded();
 }