[GTK][Wayland] Opening FedoraProject's pastebin chews CPU
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 2 Sep 2017 07:05:46 +0000 (07:05 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 2 Sep 2017 07:05:46 +0000 (07:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=175942

Reviewed by Žan Doberšek.

This regressed when we introduced the display refresh monitor. The monitor schedules updates immediately,
because we removed the option to not do frame sync in X11 to let swapBuffers do the throttling, but that's
not happening in Wayland because the nested compositor is dispatching frame callbacks on surface commit.
We need to ensure that frame callbacks are dispatched on every monitor refresh, because swapBuffers waits for
frame callbacks to be queued on display.

* UIProcess/gtk/WaylandCompositor.cpp:
(WebKit::WaylandCompositor::Surface::~Surface): Destroy pending frame callbacks too.
(WebKit::WaylandCompositor::Surface::setWebPage): Add a tick callback to the web view widget to flush all
committed frame callbacks on every frame update.
(WebKit::WaylandCompositor::Surface::requestFrame): Add the callbacks to m_pendingFrameCallbackList.
(WebKit::WaylandCompositor::Surface::flushFrameCallbacks): Dispatch all committed frame callabcks.
(WebKit::WaylandCompositor::Surface::flushPendingFrameCallbacks): Dispatch all pending frame callbacks.
(WebKit::WaylandCompositor::Surface::commit): Do not dispatch frame callbacks here, move them to the list of
committed frame callbacks that will be dispatched on the next frame clock update.
* UIProcess/gtk/WaylandCompositor.h:
(WebKit::WaylandCompositor::Surface::setWebPage): Moved to the cpp.
* WebProcess/WebPage/gtk/AcceleratedSurfaceWayland.cpp:
(WebKit::AcceleratedSurfaceWayland::AcceleratedSurfaceWayland): Move surface initialization and destruction to
the compositing thread.
(WebKit::AcceleratedSurfaceWayland::initialize):
(WebKit::AcceleratedSurfaceWayland::finalize):
(WebKit::AcceleratedSurfaceWayland::~AcceleratedSurfaceWayland): Deleted.
* WebProcess/WebPage/gtk/AcceleratedSurfaceWayland.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@221514 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp
Source/WebKit/UIProcess/gtk/WaylandCompositor.h
Source/WebKit/WebProcess/WebPage/gtk/AcceleratedSurfaceWayland.cpp
Source/WebKit/WebProcess/WebPage/gtk/AcceleratedSurfaceWayland.h

index bcfa136..21014bc 100644 (file)
@@ -1,3 +1,35 @@
+2017-09-02  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK][Wayland] Opening FedoraProject's pastebin chews CPU
+        https://bugs.webkit.org/show_bug.cgi?id=175942
+
+        Reviewed by Žan Doberšek.
+
+        This regressed when we introduced the display refresh monitor. The monitor schedules updates immediately,
+        because we removed the option to not do frame sync in X11 to let swapBuffers do the throttling, but that's
+        not happening in Wayland because the nested compositor is dispatching frame callbacks on surface commit.
+        We need to ensure that frame callbacks are dispatched on every monitor refresh, because swapBuffers waits for
+        frame callbacks to be queued on display.
+
+        * UIProcess/gtk/WaylandCompositor.cpp:
+        (WebKit::WaylandCompositor::Surface::~Surface): Destroy pending frame callbacks too.
+        (WebKit::WaylandCompositor::Surface::setWebPage): Add a tick callback to the web view widget to flush all
+        committed frame callbacks on every frame update.
+        (WebKit::WaylandCompositor::Surface::requestFrame): Add the callbacks to m_pendingFrameCallbackList.
+        (WebKit::WaylandCompositor::Surface::flushFrameCallbacks): Dispatch all committed frame callabcks.
+        (WebKit::WaylandCompositor::Surface::flushPendingFrameCallbacks): Dispatch all pending frame callbacks.
+        (WebKit::WaylandCompositor::Surface::commit): Do not dispatch frame callbacks here, move them to the list of
+        committed frame callbacks that will be dispatched on the next frame clock update.
+        * UIProcess/gtk/WaylandCompositor.h:
+        (WebKit::WaylandCompositor::Surface::setWebPage): Moved to the cpp.
+        * WebProcess/WebPage/gtk/AcceleratedSurfaceWayland.cpp:
+        (WebKit::AcceleratedSurfaceWayland::AcceleratedSurfaceWayland): Move surface initialization and destruction to
+        the compositing thread.
+        (WebKit::AcceleratedSurfaceWayland::initialize):
+        (WebKit::AcceleratedSurfaceWayland::finalize):
+        (WebKit::AcceleratedSurfaceWayland::~AcceleratedSurfaceWayland): Deleted.
+        * WebProcess/WebPage/gtk/AcceleratedSurfaceWayland.h:
+
 2017-09-01  Youenn Fablet  <youenn@apple.com>
 
         Do not Reject CacheStorage promises when updating the persistent filesystem data fails
index 64692da..7594444 100644 (file)
@@ -156,7 +156,12 @@ WaylandCompositor::Surface::Surface()
 
 WaylandCompositor::Surface::~Surface()
 {
+    setWebPage(nullptr);
+
     // Destroy pending frame callbacks.
+    auto pendingList = WTFMove(m_pendingFrameCallbackList);
+    for (auto* resource : pendingList)
+        wl_resource_destroy(resource);
     auto list = WTFMove(m_frameCallbackList);
     for (auto* resource : list)
         wl_resource_destroy(resource);
@@ -170,6 +175,26 @@ WaylandCompositor::Surface::~Surface()
     glDeleteTextures(1, &m_texture);
 }
 
+void WaylandCompositor::Surface::setWebPage(WebPageProxy* webPage)
+{
+    if (m_webPage) {
+        flushPendingFrameCallbacks();
+        flushFrameCallbacks();
+        gtk_widget_remove_tick_callback(m_webPage->viewWidget(), m_tickCallbackID);
+        m_tickCallbackID = 0;
+    }
+
+    m_webPage = webPage;
+    if (!m_webPage)
+        return;
+
+    m_tickCallbackID = gtk_widget_add_tick_callback(m_webPage->viewWidget(), [](GtkWidget*, GdkFrameClock*, gpointer userData) -> gboolean {
+        auto* surface = static_cast<Surface*>(userData);
+        surface->flushFrameCallbacks();
+        return G_SOURCE_CONTINUE;
+    }, this, nullptr);
+}
+
 void WaylandCompositor::Surface::makePendingBufferCurrent()
 {
     if (m_pendingBuffer == m_buffer)
@@ -199,10 +224,10 @@ void WaylandCompositor::Surface::requestFrame(struct wl_resource* resource)
 {
     wl_resource_set_implementation(resource, nullptr, this, [](struct wl_resource* resource) {
         auto* surface = static_cast<WaylandCompositor::Surface*>(wl_resource_get_user_data(resource));
-        if (size_t item = surface->m_frameCallbackList.find(resource) != notFound)
-            surface->m_frameCallbackList.remove(item);
+        if (size_t item = surface->m_pendingFrameCallbackList.find(resource) != notFound)
+            surface->m_pendingFrameCallbackList.remove(item);
     });
-    m_frameCallbackList.append(resource);
+    m_pendingFrameCallbackList.append(resource);
 }
 
 bool WaylandCompositor::Surface::prepareTextureForPainting(unsigned& texture, IntSize& textureSize)
@@ -218,8 +243,32 @@ bool WaylandCompositor::Surface::prepareTextureForPainting(unsigned& texture, In
     return true;
 }
 
+void WaylandCompositor::Surface::flushFrameCallbacks()
+{
+    auto list = WTFMove(m_frameCallbackList);
+    for (auto* resource : list) {
+        wl_callback_send_done(resource, 0);
+        wl_resource_destroy(resource);
+    }
+}
+
+void WaylandCompositor::Surface::flushPendingFrameCallbacks()
+{
+    auto list = WTFMove(m_pendingFrameCallbackList);
+    for (auto* resource : list) {
+        wl_callback_send_done(resource, 0);
+        wl_resource_destroy(resource);
+    }
+}
+
 void WaylandCompositor::Surface::commit()
 {
+    if (!m_webPage) {
+        makePendingBufferCurrent();
+        flushPendingFrameCallbacks();
+        return;
+    }
+
     EGLDisplay eglDisplay = PlatformDisplay::sharedDisplay().eglDisplay();
     if (m_image != EGL_NO_IMAGE_KHR)
         eglDestroyImage(eglDisplay, m_image);
@@ -230,18 +279,11 @@ void WaylandCompositor::Surface::commit()
     m_imageSize = m_pendingBuffer->size();
 
     makePendingBufferCurrent();
-    if (m_webPage)
-        m_webPage->setViewNeedsDisplay(IntRect(IntPoint::zero(), m_webPage->viewSize()));
 
-    // From a Wayland point-of-view frame callbacks should be fired where the
-    // compositor knows it has *used* the committed contents, so firing them here
-    // can be surprising but we don't need them as a throttling mechanism because
-    // rendering synchronization is handled elsewhere by WebKit.
-    auto list = WTFMove(m_frameCallbackList);
-    for (auto* resource : list) {
-        wl_callback_send_done(resource, 0);
-        wl_resource_destroy(resource);
-    }
+    m_webPage->setViewNeedsDisplay(IntRect(IntPoint::zero(), m_webPage->viewSize()));
+
+    auto list = WTFMove(m_pendingFrameCallbackList);
+    m_frameCallbackList.appendVector(list);
 }
 
 static const struct wl_surface_interface surfaceInterface = {
index 14b9a83..9bbeb7a 100644 (file)
@@ -87,10 +87,12 @@ public:
         void requestFrame(struct wl_resource*);
         void commit();
 
-        void setWebPage(WebPageProxy* webPage) { m_webPage = webPage; }
+        void setWebPage(WebPageProxy*);
         bool prepareTextureForPainting(unsigned&, WebCore::IntSize&);
 
     private:
+        void flushFrameCallbacks();
+        void flushPendingFrameCallbacks();
         void makePendingBufferCurrent();
 
         WeakPtr<Buffer> m_buffer;
@@ -98,8 +100,10 @@ public:
         unsigned m_texture;
         EGLImageKHR m_image;
         WebCore::IntSize m_imageSize;
+        Vector<wl_resource*> m_pendingFrameCallbackList;
         Vector<wl_resource*> m_frameCallbackList;
         WebPageProxy* m_webPage { nullptr };
+        unsigned m_tickCallbackID { 0 };
     };
 
     bool isRunning() const { return !!m_display; }
index c7332b6..4787c99 100644 (file)
@@ -43,13 +43,17 @@ std::unique_ptr<AcceleratedSurfaceWayland> AcceleratedSurfaceWayland::create(Web
 
 AcceleratedSurfaceWayland::AcceleratedSurfaceWayland(WebPage& webPage, Client& client)
     : AcceleratedSurface(webPage, client)
-    , m_surface(WebProcess::singleton().waylandCompositorDisplay()->createSurface())
-    , m_window(wl_egl_window_create(m_surface.get(), std::max(1, m_size.width()), std::max(1, m_size.height())))
 {
+}
+
+void AcceleratedSurfaceWayland::initialize()
+{
+    m_surface = WebProcess::singleton().waylandCompositorDisplay()->createSurface();
+    m_window = wl_egl_window_create(m_surface.get(), std::max(1, m_size.width()), std::max(1, m_size.height()));
     WebProcess::singleton().waylandCompositorDisplay()->bindSurfaceToPage(m_surface.get(), m_webPage);
 }
 
-AcceleratedSurfaceWayland::~AcceleratedSurfaceWayland()
+void AcceleratedSurfaceWayland::finalize()
 {
     wl_egl_window_destroy(m_window);
 }
index c3c886e..4320215 100644 (file)
@@ -38,13 +38,15 @@ class AcceleratedSurfaceWayland final : public AcceleratedSurface {
     WTF_MAKE_NONCOPYABLE(AcceleratedSurfaceWayland); WTF_MAKE_FAST_ALLOCATED;
 public:
     static std::unique_ptr<AcceleratedSurfaceWayland> create(WebPage&, Client&);
-    ~AcceleratedSurfaceWayland();
+    ~AcceleratedSurfaceWayland() = default;
 
     uint64_t window() const override { return reinterpret_cast<uint64_t>(m_window); }
     uint64_t surfaceID() const override { return m_webPage.pageID(); }
     bool resize(const WebCore::IntSize&) override;
     bool shouldPaintMirrored() const override { return true; }
 
+    void initialize() override;
+    void finalize() override;
     void didRenderFrame() override;
 
 private: