Remove WGC3D::isContextLost references from cc

Remove WGC3D::isContextLost references from cc

The notion of whether a context is lost is a property of both the gpu::
context itself and the system providing it. For instance, the content
context provider checks if there is an error in the context or if the IPC
channel backing the command buffer is down. Thus, asking if a context is
lost really should go through the ContextProvider. This patch routes all
lost context checks from cc through the ContextProvider.

Unfortunately, this required reworking the program initialization code
considerably due to the way some DCHECKs were written. The new model is that
the program binding itself is inert upon construction and all initialization
is done in the Initialize() call. This call is still made eagerly for some
expected-to-be-common programs and lazily for the rest. This patch moves
when the linkProgram() call is issued slightly for the eagerly compiled programs,
but it shouldn't make any difference in practice.

This patch also revamps TextureLayerClient to hide the backing context since
cc only makes two calls (check for loss and insert rate limiting token) on
the client's context.

R=piman
BUG=181120
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233325

[jamesr, 2013-11-01 22:32:56]

This really snowballed from what I was expecting, but we get some nice wins too
like removing the bind_uniform_locations bool (it was true 100% of the time) and
a lot of tiny extra heap allocations for program bindings.

[piman, 2013-11-01 23:15:34]

Uncomfortably excited about this. A couple of nits, but the main thing is re:
the multiple RateLimiters

https://codereview.chromium.org/51653008/diff/50001/cc/layers/texture_layer.cc
File cc/layers/texture_layer.cc (right):

https://codereview.chromium.org/51653008/diff/50001/cc/layers/texture_layer.c...
cc/layers/texture_layer.cc:236: DCHECK_EQ(!!texture_id_,
!client_->IsContextLost());
I don't think this DCHECK is valid. For example, the TextureLayerClient may have
had cached the texture id, but doesn't know yet that the context is lost.
An alternative is to change the contract to make the client responsible for
checking, and returning 0 if its context is lost. That's reasonable too, and
would simplify a bit (you wouldn't need IsContextLost() anymore)?

https://codereview.chromium.org/51653008/diff/50001/cc/output/gl_renderer.h
File cc/output/gl_renderer.h (right):

https://codereview.chromium.org/51653008/diff/50001/cc/output/gl_renderer.h#n...
cc/output/gl_renderer.h:30: class ContextProvider;
nit: not used in this file.

https://codereview.chromium.org/51653008/diff/50001/cc/test/test_context_prov...
File cc/test/test_context_provider.cc (right):

https://codereview.chromium.org/51653008/diff/50001/cc/test/test_context_prov...
cc/test/test_context_provider.cc:150: if (context_lost_)
I couldn't locate a place where this is set.

https://codereview.chromium.org/51653008/diff/50001/cc/test/test_web_graphics...
File cc/test/test_web_graphics_context_3d.h (right):

https://codereview.chromium.org/51653008/diff/50001/cc/test/test_web_graphics...
cc/test/test_web_graphics_context_3d.h:187: unsigned
AllocateTestTextureForExternalUse();
leftover/merge conflict?

https://codereview.chromium.org/51653008/diff/50001/cc/trees/layer_tree_host.cc
File cc/trees/layer_tree_host.cc (right):

https://codereview.chromium.org/51653008/diff/50001/cc/trees/layer_tree_host....
cc/trees/layer_tree_host.cc:1119: RateLimiterMap::iterator it =
rate_limiters_.find(client);
Skia only had one context for all canvas, so it had only one RateLimiter, I
think? Now it'd have several, one for each canvas. Does it matter?

https://codereview.chromium.org/51653008/diff/50001/content/renderer/pepper/p...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/51653008/diff/50001/content/renderer/pepper/p...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1906: void
PepperPluginInstanceImpl::RateLimitContext() {}
nit: NOTREACHED()

https://codereview.chromium.org/51653008/diff/50001/ui/compositor/layer.cc
File ui/compositor/layer.cc (right):

https://codereview.chromium.org/51653008/diff/50001/ui/compositor/layer.cc#ne...
ui/compositor/layer.cc:659:
texture_->HostContext3D()->rateLimitOffscreenContextCHROMIUM();
note: we don't use rate limiting for ui::Layer anyway, so you could just make
this NOTREACHED.

[jamesr, 2013-11-02 00:19:30]

On 2013/11/01 23:15:34, piman wrote:
>
https://codereview.chromium.org/51653008/diff/50001/cc/trees/layer_tree_host....
> cc/trees/layer_tree_host.cc:1119: RateLimiterMap::iterator it =
> rate_limiters_.find(client);
> Skia only had one context for all canvas, so it had only one RateLimiter, I
> think? Now it'd have several, one for each canvas. Does it matter?

Good catch.  I think this would cause us to insert too many wait tokens and thus
block prematurely.  I've rewritten the rate limiting here:
https://codereview.chromium.org/53153006 to not require the context.  Will
rebase this patch on top of that.

[jamesr, 2013-11-02 00:44:28]

Cool! This patch is now on top of https://codereview.chromium.org/53153006/ and
is even simpler - there's no need for an IsContextLost() check since the
TLC::PrepareTexture() implementations were zeroing out their textures on lost
context already.  PTAL.

[piman, 2013-11-02 01:21:44]

If we want to be consistent with the previous behavior,
OwnedTexture::PrepareTexture in
content/browser/aura/gpu_process_transport_factory.cc needs to explicitly check
for the lost context. I think that may be needed because
OwnedTexture::OnLostResources is a posted task, and by that time, the compositor
might already have recreated its context, in which case we don't want to commit
an invalid texture.

Other than that, LGTM.

[commit-bot: I haz the power, 2013-11-05 16:12:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/51653008/580001

[commit-bot: I haz the power, 2013-11-05 16:58:52]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
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.

[commit-bot: I haz the power, 2013-11-05 19:41:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/51653008/650003

[commit-bot: I haz the power, 2013-11-05 20:32:26]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
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.

[commit-bot: I haz the power, 2013-11-06 00:41:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/51653008/1070001

[commit-bot: I haz the power, 2013-11-06 04:44:15]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-11-06 04:53:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/51653008/1070001

[commit-bot: I haz the power, 2013-11-06 08:49:37]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-11-06 09:19:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/51653008/1070001

[commit-bot: I haz the power, 2013-11-06 13:48:36]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-11-06 16:55:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/51653008/1070001

[commit-bot: I haz the power, 2013-11-06 19:45:00]

Change committed as 233325