Attachment 371441 Details for Bug 198497 – Patch

[patch] Patch

bug-198497-20190605150237.patch (text/plain), 12.62 KB, created by Said Abou-Hallawa on 2019-06-05 15:02:37 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Said Abou-Hallawa

Created: 2019-06-05 15:02:37 PDT

Size: 12.62 KB

patch

obsolete

>Subversion Revision: 246069
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ce97d1d8dfb9d3d9c0cdb9e031f2b073905c6594..8d467931205d0ac5d29a8fcf5b4282f303eaece2 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+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 NOBODY (OOPS!).
>+
>+        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-04  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][IFC] Decouple float placement and line shrinking
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 4c1eef4132ddea26ac6725d926387f821446baee..7bb0fa0f351300f69112c7feafe05a66c565582e 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,26 @@
>+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 NOBODY (OOPS!).
>+
>+        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-04  Chris Dumez  <cdumez@apple.com>
> 
>         Lazily construct the NetworkHTTPSUpgradeChecker
>diff --git a/Source/WebCore/inspector/InspectorController.cpp b/Source/WebCore/inspector/InspectorController.cpp
>index 71a07612e7581acfc5c46324b03c6fd39233a567..e0693a850a7d8e06658ab102b16a2fd081a70e6c 100644
>--- a/Source/WebCore/inspector/InspectorController.cpp
>+++ b/Source/WebCore/inspector/InspectorController.cpp
>@@ -507,6 +507,11 @@ JSC::VM& InspectorController::vm()
>     return commonVM();
> }
> 
>+void InspectorController::willComposite(Frame& frame)
>+{
>+    InspectorInstrumentation::willComposite(frame);
>+}
>+
> void InspectorController::didComposite(Frame& frame)
> {
>     InspectorInstrumentation::didComposite(frame);
>diff --git a/Source/WebCore/inspector/InspectorController.h b/Source/WebCore/inspector/InspectorController.h
>index a511ec5bb1c67c0a1bb314573ee7dafb2ad74d74..4bea43263da7037f188659662da8dbf6d831596d 100644
>--- a/Source/WebCore/inspector/InspectorController.h
>+++ b/Source/WebCore/inspector/InspectorController.h
>@@ -97,6 +97,7 @@ public:
> 
>     WEBCORE_EXPORT void setIndicating(bool);
> 
>+    WEBCORE_EXPORT void willComposite(Frame&);
>     WEBCORE_EXPORT void didComposite(Frame&);
> 
>     bool isUnderTest() const { return m_isUnderTest; }
>diff --git a/Source/WebCore/inspector/agents/InspectorTimelineAgent.cpp b/Source/WebCore/inspector/agents/InspectorTimelineAgent.cpp
>index a1d3d9144a5212cd94e79345109c2de7dffb8436..f63683fa3926b3c75ef1962eabf1691f55e10c29 100644
>--- a/Source/WebCore/inspector/agents/InspectorTimelineAgent.cpp
>+++ b/Source/WebCore/inspector/agents/InspectorTimelineAgent.cpp
>@@ -364,7 +364,6 @@ void InspectorTimelineAgent::willComposite(Frame& frame)
> 
> void InspectorTimelineAgent::didComposite()
> {
>-    ASSERT(m_startedComposite);
>     didCompleteCurrentRecord(TimelineRecordType::Composite);
>     m_startedComposite = false;
> }
>diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
>index 120f12df0741a533d3090f95a621a6e5d4733c0d..a1bb494e2ebe394c3dc35524b0d663f1f1882783 100644
>--- a/Source/WebCore/page/FrameView.cpp
>+++ b/Source/WebCore/page/FrameView.cpp
>@@ -1148,10 +1148,6 @@ bool FrameView::isEnclosedInCompositingLayer() const
> 
> bool FrameView::flushCompositingStateIncludingSubframes()
> {
>-#if PLATFORM(COCOA)
>-    InspectorInstrumentation::willComposite(frame());
>-#endif
>-
>     bool allFramesFlushed = flushCompositingStateForThisFrame(frame());
> 
>     for (Frame* child = frame().tree().firstRenderedChild(); child; child = child->tree().traverseNextRendered(m_frame.ptr())) {
>diff --git a/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h b/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h
>index 433159f8594ffea0de8cef79a2660de6147770da..a6d7b8065f7787c54106cab461564f65a7b3bd8c 100644
>--- a/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h
>+++ b/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h
>@@ -114,7 +114,7 @@ private:
>     void updateScrolledExposedRect();
>     void updateRootLayers();
> 
>-    void flushInitialDeferredPaint();
>+    void addCommitHandlers();
>     void flushLayers();
> 
>     WebCore::TiledBacking* mainFrameTiledBacking() const;
>diff --git a/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm b/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm
>index b291df0b7dfdf7939e7daae723420265db19c363..4e4762c9d1b977e3f151d5d971b75bb198c88ad7 100644
>--- a/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm
>+++ b/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm
>@@ -325,6 +325,29 @@ bool RemoteLayerTreeDrawingArea::adjustLayerFlushThrottling(WebCore::LayerFlushT
>     }
>     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()
> {
>@@ -356,13 +379,7 @@ void RemoteLayerTreeDrawingArea::flushLayers()
>     if (m_scrolledViewExposedRect)
>         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)
>         m_webPage.mainFrameView()->invalidateImagesWithAsyncDecodes();
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index f2b24c2833437d8970db8c2b42d843ab296eb75f..ecfc958ac26400652aaf0208e60abe2fdd2e57da 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -1206,6 +1206,9 @@ public:
>     bool userIsInteracting() const { return m_userIsInteracting; }
>     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&&);
> 
>@@ -1935,6 +1938,7 @@ private:
>     OptionSet<LayerTreeFreezeReason> m_layerTreeFreezeReasons;
>     bool m_isSuspended { false };
>     bool m_needsFontAttributes { false };
>+    bool m_firstFlushAfterCommit { false };
> #if PLATFORM(COCOA)
>     WeakPtr<RemoteObjectRegistry> m_remoteObjectRegistry;
> #endif
>diff --git a/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h b/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h
>index 1d8aeeeb55f5946b99887a436ce1f2208cfda9dc..5a1d7b9b0df54507b674bec1c708da4714b9f1eb 100644
>--- a/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h
>+++ b/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h
>@@ -88,6 +88,7 @@ private:
> 
>     bool addMilestonesToDispatch(OptionSet<WebCore::LayoutMilestone> paintMilestones) override;
> 
>+    void addCommitHandlers();
>     enum class FlushType { Normal, TransientZoom };
>     void flushLayers(FlushType = FlushType::Normal);
> 
>diff --git a/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm b/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm
>index 8095e67d8f6dff21bac6a1e04706bb1aacfaf07e..ecfd9225068789ca8c436592096fff066ae30129 100644
>--- a/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm
>+++ b/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm
>@@ -431,6 +431,31 @@ void TiledCoreAnimationDrawingArea::addTransactionCallbackID(CallbackID callback
>     scheduleCompositingLayerFlush();
> }
> 
>+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)
> {
>     if (layerTreeStateIsFrozen())
>@@ -455,16 +480,8 @@ void TiledCoreAnimationDrawingArea::flushLayers(FlushType flushType)
>         if (m_viewOverlayRootLayer)
>             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();
> 
> #if ENABLE(ASYNC_SCROLLING)

Actions: View | Formatted Diff | Diff

Attachments on bug 198497: 371214 | 371235 | 371378 | 371441