[webkit-changes] [198525] trunk/Source/WebKit2

Tue, 22 Mar 2016 02:45:07 -0700

Title: [198525] trunk/Source/WebKit2
Revision
198525
Author
[email protected]
Date
2016-03-22 02:44:47 -0700 (Tue, 22 Mar 2016)

Log Message

[CoordinatedGraphics] Polish std::function<> usage in ThreadedCompositor, CoordinatedGraphicsScene
https://bugs.webkit.org/show_bug.cgi?id=155726

Reviewed by Darin Adler.

Adjust the methods in ThreadedCompositor and CoordinatedGraphicsScene
classes to accept std::function<> arguments via rvalue references. This
should prevent both unnecessary copies and moves.

Fix lambda expressions that are most commonly used to construct the
std::function<> objects so that they don't capture-by-value by default,
but instead list the captured values verbosely. This part alone exposed
an issue in ThreadedCompositor::didChangeVisibleRect() where we were
capturing the `this' value by default, instead of a protector RefPtr.

* Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:
(WebKit::CoordinatedGraphicsScene::dispatchOnMainThread):
(WebKit::CoordinatedGraphicsScene::dispatchOnClientRunLoop):
(WebKit::CoordinatedGraphicsScene::paintToCurrentGLContext):
(WebKit::CoordinatedGraphicsScene::onNewBufferAvailable):
(WebKit::CoordinatedGraphicsScene::commitSceneState):
(WebKit::CoordinatedGraphicsScene::purgeGLResources):
(WebKit::CoordinatedGraphicsScene::commitScrollOffset):
(WebKit::CoordinatedGraphicsScene::appendUpdate):
(WebKit::CoordinatedGraphicsScene::setActive):
* Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h:
* Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:
(WebKit::CompositingRunLoop::CompositingRunLoop):
(WebKit::CompositingRunLoop::callOnCompositingRunLoop):
(WebKit::ThreadedCompositor::setNeedsDisplay):
(WebKit::ThreadedCompositor::setNativeSurfaceHandleForCompositing):
(WebKit::ThreadedCompositor::setDeviceScaleFactor):
(WebKit::ThreadedCompositor::didChangeViewportSize):
(WebKit::ThreadedCompositor::didChangeViewportAttribute):
(WebKit::ThreadedCompositor::didChangeContentsSize):
(WebKit::ThreadedCompositor::scrollTo):
(WebKit::ThreadedCompositor::scrollBy):
(WebKit::ThreadedCompositor::didChangeVisibleRect):
(WebKit::ThreadedCompositor::callOnCompositingThread):
* Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h:

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (198524 => 198525)


--- trunk/Source/WebKit2/ChangeLog	2016-03-22 09:38:12 UTC (rev 198524)
+++ trunk/Source/WebKit2/ChangeLog	2016-03-22 09:44:47 UTC (rev 198525)
@@ -1,5 +1,48 @@
 2016-03-22  Zan Dobersek  <[email protected]>
 
+        [CoordinatedGraphics] Polish std::function<> usage in ThreadedCompositor, CoordinatedGraphicsScene
+        https://bugs.webkit.org/show_bug.cgi?id=155726
+
+        Reviewed by Darin Adler.
+
+        Adjust the methods in ThreadedCompositor and CoordinatedGraphicsScene
+        classes to accept std::function<> arguments via rvalue references. This
+        should prevent both unnecessary copies and moves.
+
+        Fix lambda expressions that are most commonly used to construct the
+        std::function<> objects so that they don't capture-by-value by default,
+        but instead list the captured values verbosely. This part alone exposed
+        an issue in ThreadedCompositor::didChangeVisibleRect() where we were
+        capturing the `this' value by default, instead of a protector RefPtr.
+
+        * Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:
+        (WebKit::CoordinatedGraphicsScene::dispatchOnMainThread):
+        (WebKit::CoordinatedGraphicsScene::dispatchOnClientRunLoop):
+        (WebKit::CoordinatedGraphicsScene::paintToCurrentGLContext):
+        (WebKit::CoordinatedGraphicsScene::onNewBufferAvailable):
+        (WebKit::CoordinatedGraphicsScene::commitSceneState):
+        (WebKit::CoordinatedGraphicsScene::purgeGLResources):
+        (WebKit::CoordinatedGraphicsScene::commitScrollOffset):
+        (WebKit::CoordinatedGraphicsScene::appendUpdate):
+        (WebKit::CoordinatedGraphicsScene::setActive):
+        * Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h:
+        * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:
+        (WebKit::CompositingRunLoop::CompositingRunLoop):
+        (WebKit::CompositingRunLoop::callOnCompositingRunLoop):
+        (WebKit::ThreadedCompositor::setNeedsDisplay):
+        (WebKit::ThreadedCompositor::setNativeSurfaceHandleForCompositing):
+        (WebKit::ThreadedCompositor::setDeviceScaleFactor):
+        (WebKit::ThreadedCompositor::didChangeViewportSize):
+        (WebKit::ThreadedCompositor::didChangeViewportAttribute):
+        (WebKit::ThreadedCompositor::didChangeContentsSize):
+        (WebKit::ThreadedCompositor::scrollTo):
+        (WebKit::ThreadedCompositor::scrollBy):
+        (WebKit::ThreadedCompositor::didChangeVisibleRect):
+        (WebKit::ThreadedCompositor::callOnCompositingThread):
+        * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h:
+
+2016-03-22  Zan Dobersek  <[email protected]>
+
         [CoordinatedGraphics] Prefer RunLoop::main().dispatch() over callOnMainThread()
         https://bugs.webkit.org/show_bug.cgi?id=155725

Modified: trunk/Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp (198524 => 198525)


--- trunk/Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp	2016-03-22 09:38:12 UTC (rev 198524)
+++ trunk/Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp	2016-03-22 09:44:47 UTC (rev 198525)
@@ -34,7 +34,7 @@
 
 namespace WebKit {
 
-void CoordinatedGraphicsScene::dispatchOnMainThread(std::function<void()> function)
+void CoordinatedGraphicsScene::dispatchOnMainThread(std::function<void()>&& function)
 {
     if (isMainThread())
         function();
@@ -42,7 +42,7 @@
         RunLoop::main().dispatch(WTFMove(function));
 }
 
-void CoordinatedGraphicsScene::dispatchOnClientRunLoop(std::function<void()> function)
+void CoordinatedGraphicsScene::dispatchOnClientRunLoop(std::function<void()>&& function)
 {
     if (&m_clientRunLoop == &RunLoop::current())
         function();
@@ -115,7 +115,7 @@
 
     if (currentRootLayer->descendantsOrSelfHaveRunningAnimations()) {
         RefPtr<CoordinatedGraphicsScene> protector(this);
-        dispatchOnClientRunLoop([=] {
+        dispatchOnClientRunLoop([protector] {
             protector->updateViewport();
         });
     }
@@ -203,7 +203,7 @@
 void CoordinatedGraphicsScene::onNewBufferAvailable()
 {
     RefPtr<CoordinatedGraphicsScene> protector(this);
-    dispatchOnClientRunLoop([=] {
+    dispatchOnClientRunLoop([protector] {
         protector->updateViewport();
     });
 }
@@ -609,7 +609,7 @@
 
     // The pending tiles state is on its way for the screen, tell the web process to render the next one.
     RefPtr<CoordinatedGraphicsScene> protector(this);
-    dispatchOnMainThread([=] {
+    dispatchOnMainThread([protector] {
         protector->renderNextFrame();
     });
 }
@@ -677,7 +677,7 @@
     setActive(false);
 
     RefPtr<CoordinatedGraphicsScene> protector(this);
-    dispatchOnMainThread([=] {
+    dispatchOnMainThread([protector] {
         protector->purgeBackingStores();
     });
 }
@@ -690,7 +690,7 @@
 void CoordinatedGraphicsScene::commitScrollOffset(uint32_t layerID, const IntSize& offset)
 {
     RefPtr<CoordinatedGraphicsScene> protector(this);
-    dispatchOnMainThread([=] {
+    dispatchOnMainThread([protector, layerID, offset] {
         protector->dispatchCommitScrollOffset(layerID, offset);
     });
 }
@@ -716,7 +716,7 @@
     m_client = 0;
 }
 
-void CoordinatedGraphicsScene::appendUpdate(std::function<void()> function)
+void CoordinatedGraphicsScene::appendUpdate(std::function<void()>&& function)
 {
     if (!m_isActive)
         return;
@@ -738,7 +738,7 @@
     m_isActive = active;
     if (m_isActive) {
         RefPtr<CoordinatedGraphicsScene> protector(this);
-        dispatchOnMainThread([=] {
+        dispatchOnMainThread([protector] {
             protector->renderNextFrame();
         });
     }

Modified: trunk/Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h (198524 => 198525)


--- trunk/Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h	2016-03-22 09:38:12 UTC (rev 198524)
+++ trunk/Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h	2016-03-22 09:44:47 UTC (rev 198525)
@@ -73,7 +73,7 @@
     void paintToCurrentGLContext(const WebCore::TransformationMatrix&, float, const WebCore::FloatRect&, const WebCore::Color& backgroundColor, bool drawsBackground, const WebCore::FloatPoint&, WebCore::TextureMapper::PaintFlags = 0);
     void paintToGraphicsContext(PlatformGraphicsContext*, const WebCore::Color& backgroundColor, bool drawsBackground);
     void detach();
-    void appendUpdate(std::function<void()>);
+    void appendUpdate(std::function<void()>&&);
 
     WebCore::TextureMapperLayer* findScrollableContentsLayerAt(const WebCore::FloatPoint&);
 
@@ -131,8 +131,8 @@
     void syncRemoteContent();
     void adjustPositionForFixedLayers(const WebCore::FloatPoint& contentPosition);
 
-    void dispatchOnMainThread(std::function<void()>);
-    void dispatchOnClientRunLoop(std::function<void()>);
+    void dispatchOnMainThread(std::function<void()>&&);
+    void dispatchOnClientRunLoop(std::function<void()>&&);
     void updateViewport();
     void renderNextFrame();
     void purgeBackingStores();

Modified: trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp (198524 => 198525)


--- trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp	2016-03-22 09:38:12 UTC (rev 198524)
+++ trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp	2016-03-22 09:44:47 UTC (rev 198525)
@@ -52,7 +52,7 @@
         WaitUntilNextFrame,
     };
 
-    CompositingRunLoop(std::function<void()> updateFunction)
+    CompositingRunLoop(std::function<void()>&& updateFunction)
         : m_runLoop(RunLoop::current())
         , m_updateTimer(m_runLoop, this, &CompositingRunLoop::updateTimerFired)
         , m_updateFunction(WTFMove(updateFunction))
@@ -60,7 +60,7 @@
     {
     }
 
-    void callOnCompositingRunLoop(std::function<void()> function)
+    void callOnCompositingRunLoop(std::function<void()>&& function)
     {
         if (&m_runLoop == &RunLoop::current()) {
             function();
@@ -130,7 +130,7 @@
 void ThreadedCompositor::setNeedsDisplay()
 {
     RefPtr<ThreadedCompositor> protector(this);
-    callOnCompositingThread([=] {
+    callOnCompositingThread([protector] {
         protector->scheduleDisplayImmediately();
     });
 }
@@ -138,7 +138,7 @@
 void ThreadedCompositor::setNativeSurfaceHandleForCompositing(uint64_t handle)
 {
     RefPtr<ThreadedCompositor> protector(this);
-    callOnCompositingThread([=] {
+    callOnCompositingThread([protector, handle] {
         protector->m_nativeSurfaceHandle = handle;
         protector->m_scene->setActive(true);
     });
@@ -147,24 +147,24 @@
 void ThreadedCompositor::setDeviceScaleFactor(float scale)
 {
     RefPtr<ThreadedCompositor> protector(this);
-    callOnCompositingThread([=] {
+    callOnCompositingThread([protector, scale] {
         protector->m_deviceScaleFactor = scale;
         protector->scheduleDisplayImmediately();
     });
 }
 
-void ThreadedCompositor::didChangeViewportSize(const IntSize& newSize)
+void ThreadedCompositor::didChangeViewportSize(const IntSize& size)
 {
     RefPtr<ThreadedCompositor> protector(this);
-    callOnCompositingThread([=] {
-        protector->viewportController()->didChangeViewportSize(newSize);
+    callOnCompositingThread([protector, size] {
+        protector->viewportController()->didChangeViewportSize(size);
     });
 }
 
 void ThreadedCompositor::didChangeViewportAttribute(const ViewportAttributes& attr)
 {
     RefPtr<ThreadedCompositor> protector(this);
-    callOnCompositingThread([=] {
+    callOnCompositingThread([protector, attr] {
         protector->viewportController()->didChangeViewportAttribute(attr);
     });
 }
@@ -172,7 +172,7 @@
 void ThreadedCompositor::didChangeContentsSize(const IntSize& size)
 {
     RefPtr<ThreadedCompositor> protector(this);
-    callOnCompositingThread([=] {
+    callOnCompositingThread([protector, size] {
         protector->viewportController()->didChangeContentsSize(size);
     });
 }
@@ -180,7 +180,7 @@
 void ThreadedCompositor::scrollTo(const IntPoint& position)
 {
     RefPtr<ThreadedCompositor> protector(this);
-    callOnCompositingThread([=] {
+    callOnCompositingThread([protector, position] {
         protector->viewportController()->scrollTo(position);
     });
 }
@@ -188,7 +188,7 @@
 void ThreadedCompositor::scrollBy(const IntSize& delta)
 {
     RefPtr<ThreadedCompositor> protector(this);
-    callOnCompositingThread([=] {
+    callOnCompositingThread([protector, delta] {
         protector->viewportController()->scrollBy(delta);
     });
 }
@@ -250,10 +250,11 @@
 
 void ThreadedCompositor::didChangeVisibleRect()
 {
+    RefPtr<ThreadedCompositor> protector(this);
     FloatRect visibleRect = viewportController()->visibleContentsRect();
     float scale = viewportController()->pageScaleFactor();
-    RunLoop::main().dispatch([=] {
-        m_client->setVisibleContentsRect(visibleRect, FloatPoint::zero(), scale);
+    RunLoop::main().dispatch([protector, visibleRect, scale] {
+        protector->m_client->setVisibleContentsRect(visibleRect, FloatPoint::zero(), scale);
     });
 
     scheduleDisplayImmediately();
@@ -289,7 +290,7 @@
     setNeedsDisplay();
 }
 
-void ThreadedCompositor::callOnCompositingThread(std::function<void()> function)
+void ThreadedCompositor::callOnCompositingThread(std::function<void()>&& function)
 {
     m_compositingRunLoop->callOnCompositingRunLoop(WTFMove(function));
 }

Modified: trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h (198524 => 198525)


--- trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h	2016-03-22 09:38:12 UTC (rev 198524)
+++ trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h	2016-03-22 09:44:47 UTC (rev 198525)
@@ -94,7 +94,7 @@
     WebCore::GLContext* glContext();
     SimpleViewportController* viewportController() { return m_viewportController.get(); }
 
-    void callOnCompositingThread(std::function<void()>);
+    void callOnCompositingThread(std::function<void()>&&);
     void createCompositingThread();
     void runCompositingThread();
     void terminateCompositingThread();

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to