Changeset 246142 in webkit

Timestamp:

Jun 5, 2019 8:39:08 PM (5 years ago)

Author:

commit-queue@webkit.org

Message:

[Cocoa] REGRESSION(r244182): Inspector thinks CA commits can be nested
​https://bugs.webkit.org/show_bug.cgi?id=198497

Patch by Said Abou-Hallawa <​sabouhallawa@apple.com> on 2019-06-05
Reviewed by Simon Fraser.

Source/WebCore:

WebInspecter should coalesce nested composites as one recorded composite.
This can be done by ensuring that we only process CA preCommit and postCommit
once per nested commits.

• inspector/InspectorController.cpp:

(WebCore::InspectorController::willComposite):

• inspector/InspectorController.h:

Export willComposite(). We want to call willComposite()/ didComposite()
from CA preCommit and postCommit handlers in flushLayers().

• inspector/agents/InspectorTimelineAgent.cpp:

(WebCore::InspectorTimelineAgent::didComposite):
Unrelated change: didComposite() should not assert that we're in the middle
of a composite. Web Inspector may connect in the middle of a composite.

• page/FrameView.cpp:

(WebCore::FrameView::flushCompositingStateIncludingSubframes):
InspectorController::willComposite() will be called form CA preCommit
handler of flushLayers().

Source/WebKit:

Call InspectorController::willComposite() from the CA preCommit handler
similar to the call to InspectorController::didComposite() in the CA
postCommit handler. Ensure these calls will be once for nested commits.

• WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h:
• WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:

(WebKit::RemoteLayerTreeDrawingArea::addCommitHandlers):
(WebKit::RemoteLayerTreeDrawingArea::flushLayers):

• WebProcess/WebPage/WebPage.h:

(WebKit::WebPage::firstFlushAfterCommit const):
(WebKit::WebPage::setFirstFlushAfterCommit):

• WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
• WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:

(WebKit::TiledCoreAnimationDrawingArea::addCommitHandlers):
(WebKit::TiledCoreAnimationDrawingArea::flushLayers):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/inspector/InspectorController.cpp (modified) (1 diff)
• WebCore/inspector/InspectorController.h (modified) (1 diff)
• WebCore/inspector/agents/InspectorTimelineAgent.cpp (modified) (1 diff)
• WebCore/page/FrameView.cpp (modified) (1 diff)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h (modified) (1 diff)
• WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm (modified) (2 diffs)
• WebKit/WebProcess/WebPage/WebPage.h (modified) (2 diffs)
• WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h (modified) (1 diff)
• WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-06-05  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        [Cocoa] REGRESSION(r244182): Inspector thinks CA commits can be nested
+        https://bugs.webkit.org/show_bug.cgi?id=198497
+
+        Reviewed by Simon Fraser.
+
+        WebInspecter should coalesce nested composites as one recorded composite.
+        This can be done by ensuring that we only process CA preCommit and postCommit
+        once per nested commits.
+
+        * inspector/InspectorController.cpp:
+        (WebCore::InspectorController::willComposite):
+        * inspector/InspectorController.h:
+        Export willComposite(). We want to call willComposite()/ didComposite()
+        from CA preCommit and postCommit handlers in flushLayers().
+
+        * inspector/agents/InspectorTimelineAgent.cpp:
+        (WebCore::InspectorTimelineAgent::didComposite):
+        Unrelated change: didComposite() should not assert that we're in the middle
+        of a composite. Web Inspector may connect in the middle of a composite.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::flushCompositingStateIncludingSubframes):
+        InspectorController::willComposite() will be called form CA preCommit
+        handler of flushLayers().
+
 2019-06-05  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebCore/inspector/InspectorController.cpp

@@ -508 +508 @@
 }
 
+void InspectorController::willComposite(Frame& frame)
+{
+    InspectorInstrumentation::willComposite(frame);
+}
+
 void InspectorController::didComposite(Frame& frame)
 {

trunk/Source/WebCore/inspector/InspectorController.h

@@ -98 +98 @@
     WEBCORE_EXPORT void setIndicating(bool);
 
+    WEBCORE_EXPORT void willComposite(Frame&);
     WEBCORE_EXPORT void didComposite(Frame&);
 

trunk/Source/WebCore/inspector/agents/InspectorTimelineAgent.cpp

@@ -365 +365 @@
 void InspectorTimelineAgent::didComposite()
 {
-    ASSERT(m_startedComposite);
     didCompleteCurrentRecord(TimelineRecordType::Composite);
     m_startedComposite = false;

trunk/Source/WebCore/page/FrameView.cpp

@@ -1149 +1149 @@
 bool FrameView::flushCompositingStateIncludingSubframes()
 {
-#if PLATFORM(COCOA)
-    InspectorInstrumentation::willComposite(frame());
-#endif
-
     bool allFramesFlushed = flushCompositingStateForThisFrame(frame());
 

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-06-05  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        [Cocoa] REGRESSION(r244182): Inspector thinks CA commits can be nested
+        https://bugs.webkit.org/show_bug.cgi?id=198497
+
+        Reviewed by Simon Fraser.
+
+        Call InspectorController::willComposite() from the CA preCommit handler
+        similar to the call to InspectorController::didComposite() in the CA
+        postCommit handler. Ensure these calls will be once for nested commits.
+
+        * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h:
+        * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
+        (WebKit::RemoteLayerTreeDrawingArea::addCommitHandlers):
+        (WebKit::RemoteLayerTreeDrawingArea::flushLayers):
+        * WebProcess/WebPage/WebPage.h:
+        (WebKit::WebPage::firstFlushAfterCommit const):
+        (WebKit::WebPage::setFirstFlushAfterCommit):
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
+        (WebKit::TiledCoreAnimationDrawingArea::addCommitHandlers):
+        (WebKit::TiledCoreAnimationDrawingArea::flushLayers):
+
 2019-06-05  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h

@@ -115 +115 @@
     void updateRootLayers();
 
-    void flushInitialDeferredPaint();
+    void addCommitHandlers();
     void flushLayers();
 

trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm

@@ -326 +326 @@
     return true;
 }
+    
+void RemoteLayerTreeDrawingArea::addCommitHandlers()
+{
+    if (m_webPage.firstFlushAfterCommit())
+        return;
+
+    [CATransaction addCommitHandler:[retainedPage = makeRefPtr(&m_webPage)] {
+        if (Page* corePage = retainedPage->corePage()) {
+            if (Frame* coreFrame = retainedPage->mainFrame())
+                corePage->inspectorController().willComposite(*coreFrame);
+        }
+    } forPhase:kCATransactionPhasePreCommit];
+    
+    [CATransaction addCommitHandler:[retainedPage = makeRefPtr(&m_webPage)] {
+        if (Page* corePage = retainedPage->corePage()) {
+            if (Frame* coreFrame = retainedPage->mainFrame())
+                corePage->inspectorController().didComposite(*coreFrame);
+        }
+        retainedPage->setFirstFlushAfterCommit(false);
+    } forPhase:kCATransactionPhasePostCommit];
+    
+    m_webPage.setFirstFlushAfterCommit(true);
+}
 
 void RemoteLayerTreeDrawingArea::flushLayers()
@@ -357 +380 @@
         visibleRect.intersect(m_scrolledViewExposedRect.value());
 
-    RefPtr<WebPage> protectedWebPage = &m_webPage;
-    [CATransaction addCommitHandler:[protectedWebPage] {
-        if (Page* corePage = protectedWebPage->corePage()) {
-            if (Frame* coreFrame = protectedWebPage->mainFrame())
-                corePage->inspectorController().didComposite(*coreFrame);
-        }
-    } forPhase:kCATransactionPhasePostCommit];
+    addCommitHandlers();
 
     if (m_nextFlushIsForImmediatePaint)

trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

@@ -1207 +1207 @@
     void setUserIsInteracting(bool userIsInteracting) { m_userIsInteracting = userIsInteracting; }
 
+    bool firstFlushAfterCommit() const { return m_firstFlushAfterCommit; }
+    void setFirstFlushAfterCommit(bool f) { m_firstFlushAfterCommit = f; }
+    
 private:
     WebPage(WebCore::PageIdentifier, WebPageCreationParameters&&);
@@ -1934 +1937 @@
     bool m_isSuspended { false };
     bool m_needsFontAttributes { false };
+    bool m_firstFlushAfterCommit { false };
 #if PLATFORM(COCOA)
     WeakPtr<RemoteObjectRegistry> m_remoteObjectRegistry;

trunk/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h

@@ -89 +89 @@
     bool addMilestonesToDispatch(OptionSet<WebCore::LayoutMilestone> paintMilestones) override;
 
+    void addCommitHandlers();
     enum class FlushType { Normal, TransientZoom };
     void flushLayers(FlushType = FlushType::Normal);

trunk/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm

@@ -432 +432 @@
 }
 
+void TiledCoreAnimationDrawingArea::addCommitHandlers()
+{
+    if (m_webPage.firstFlushAfterCommit())
+        return;
+
+    [CATransaction addCommitHandler:[retainedPage = makeRefPtr(&m_webPage)] {
+        if (Page* corePage = retainedPage->corePage()) {
+            if (Frame* coreFrame = retainedPage->mainFrame())
+                corePage->inspectorController().willComposite(*coreFrame);
+        }
+    } forPhase:kCATransactionPhasePreCommit];
+
+    [CATransaction addCommitHandler:[retainedPage = makeRefPtr(&m_webPage)] {
+        if (Page* corePage = retainedPage->corePage()) {
+            if (Frame* coreFrame = retainedPage->mainFrame())
+                corePage->inspectorController().didComposite(*coreFrame);
+        }
+        if (auto drawingArea = static_cast<TiledCoreAnimationDrawingArea*>(retainedPage->drawingArea()))
+            drawingArea->sendPendingNewlyReachedPaintingMilestones();
+        retainedPage->setFirstFlushAfterCommit(false);
+    } forPhase:kCATransactionPhasePostCommit];
+    
+    m_webPage.setFirstFlushAfterCommit(true);
+}
+
 void TiledCoreAnimationDrawingArea::flushLayers(FlushType flushType)
 {
@@ -456 +481 @@
             m_viewOverlayRootLayer->flushCompositingState(visibleRect);
 
-        RefPtr<WebPage> retainedPage = &m_webPage;
-        [CATransaction addCommitHandler:[retainedPage] {
-            if (Page* corePage = retainedPage->corePage()) {
-                if (Frame* coreFrame = retainedPage->mainFrame())
-                    corePage->inspectorController().didComposite(*coreFrame);
-            }
-            if (auto drawingArea = static_cast<TiledCoreAnimationDrawingArea*>(retainedPage->drawingArea()))
-                drawingArea->sendPendingNewlyReachedPaintingMilestones();
-        } forPhase:kCATransactionPhasePostCommit];
-
+        addCommitHandlers();
+        
         bool didFlushAllFrames = m_webPage.mainFrameView()->flushCompositingStateIncludingSubframes();