cc: switch to new WebLayerTreeViewClient API for requesting font atlas

This change switches the compositor to the new WebLayerTreeViewClient::createFontAtlas() API. LayerTreeHost now manages whether a font atlas needs to be loaded.

commit order:
https://bugs.webkit.org/show_bug.cgi?id=102958
https://codereview.chromium.org/11413123
https://bugs.webkit.org/show_bug.cgi?id=102960

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=170614

[jamesr, 2012-11-27 21:07:43]

https://codereview.chromium.org/11413123/diff/2001/cc/heads_up_display_layer.h
File cc/heads_up_display_layer.h (right):

https://codereview.chromium.org/11413123/diff/2001/cc/heads_up_display_layer....
cc/heads_up_display_layer.h:38: bool m_hasFontAtlas;
why do we need a bool? Can't we just null-check m_fontAtlas?

https://codereview.chromium.org/11413123/diff/2001/cc/layer_tree_host.cc
File cc/layer_tree_host.cc (right):

https://codereview.chromium.org/11413123/diff/2001/cc/layer_tree_host.cc#newc...
cc/layer_tree_host.cc:310: createFontAtlasIfNeeded();
will we ever get setShowFPSCounter(false) calls? we don't need the font atlas
unless the fps counter is actually being shown, right?

[danakj, 2012-11-27 21:26:14]

https://codereview.chromium.org/11413123/diff/2001/cc/heads_up_display_layer.h
File cc/heads_up_display_layer.h (right):

https://codereview.chromium.org/11413123/diff/2001/cc/heads_up_display_layer....
cc/heads_up_display_layer.h:38: bool m_hasFontAtlas;
On 2012/11/27 21:07:44, jamesr wrote:
> why do we need a bool? Can't we just null-check m_fontAtlas?

This seems needed to remember we gave a font atlas to the impl thread.

[egraether, 2012-11-27 21:49:22]

https://codereview.chromium.org/11413123/diff/2001/cc/heads_up_display_layer.h
File cc/heads_up_display_layer.h (right):

https://codereview.chromium.org/11413123/diff/2001/cc/heads_up_display_layer....
cc/heads_up_display_layer.h:38: bool m_hasFontAtlas;
On 2012/11/27 21:26:14, danakj wrote:
> On 2012/11/27 21:07:44, jamesr wrote:
> > why do we need a bool? Can't we just null-check m_fontAtlas?
> 
> This seems needed to remember we gave a font atlas to the impl thread.

Yeah, it's there because as soon as the fontAtlas is passed to the HudLayerImpl
you can't check it anymore.

This check will probably fall off, as soon as I start fixing the hudLayer
creation/destruction.

https://codereview.chromium.org/11413123/diff/2001/cc/layer_tree_host.cc
File cc/layer_tree_host.cc (right):

https://codereview.chromium.org/11413123/diff/2001/cc/layer_tree_host.cc#newc...
cc/layer_tree_host.cc:310: createFontAtlasIfNeeded();
On 2012/11/27 21:07:44, jamesr wrote:
> will we ever get setShowFPSCounter(false) calls? we don't need the font atlas
> unless the fps counter is actually being shown, right?

right, I'll add a check for that.
ATM there shouldn't be a show=false call unless there was a show=true call
before.

[piman, 2012-11-27 21:54:02]

https://codereview.chromium.org/11413123/diff/2001/cc/layer_tree_host_client.h
File cc/layer_tree_host_client.h (right):

https://codereview.chromium.org/11413123/diff/2001/cc/layer_tree_host_client....
cc/layer_tree_host_client.h:41: virtual scoped_ptr<cc::FontAtlas>
createFontAtlas() = 0;
drive-by: you'll need to fix ui::Compositor in ui/compositor/compositor.{h, cc}

Make sure you build aura.

[jamesr, 2012-11-28 19:20:51]

lgtm

[piman, 2012-11-28 19:25:03]

LGTM+nit

https://codereview.chromium.org/11413123/diff/8002/ui/compositor/compositor.cc
File ui/compositor/compositor.cc (right):

https://codereview.chromium.org/11413123/diff/8002/ui/compositor/compositor.c...
ui/compositor/compositor.cc:547: {
nit: chrome style - '{' goes on the previous line.

[commit-bot: I haz the power, 2012-11-28 19:43:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/egraether@chromium.org/11413123/3008

[commit-bot: I haz the power, 2012-11-28 20:05:22]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[egraether, 2012-11-30 21:27:55]

I rebased this patch to the LayerTreeDebugState patch I landed yesterday
(https://codereview.chromium.org/11414017/).

Some parts in LayerTreeHost changed.
Can you please have another look James?

https://codereview.chromium.org/11413123/diff/2006/cc/layer_tree_host.cc
File cc/layer_tree_host.cc (right):

https://codereview.chromium.org/11413123/diff/2006/cc/layer_tree_host.cc#newc...
cc/layer_tree_host.cc:347: void LayerTreeHost::willCommit()
With LayerTreeDebugState and font atlas creation on demand it was possible to
put back all hudLayer logic into LayerTreeHost::willCommit().

https://codereview.chromium.org/11413123/diff/2006/cc/layer_tree_host.cc#newc...
cc/layer_tree_host.cc:362: m_hudLayer = 0;
The hudLayer gets released when no longer needed, the fontAtlas is lost as well
though.

[jamesr, 2012-11-30 22:10:24]

lgtm

[commit-bot: I haz the power, 2012-11-30 22:26:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/egraether@chromium.org/11413123/2006

[commit-bot: I haz the power, 2012-12-01 00:42:42]

Change committed as 170614