Skip to content

Commit

Permalink
Merge r221514 - [GTK][Wayland] Opening FedoraProject's pastebin chews…
Browse files Browse the repository at this point in the history 
… 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:
  • Loading branch information
@carlosgcampos
carlosgcampos committed Sep 4, 2017
1 parent 755a7f1 commit ca281ce
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 19 deletions.
32 changes: 32 additions & 0 deletions Source/WebKit/ChangeLog
View 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-08-31 Carlos Garcia Campos <cgarcia@igalia.com>

[GTK] Several InputMethodFilter tests are failing and crashing
Expand Down
70 changes: 56 additions & 14 deletions Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp
View file
Expand Up @@ -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);
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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);
Expand All @@ -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 = {
Expand Down
6 changes: 5 additions & 1 deletion Source/WebKit/UIProcess/gtk/WaylandCompositor.h
View file
Expand Up @@ -87,19 +87,23 @@ class WaylandCompositor {
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;
WeakPtr<Buffer> m_pendingBuffer;
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; }
Expand Down
10 changes: 7 additions & 3 deletions Source/WebKit/WebProcess/WebPage/gtk/AcceleratedSurfaceWayland.cpp
View file
Expand Up @@ -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);
}
Expand Down
4 changes: 3 additions & 1 deletion Source/WebKit/WebProcess/WebPage/gtk/AcceleratedSurfaceWayland.h
View file
Expand Up @@ -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:
Expand Down

0 comments on commit ca281ce

Please sign in to comment.