Bug 131353 – Unify and factor out page overlay implementations

• Component: WebKit2
• Status: RESOLVED FIXED

[Tim Horton]

There's no reason (as far as I can tell) to reimplement straightforward and platform-independent page overlay functionality in various places, and there's no reason for it to be scattered all over DrawingArea and WebPage.

[Tim Horton]

Created attachment 228823 [details]
WIP

[WebKit Commit Bot]

Attachment 228823 [details] did not pass style-queue:


ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.h:45:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:1222:  Should have a space between // and comment  [whitespace/comments] [4]
Total errors found: 2 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Created attachment 228875 [details]
patch

[Simon Fraser (smfr)]

Comment on attachment 228875 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228875&action=review

> Source/WebKit2/WebProcess/WebPage/PageOverlayController.cpp:89
> +    m_webPage->drawingArea()->scheduleCompositingLayerFlush();

It this necessary? I would have thought that the GraphicsLayer tree modifications were enough to trigger it.

> Source/WebKit2/WebProcess/WebPage/PageOverlayController.cpp:114
> +    m_webPage->drawingArea()->scheduleCompositingLayerFlush();

Ditto.

> Source/WebKit2/WebProcess/WebPage/PageOverlayController.cpp:128
> +    m_webPage->drawingArea()->scheduleCompositingLayerFlush();

Ditto

[Tim Horton]

Comment on attachment 228875 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228875&action=review

>> Source/WebKit2/WebProcess/WebPage/PageOverlayController.cpp:89
>> +    m_webPage->drawingArea()->scheduleCompositingLayerFlush();
> 
> It this necessary? I would have thought that the GraphicsLayer tree modifications were enough to trigger it.

I think it's not, if I implement notifyFlushRequired here (since all of these GraphicsLayers use this class as their client) and forward it along to the DrawingArea. That's much neater, thanks!

[Tim Horton]

Created attachment 228881 [details]
found more code to delete

[Tim Horton]

Created attachment 228883 [details]
rebase

[Tim Horton]

Created attachment 228886 [details]
rebasex2

still building post-rebase. need to test some  exposedrect-enabled apps.

[Tim Horton]

http://trac.webkit.org/changeset/166975

[Tim Horton]

Created attachment 228912 [details]
fix a tiny bug

[Anders Carlsson]

Comment on attachment 228912 [details]
fix a tiny bug

View in context: https://bugs.webkit.org/attachment.cgi?id=228912&action=review

> Source/WebKit2/WebProcess/WebPage/PageOverlayController.cpp:105
> +    if (m_pageOverlays.size())

This should be

if (!m_pageOverlays.isEmpty())

[Tim Horton]

http://trac.webkit.org/changeset/166978

[Csaba Osztrogonác]

New bug report to fix the Coordinated Graphics build and functionality -
https://bugs.webkit.org/show_bug.cgi?id=131425

[Csaba Osztrogonác]

(In reply to comment #13)
> New bug report to fix the Coordinated Graphics build and functionality -
> https://bugs.webkit.org/show_bug.cgi?id=131425

It seems GTK (DrawingAreaImpl.cpp) needs similar fix too.