[webkit-reviews] review granted: [Bug 37313] [Qt] Fix or remove the runtime flag for accelerated compositing. : [Attachment 53745] name changes, better punctuation grammar :)

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Noam Rosenthal
<noam.rosenthal at nokia.com>'s request for review:
Bug 37313: [Qt] Fix or remove the runtime flag for accelerated compositing.
https://bugs.webkit.org/show_bug.cgi?id=37313

Attachment 53745: name changes, better punctuation grammar :)
https://bugs.webkit.org/attachment.cgi?id=53745&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 36db5d8..d796d7b 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,29 @@
> +2010-04-19  No'am Rosenthal	<noam.rosenthal at nokia.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   [Qt] Fix or remove the runtime flag for accelerated compositing.
> +
> +	   This adds a way for a chrome client to disallow layers from becoming
composited,
> +	   even if the settings enable AC.

Please expand AC. You're the only people who use that contraction :)

> diff --git a/WebCore/page/ChromeClient.h b/WebCore/page/ChromeClient.h
> @@ -214,6 +214,9 @@ namespace WebCore {
>	   // Sets a flag to specify that the view needs to be updated, so we
need
>	   // to do an eager layout before the drawing.
>	   virtual void scheduleCompositingLayerSync() = 0;
> +	   // Returns whether or not the client can render the composited
layer,
> +	   // regardless of the settings.
> +	   virtual bool allowsAcceleratedCompositing() { return true; }

Method should be |const|


> diff --git a/WebCore/platform/qt/QWebPageClient.h
b/WebCore/platform/qt/QWebPageClient.h
> index f03ff08..491a9f3 100644
> --- a/WebCore/platform/qt/QWebPageClient.h
> +++ b/WebCore/platform/qt/QWebPageClient.h
> @@ -55,6 +55,7 @@ public:
>      // if scheduleSync is true, we schedule a sync ourselves. otherwise,
>      // we wait for the next update and sync the layers then.
>      virtual void markForSync(bool scheduleSync = false) {}
> +    virtual bool allowsAcceleratedCompositing() { return false; }

Fix this to match the constness in the API.

> diff --git a/WebCore/rendering/RenderLayerCompositor.cpp
b/WebCore/rendering/RenderLayerCompositor.cpp
> index 9430613..6f856b0 100644
> --- a/WebCore/rendering/RenderLayerCompositor.cpp
> +++ b/WebCore/rendering/RenderLayerCompositor.cpp
> @@ -131,6 +131,15 @@ void
RenderLayerCompositor::cacheAcceleratedCompositingFlags()
>	   showRepaintCounter = settings->showRepaintCounter();
>      }
>  
> +    // We allow the chrome to override the settings, in case the page is
rendered
> +    // on a chrome that doesn't allow accelerated compositing.

I think a comment like:
 // Allow the ChromeClient to veto use of accelerated compositing.
would be clearer.