Changeset 205075 in webkit

Timestamp:

Aug 27, 2016, 2:18:54 AM (9 years ago)

Author:

Carlos Garcia Campos

Message:

[GTK][Threaded Compositor] Several flaky tests
​https://bugs.webkit.org/show_bug.cgi?id=161242

Reviewed by Michael Catanzaro.

We still have a lot of flaky tests since we switched to the threaded compositor. The UI process might
take the screenshot too early, before everything is actually painted. I can't reproduce the problem, so this is
actually a speculative fix or workaround. Our implementation of DrawingArea::dispatchAfterEnsuringDrawing() is
quite simple, we just dispatch the callback in the next run loop iteration, which doesn't really ensures any
drawing at all. So, we can wait for draw events before dispatching the given callback. Since we don't really
know if draw events were already processed before dispatchAfterEnsuringDrawing() is called, or if there will be
more than one damage event in a short time, this patch waits up to 1 second for draw events, and if a draw
happens it stops if there isn't another draw event in the next 100ms. This should ensure a drawing if it was
really needed.

• UIProcess/DrawingAreaProxyImpl.cpp:

(WebKit::DrawingAreaProxyImpl::DrawingMonitor::DrawingMonitor):
(WebKit::DrawingAreaProxyImpl::DrawingMonitor::~DrawingMonitor):
(WebKit::DrawingAreaProxyImpl::DrawingMonitor::webViewDrawCallback):
(WebKit::DrawingAreaProxyImpl::DrawingMonitor::start):
(WebKit::DrawingAreaProxyImpl::DrawingMonitor::stop):
(WebKit::DrawingAreaProxyImpl::DrawingMonitor::didDraw):
(WebKit::DrawingAreaProxyImpl::dispatchAfterEnsuringDrawing):

• UIProcess/DrawingAreaProxyImpl.h:

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/DrawingAreaProxyImpl.cpp (modified) (2 diffs)
• UIProcess/DrawingAreaProxyImpl.h (modified) (1 diff)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2016-08-27  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK][Threaded Compositor] Several flaky tests
+        https://bugs.webkit.org/show_bug.cgi?id=161242
+
+        Reviewed by Michael Catanzaro.
+
+        We still have a lot of flaky tests since we switched to the threaded compositor. The UI process might
+        take the screenshot too early, before everything is actually painted. I can't reproduce the problem, so this is
+        actually a speculative fix or workaround. Our implementation of DrawingArea::dispatchAfterEnsuringDrawing() is
+        quite simple, we just dispatch the callback in the next run loop iteration, which doesn't really ensures any
+        drawing at all. So, we can wait for draw events before dispatching the given callback. Since we don't really
+        know if draw events were already processed before dispatchAfterEnsuringDrawing() is called, or if there will be
+        more than one damage event in a short time, this patch waits up to 1 second for draw events, and if a draw
+        happens it stops if there isn't another draw event in the next 100ms. This should ensure a drawing if it was
+        really needed.
+
+        * UIProcess/DrawingAreaProxyImpl.cpp:
+        (WebKit::DrawingAreaProxyImpl::DrawingMonitor::DrawingMonitor):
+        (WebKit::DrawingAreaProxyImpl::DrawingMonitor::~DrawingMonitor):
+        (WebKit::DrawingAreaProxyImpl::DrawingMonitor::webViewDrawCallback):
+        (WebKit::DrawingAreaProxyImpl::DrawingMonitor::start):
+        (WebKit::DrawingAreaProxyImpl::DrawingMonitor::stop):
+        (WebKit::DrawingAreaProxyImpl::DrawingMonitor::didDraw):
+        (WebKit::DrawingAreaProxyImpl::dispatchAfterEnsuringDrawing):
+        * UIProcess/DrawingAreaProxyImpl.h:
+
 2016-08-26  Sam Weinig  <sam@webkit.org>
 

trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp

@@ -37 +37 @@
 #include <WebCore/Region.h>
 
+#if PLATFORM(GTK)
+#include <gtk/gtk.h>
+#endif
+
 using namespace WebCore;
 
@@ -188 +192 @@
 }
 
+DrawingAreaProxyImpl::DrawingMonitor::DrawingMonitor(WebPageProxy& webPage)
+    : m_webPage(webPage)
+    , m_timer(RunLoop::main(), this, &DrawingMonitor::stop)
+{
+}
+
+DrawingAreaProxyImpl::DrawingMonitor::~DrawingMonitor()
+{
+    m_callback = nullptr;
+    stop();
+}
+
+int DrawingAreaProxyImpl::DrawingMonitor::webViewDrawCallback(DrawingAreaProxyImpl::DrawingMonitor* monitor)
+{
+    monitor->didDraw();
+    return FALSE;
+}
+
+void DrawingAreaProxyImpl::DrawingMonitor::start(std::function<void (CallbackBase::Error)> callback)
+{
+    m_startTime = monotonicallyIncreasingTimeMS();
+    m_callback = callback;
+#if PLATFORM(GTK)
+    g_signal_connect_swapped(m_webPage.viewWidget(), "draw", reinterpret_cast<GCallback>(webViewDrawCallback), this);
+    m_timer.startOneShot(1);
+#else
+    m_timer.startOneShot(0);
+#endif
+}
+
+void DrawingAreaProxyImpl::DrawingMonitor::stop()
+{
+    m_timer.stop();
+#if PLATFORM(GTK)
+    g_signal_handlers_disconnect_by_func(m_webPage.viewWidget(), reinterpret_cast<gpointer>(webViewDrawCallback), this);
+#endif
+    m_startTime = 0;
+    if (m_callback) {
+        m_callback(CallbackBase::Error::None);
+        m_callback = nullptr;
+    }
+}
+
+void DrawingAreaProxyImpl::DrawingMonitor::didDraw()
+{
+    // We wait up to 1 second for draw events. If there are several draw events queued quickly,
+    // we want to wait until all of them have been processed, so after receiving a draw, we wait
+    // up to 100ms for the next one or stop.
+    if (monotonicallyIncreasingTimeMS() - m_startTime > 1000)
+        stop();
+    else
+        m_timer.startOneShot(0.100);
+}
+
+void DrawingAreaProxyImpl::dispatchAfterEnsuringDrawing(std::function<void(CallbackBase::Error)> callbackFunction)
+{
+    if (!m_webPageProxy.isValid()) {
+        callbackFunction(CallbackBase::Error::OwnerWasInvalidated);
+        return;
+    }
+
+    if (!m_drawingMonitor)
+        m_drawingMonitor = std::make_unique<DrawingAreaProxyImpl::DrawingMonitor>(m_webPageProxy);
+    m_drawingMonitor->start(callbackFunction);
+}
+
 } // namespace WebKit

trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h

@@ -61 +61 @@
     void discardBackingStore();
 
+    void dispatchAfterEnsuringDrawing(std::function<void(CallbackBase::Error)>) override;
+
+    class DrawingMonitor {
+        WTF_MAKE_NONCOPYABLE(DrawingMonitor); WTF_MAKE_FAST_ALLOCATED;
+    public:
+        DrawingMonitor(WebPageProxy&);
+        ~DrawingMonitor();
+
+        void start(std::function<void (CallbackBase::Error)>);
+
+    private:
+        static int webViewDrawCallback(DrawingMonitor*);
+
+        void stop();
+        void didDraw();
+
+        WebPageProxy& m_webPage;
+        double m_startTime { 0 };
+        std::function<void (CallbackBase::Error)> m_callback;
+        RunLoop::Timer<DrawingMonitor> m_timer;
+    };
+
     bool m_isBackingStoreDiscardable { true };
     std::unique_ptr<BackingStore> m_backingStore;
     RunLoop::Timer<DrawingAreaProxyImpl> m_discardBackingStoreTimer;
+    std::unique_ptr<DrawingMonitor> m_drawingMonitor;
 };