Fix scrollbars missing after lost context

Fix scrollbars missing after lost context

We can't skip updates if we're simply !dirty, because we may have lost the
textures. updatePart already takes both into account.

BUG=None
TEST=ScrollbarLayerLostContext


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

[piman, 2013-01-18 04:48:54]

I thought this might be related to the scrollbar issues with ubercomp, but it's
not. Still this is worth fixing.

Some of the churn is just to fix FakeWebGraphicsContext3D::loseContextCHROMIUM
to match the interface in WebGraphicsContext3D, to avoid unnecessary casts.

[danakj, 2013-01-18 05:07:49]

LGTM tho I'd prefer the test with the other lost context tests, where there's
helper functions for stuff being recreated in the test now (then we don't have
to re-code-style it later too).

https://codereview.chromium.org/11941010/diff/1/cc/scrollbar_layer_unittest.cc
File cc/scrollbar_layer_unittest.cc (right):

https://codereview.chromium.org/11941010/diff/1/cc/scrollbar_layer_unittest.c...
cc/scrollbar_layer_unittest.cc:199: class ScrollbarLayerLostContext : public
ThreadedTest {
Can you stick this in layer_tree_host_unittest_context.cc with the other
lost-context tests?

then you can reuse some machinery there like LoseContext() and
createOutputSurface()

https://codereview.chromium.org/11941010/diff/1/cc/scrollbar_layer_unittest.c...
cc/scrollbar_layer_unittest.cc:205:
m_layerTreeHost->initializeRendererIfNeeded();
this should be done by base classes already, shouldn't need it.

[piman, 2013-01-18 05:37:09]

On Thu, Jan 17, 2013 at 9:07 PM, <danakj@chromium.org> wrote:

> LGTM tho I'd prefer the test with the other lost context tests, where
> there's
> helper functions for stuff being recreated in the test now (then we don't
> have
> to re-code-style it later too).
>

Heh, I went out of my way to move it to scrollbar_layer_unittest since
that's a test very specific to the scrollbar (and not so much the LTH
infrastructure). But if you prefer it there, sure.



>
>
> https://codereview.chromium.**org/11941010/diff/1/cc/**
>
scrollbar_layer_unittest.cc<https://codereview.chromium.org/11941010/diff/1/cc/scrollbar_layer_unittest.cc>
> File cc/scrollbar_layer_unittest.cc (right):
>
> https://codereview.chromium.**org/11941010/diff/1/cc/**
>
scrollbar_layer_unittest.cc#**newcode199<https://codereview.chromium.org/11941010/diff/1/cc/scrollbar_layer_unittest.cc#newcode199>
> cc/scrollbar_layer_unittest.**cc:199: class ScrollbarLayerLostContext :
> public ThreadedTest {
> Can you stick this in layer_tree_host_unittest_**context.cc with the other
> lost-context tests?
>
> then you can reuse some machinery there like LoseContext() and
> createOutputSurface()
>
> https://codereview.chromium.**org/11941010/diff/1/cc/**
>
scrollbar_layer_unittest.cc#**newcode205<https://codereview.chromium.org/11941010/diff/1/cc/scrollbar_layer_unittest.cc#newcode205>
> cc/scrollbar_layer_unittest.**cc:205:
> m_layerTreeHost->**initializeRendererIfNeeded();
> this should be done by base classes already, shouldn't need it.
>
>
https://codereview.chromium.**org/11941010/<https://codereview.chromium.org/1...
>

[danakj, 2013-01-18 06:47:08]

On 2013/01/18 05:37:09, piman wrote:
> On Thu, Jan 17, 2013 at 9:07 PM, <mailto:danakj@chromium.org> wrote:
> 
> > LGTM tho I'd prefer the test with the other lost context tests, where
> > there's
> > helper functions for stuff being recreated in the test now (then we don't
> > have
> > to re-code-style it later too).
> >
> 
> Heh, I went out of my way to move it to scrollbar_layer_unittest since
> that's a test very specific to the scrollbar (and not so much the LTH
> infrastructure). But if you prefer it there, sure.

My plan, at least, has been that
1) foo_unittest.cc would hold small unit tests that more-or-less instantiate and
directly test the methods of Foo.
2) layer_tree_host_unittest_foo.cc would hold integration tests (ie
ThreadedTests, previously mostly LayerTreeHostTests) that test the compositor's
behaviour with respect to feature or area Foo.

And that hopefully we can make nice test helper methods/base class test cases in
2) that can apply to many tests in the suite to reduce LOC needed to add new
integration tests to them to a minimum.

Maybe the layer_tree_host_unittest_ prefix isn't super helpful to explaining
that, and it is only historical in nature..

[piman, 2013-01-18 23:10:08]

https://codereview.chromium.org/11941010/diff/1/cc/scrollbar_layer_unittest.cc
File cc/scrollbar_layer_unittest.cc (right):

https://codereview.chromium.org/11941010/diff/1/cc/scrollbar_layer_unittest.c...
cc/scrollbar_layer_unittest.cc:199: class ScrollbarLayerLostContext : public
ThreadedTest {
On 2013/01/18 05:07:49, danakj wrote:
> Can you stick this in layer_tree_host_unittest_context.cc with the other
> lost-context tests?
> 
> then you can reuse some machinery there like LoseContext() and
> createOutputSurface()

Done.

https://codereview.chromium.org/11941010/diff/1/cc/scrollbar_layer_unittest.c...
cc/scrollbar_layer_unittest.cc:205:
m_layerTreeHost->initializeRendererIfNeeded();
On 2013/01/18 05:07:49, danakj wrote:
> this should be done by base classes already, shouldn't need it.

Done.

[danakj, 2013-01-18 23:16:20]

LGTM w/ style nits

https://codereview.chromium.org/11941010/diff/1009/cc/layer_tree_host_unittes...
File cc/layer_tree_host_unittest_context.cc (right):

https://codereview.chromium.org/11941010/diff/1009/cc/layer_tree_host_unittes...
cc/layer_tree_host_unittest_context.cc:869: public:
nit: 1 space before

https://codereview.chromium.org/11941010/diff/1009/cc/layer_tree_host_unittes...
cc/layer_tree_host_unittest_context.cc:873: scoped_refptr<Layer> scroll_layer =
Layer::create();
nit: 2space indents

[piman, 2013-01-18 23:29:14]

https://codereview.chromium.org/11941010/diff/1009/cc/layer_tree_host_unittes...
File cc/layer_tree_host_unittest_context.cc (right):

https://codereview.chromium.org/11941010/diff/1009/cc/layer_tree_host_unittes...
cc/layer_tree_host_unittest_context.cc:869: public:
On 2013/01/18 23:16:20, danakj wrote:
> nit: 1 space before

Done.

https://codereview.chromium.org/11941010/diff/1009/cc/layer_tree_host_unittes...
cc/layer_tree_host_unittest_context.cc:873: scoped_refptr<Layer> scroll_layer =
Layer::create();
On 2013/01/18 23:16:20, danakj wrote:
> nit: 2space indents

Done.

[commit-bot: I haz the power, 2013-01-18 23:29:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/11941010/9001

[danakj, 2013-01-18 23:30:14]

Thanks :) Sorry for making you move code around.. LGTM++

[commit-bot: I haz the power, 2013-01-18 23:55:12]

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

[danakj, 2013-01-18 23:57:11]

HEAD is junk

[commit-bot: I haz the power, 2013-01-18 23:58:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/11941010/9001

[commit-bot: I haz the power, 2013-01-19 00:14:02]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
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-01-19 00:25:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/11941010/13002

[commit-bot: I haz the power, 2013-01-19 04:16:14]

Change committed as 177829