Changeset 221514 in webkit

Timestamp:

Sep 2, 2017 12:05:46 AM (7 years ago)

Author:

Carlos Garcia Campos

Message:

[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:

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/gtk/WaylandCompositor.cpp (modified) (5 diffs)
• UIProcess/gtk/WaylandCompositor.h (modified) (2 diffs)
• WebProcess/WebPage/gtk/AcceleratedSurfaceWayland.cpp (modified) (1 diff)
• WebProcess/WebPage/gtk/AcceleratedSurfaceWayland.h (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp

@@ -157 +157 @@
 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)
@@ -171 +176 @@
 }
 
+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()
 {
@@ -200 +225 @@
     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);
 }
 
@@ -219 +244 @@
 }
 
+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)
@@ -231 +280 @@
 
     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);
 }
 

trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.h

@@ -88 +88 @@
         void commit();
 
-        void setWebPage(WebPageProxy* webPage) { m_webPage = webPage; }
+        void setWebPage(WebPageProxy*);
         bool prepareTextureForPainting(unsigned&, WebCore::IntSize&);
 
     private:
+        void flushFrameCallbacks();
+        void flushPendingFrameCallbacks();
         void makePendingBufferCurrent();
 
@@ -99 +101 @@
         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 };
     };
 

trunk/Source/WebKit/WebProcess/WebPage/gtk/AcceleratedSurfaceWayland.cpp

@@ -44 +44 @@
 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);

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

@@ -39 +39 @@
 public:
     static std::unique_ptr<AcceleratedSurfaceWayland> create(WebPage&, Client&);
-    ~AcceleratedSurfaceWayland();
+    ~AcceleratedSurfaceWayland() = default;
 
     uint64_t window() const override { return reinterpret_cast<uint64_t>(m_window); }
@@ -46 +46 @@
     bool shouldPaintMirrored() const override { return true; }
 
+    void initialize() override;
+    void finalize() override;
     void didRenderFrame() override;