Bug 49396 – Make WebWidget actively notify client when compositing enables

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Nat Duca]

Right now, RenderWidget pasively determines when compositing enables based on checking isAcceleratedCompositingActive.

This patch adds active notification for when compositing enables and disables.

[Nat Duca]

Created attachment 73633 [details]
Proposed patch

[Darin Fisher (:fishd, Google)]

Comment on attachment 73633 [details]
Proposed patch

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

> WebKit/chromium/src/WebViewImpl.cpp:2383
> +        m_layerRenderer->finish(); // finish all GL rendering before we hide the window? TODO(nduca) fixthisshit

TODO(nduca) -> FIXME and leave a more descriptive comment for someone else who may be reading this code.

> WebKit/chromium/src/WebViewImpl.cpp:2390
> +        m_layerRenderer->resizeOnscreenContent(WebCore::IntSize(std::max(1, m_size.width),

no need for the WebCore prefix.

> WebKit/chromium/public/WebWidgetClient.h:54
> +    virtual void didChangeAcceleratedCompositingActive(bool active) { }

nit: how about didActivateAcceleratedCompositing(bool)?

[Nat Duca]

Created attachment 73919 [details]
Round 2

[Darin Fisher (:fishd, Google)]

Comment on attachment 73919 [details]
Round 2

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

> WebKit/chromium/src/WebViewImpl.cpp:-1022
> -        m_layerRenderer->present();

nice to see this cleanup!

> WebKit/chromium/public/WebWidgetClient.h:54
> +    virtual void didAcceleratedCompositingEnable(bool active) { }

why this over didActivateAcceleratedCompositing?  when vangelis added the isAcceleratedCompositingActive,
i suggested using "enabled" instead of "active" as the suffix, but he argued against "enable" since that
sounds like the compile-time option (is the code enabled at all).  so, we went with "active", and he used
that throughout the codebase (both webkit and chrome).  can we stick with "active"/"activate"?

[Nat Duca]

> why this over didActivateAcceleratedCompositing?

My bad. I made this change last Friday before I got your emailed ocmmets; the name I jotted down on the post-it was different. Will upload a new patch shortly.

[Nat Duca]

Created attachment 73929 [details]
Better name

[WebKit Commit Bot]

Comment on attachment 73929 [details]
Better name

Clearing flags on attachment: 73929

Committed r72058: <http://trac.webkit.org/changeset/72058>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.