Use WebCompositor only when --enable-threaded-compositing.

Use WebCompositor only when --enable-threaded-compositing.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106841

[nduca, 2011-10-18 21:45:19]

Not sure if this is the cleanest way to do this. Alternative suggestions?

The end-goal of this is to use whether we called WebCompositor::setThread to
decide whether we're in threaded mode versus not, rather than having CCSettings
control the choice and then being vulnerable to
CCSettings::enableThreadedCompositing being true but not having a thread.

[jamesr, 2011-10-18 21:48:56]

On 2011/10/18 21:45:19, nduca wrote:
> Not sure if this is the cleanest way to do this. Alternative suggestions?
> 
> The end-goal of this is to use whether we called WebCompositor::setThread to
> decide whether we're in threaded mode versus not, rather than having
CCSettings
> control the choice and then being vulnerable to
> CCSettings::enableThreadedCompositing being true but not having a thread.

I think it'd be nicer to check this in render_thread_impl.cc and decide whether
or not to create the CompositorThread at all based off of the flag, and then
null-check anywhere that grabs RenderThreadImpl::compositor_thread() - right now
there's one caller in RenderWidget. That way if the flag isn't set we don't
construct a thread at all.

[darin (slow to review), 2011-10-18 22:58:00]

> I think it'd be nicer to check this in render_thread_impl.cc and decide
whether
> or not to create the CompositorThread at all based off of the flag, and then
> null-check anywhere that grabs RenderThreadImpl::compositor_thread() - right
now
> there's one caller in RenderWidget. That way if the flag isn't set we don't
> construct a thread at all.

+1

[nduca, 2011-10-18 22:58:35]

Sounds like a plan. I'll get back to y'all after this message from our
sponsors... ;)

[nduca, 2011-10-20 21:27:28]

darin, jamesr: ping

[jamesr, 2011-10-20 22:51:52]

I like this pattern - what do you think, Darin?

http://codereview.chromium.org/8344021/diff/5001/content/renderer/render_widg...
File content/renderer/render_widget.cc (right):

http://codereview.chromium.org/8344021/diff/5001/content/renderer/render_widg...
content/renderer/render_widget.cc:930: if (compositor_thread)
in webkit we normally fold the assignment into the if() to scope the local. Do
we do this in chrome?

[darin (slow to review), 2011-10-21 05:03:35]

LGTM

http://codereview.chromium.org/8344021/diff/5001/content/renderer/render_widg...
File content/renderer/render_widget.cc (right):

http://codereview.chromium.org/8344021/diff/5001/content/renderer/render_widg...
content/renderer/render_widget.cc:930: if (compositor_thread)
On 2011/10/20 22:51:52, jamesr wrote:
> in webkit we normally fold the assignment into the if() to scope the local. Do
> we do this in chrome?

I don't think there is a strong convention.  It'd probably lead to some link
breaking here, which might be worse :)