It reduces the memory consumption and improves the performance. Also, since performance is better we don't need to keep the scroll surface allocated all the time, we can create it on demand and keep it allocated only while scrolling too fast.
Created attachment 319601 [details] Patch This reduced my ephy UI process memory from 307MB to 183MB. Still higher than using X11 (54MB), but much better.
Attachment 319601 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/cairo/BackingStoreBackendCairoImpl.cpp:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/platform/graphics/cairo/BackingStoreBackendCairoImpl.cpp:44: More than one command on the same line in if [whitespace/parens] [4] Total errors found: 2 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 319601 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=319601&action=review > Source/WebCore/platform/graphics/cairo/BackingStoreBackendCairoImpl.cpp:37 > + cairo_surface_set_user_data(surface.get(), &s_surfaceDataKey, surfaceData, [](void* data) { fastFree(data); }); Style checker doesn't like this, and I agree that it would be easier to read on two lines. But up to you. > Source/WebCore/platform/graphics/cairo/BackingStoreBackendCairoImpl.cpp:44 > + , m_scrolledHysteresis([this](HysteresisState state) { if (state == HysteresisState::Stopped) m_scrollSurface = nullptr; }, scrollHysteresisDuration) Style checker doesn't like this either. Please let's not use lambdas in initializer lists. Can't you initialize it inside the body of the constructor? > Source/WebCore/platform/graphics/cairo/BackingStoreBackendCairoImpl.h:21 > #include "BackingStoreBackendCairo.h" I think it belongs inside the #if USE(CAIRO) guard.
(In reply to Michael Catanzaro from comment #3) > Comment on attachment 319601 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=319601&action=review Thanks for the review. > > Source/WebCore/platform/graphics/cairo/BackingStoreBackendCairoImpl.cpp:37 > > + cairo_surface_set_user_data(surface.get(), &s_surfaceDataKey, surfaceData, [](void* data) { fastFree(data); }); > > Style checker doesn't like this, and I agree that it would be easier to read > on two lines. But up to you. I agree when using a longer body, but in this case I think it reads better this way. > > Source/WebCore/platform/graphics/cairo/BackingStoreBackendCairoImpl.cpp:44 > > + , m_scrolledHysteresis([this](HysteresisState state) { if (state == HysteresisState::Stopped) m_scrollSurface = nullptr; }, scrollHysteresisDuration) > > Style checker doesn't like this either. Please let's not use lambdas in > initializer lists. Can't you initialize it inside the body of the > constructor? No, because it's stack allocated. > > Source/WebCore/platform/graphics/cairo/BackingStoreBackendCairoImpl.h:21 > > #include "BackingStoreBackendCairo.h" > > I think it belongs inside the #if USE(CAIRO) guard. There's no problem because BackingStoreBackendCairo.h is already guarded, but I agree it's better inside the guard here in the header.
Committed r221521: <http://trac.webkit.org/changeset/221521>