Refactor BrowserViewRenderer-RenderThreadManager relationship.

Refactor BrowserViewRenderer-RenderThreadManager relationship.

Make BrowserViewRenderer a CompositorFrameProducer and
RenderThreadManager a CompositorFrameConsumer, that implicitly
have a 1:many relationship. Each end can notify the other when it
is about to be destroyed, so that the relationship can be cleaned
up appropriately.

Both BVR and RTM must then cope with not having a valid
association with the other.

BUG=597167
Committed: https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b
Cr-Commit-Position: refs/heads/master@{#389147}

[Tobias Sargeant, 2016-04-21 16:31:53]

tobiasjs@chromium.org changed reviewers:
+ boliu@chromium.org

[Tobias Sargeant, 2016-04-21 16:42:53]

Description was changed from

==========
Refactor BrowserViewRenderer-RenderThreadManager relationship.

BUG=597167
==========

to

==========
Refactor BrowserViewRenderer-RenderThreadManager relationship.

Make BrowserViewRenderer a CompositorFrameProducer and RenderThreadManager a
CompositorFrameConsumer, that implicitly have a 1:many relationship. Each end
can notify the other when it is about to be destroyed, so that the relationship
can be cleaned up appropriately.

Both BVR and RTM must then cope with not having a valid association with the
other.

BUG=597167
==========

[Tobias Sargeant, 2016-04-21 16:43:23]

Description was changed from

==========
Refactor BrowserViewRenderer-RenderThreadManager relationship.

Make BrowserViewRenderer a CompositorFrameProducer and RenderThreadManager a
CompositorFrameConsumer, that implicitly have a 1:many relationship. Each end
can notify the other when it is about to be destroyed, so that the relationship
can be cleaned up appropriately.

Both BVR and RTM must then cope with not having a valid association with the
other.

BUG=597167
==========

to

==========
Refactor BrowserViewRenderer-RenderThreadManager relationship.

Make BrowserViewRenderer a CompositorFrameProducer and
RenderThreadManager a CompositorFrameConsumer, that implicitly
have a 1:many relationship. Each end can notify the other when it
is about to be destroyed, so that the relationship can be cleaned
up appropriately.

Both BVR and RTM must then cope with not having a valid
association with the other.

BUG=597167
==========

[boliu, 2016-04-21 17:05:40]

didn't look at tests

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:115:
compositor_frame_consumer_->OnCompositorFrameProducerWillDestroy();
This seems redundant. Just do SetCompositorFrameConsumer(nullptr) here and looks
things will just work? Don't need the WillDestroy on consumer.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:148:
ReleaseHardware(compositor_frame_consumer_);
This code smells bad..

ReleaseHardware "mostly" means set the memory budget to 0. Since the consumer
potentially holds other resources, it's the consumer to ensure those resources
are returned in trim. So ReleaseHardware should happen regardless, and shouldn't
care whether there is a consumer or not.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:203: if
(!compositor_frame_consumer_) {
why did a DCHECK become a null check..? bad code smell again

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:267:
DCHECK(compositor_frame_consumer_ == compositor_frame_consumer);
there is DCHECK_EQ

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:277: compositor_frame_consumer_
= nullptr;
SetCompositorFrameConsumer(nullptr)

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:425: if
(compositor_frame_consumer_) {
ditto

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/browser_view_renderer.h (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.h:12: #include
"android_webview/browser/compositor_frame_consumer.h"
should only need a forward declaration in header

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/compositor_frame_consumer.h (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/compositor_frame_consumer.h:11: #include
"cc/output/compositor_frame_ack.h"
Only need cc/resources/returned_resource.h for cc::ReturnedResourceArray?

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/compositor_frame_producer.h (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/compositor_frame_producer.h:14: virtual void
OnParentDrawConstraintsUpdated(CompositorFrameConsumer*) = 0;
style: params need names

So one producer could have many consumers? Otherwise why have the params here at
all? That's not really reflected in the producer api?

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/render_thread_manager.h (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/render_thread_manager.h:57: 
remove blank line

https://codereview.chromium.org/1911433002/diff/120001/android_webview/native...
File android_webview/native/aw_gl_functor.h (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/native...
android_webview/native/aw_gl_functor.h:11: #include
"android_webview/browser/compositor_frame_producer.h"
don't need producer

https://codereview.chromium.org/1911433002/diff/120001/android_webview/native...
android_webview/native/aw_gl_functor.h:35: return
static_cast<CompositorFrameConsumer*>(&render_thread_manager_);
huh? why does this need to be a cast?

[boliu, 2016-04-21 17:07:43]

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:267:
DCHECK(compositor_frame_consumer_ == compositor_frame_consumer);
On 2016/04/21 17:05:39, boliu wrote:
> there is DCHECK_EQ

If it's supposed to be 1:many, how are these DCHECKs going to hold?

I think what you are going for is 1:1 at any one point in time, but the link can
change at run time.

[Tobias Sargeant, 2016-04-21 17:21:12]

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:115:
compositor_frame_consumer_->OnCompositorFrameProducerWillDestroy();
On 2016/04/21 17:05:39, boliu wrote:
> This seems redundant. Just do SetCompositorFrameConsumer(nullptr) here and
looks
> things will just work? Don't need the WillDestroy on consumer.

OK.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:148:
ReleaseHardware(compositor_frame_consumer_);
On 2016/04/21 17:05:39, boliu wrote:
> This code smells bad..
> 
> ReleaseHardware "mostly" means set the memory budget to 0. Since the consumer
> potentially holds other resources, it's the consumer to ensure those resources
> are returned in trim. So ReleaseHardware should happen regardless, and
shouldn't
> care whether there is a consumer or not.

Yes. The test for whether there is a consumer should be pushed down into
ReleaseHardware.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:203: if
(!compositor_frame_consumer_) {
On 2016/04/21 17:05:39, boliu wrote:
> why did a DCHECK become a null check..? bad code smell again

If RTM is destroyed before BVR, then unless there's something that stops
OnDrawHardware from happening, we will need to handle not having a
compositor_frame_consumer_.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:267:
DCHECK(compositor_frame_consumer_ == compositor_frame_consumer);
On 2016/04/21 17:07:42, boliu wrote:
> On 2016/04/21 17:05:39, boliu wrote:
> > there is DCHECK_EQ
> 
> If it's supposed to be 1:many, how are these DCHECKs going to hold?
> 
> I think what you are going for is 1:1 at any one point in time, but the link
can
> change at run time.

Eventually I was envisaging that we have an active consumer, and a list of all
consumers. That way every consumer on the list can be signalled to release
memory, but only the one that is active receives frames, and that's the only one
from which we care about updated draw constraints.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/compositor_frame_consumer.h (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/compositor_frame_consumer.h:11: #include
"cc/output/compositor_frame_ack.h"
On 2016/04/21 17:05:39, boliu wrote:
> Only need cc/resources/returned_resource.h for cc::ReturnedResourceArray?

Ok. Just moved the include from render_thread_manager.h - but could probably be
more specific.

[boliu, 2016-04-21 17:25:47]

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:203: if
(!compositor_frame_consumer_) {
On 2016/04/21 17:21:12, Tobias Sargeant wrote:
> On 2016/04/21 17:05:39, boliu wrote:
> > why did a DCHECK become a null check..? bad code smell again
> 
> If RTM is destroyed before BVR, then unless there's something that stops
> OnDrawHardware from happening, we will need to handle not having a
> compositor_frame_consumer_.

hmm, should the gurantee should be that BVRClient knows RTM is gone, and stop
calling into OnDrawHardware instead?

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:267:
DCHECK(compositor_frame_consumer_ == compositor_frame_consumer);
On 2016/04/21 17:21:12, Tobias Sargeant wrote:
> On 2016/04/21 17:07:42, boliu wrote:
> > On 2016/04/21 17:05:39, boliu wrote:
> > > there is DCHECK_EQ
> > 
> > If it's supposed to be 1:many, how are these DCHECKs going to hold?
> > 
> > I think what you are going for is 1:1 at any one point in time, but the link
> can
> > change at run time.
> 
> Eventually I was envisaging that we have an active consumer, and a list of all
> consumers. That way every consumer on the list can be signalled to release
> memory, but only the one that is active receives frames, and that's the only
one
> from which we care about updated draw constraints.

That's fine, but you need to make this CL consistent

[Tobias Sargeant, 2016-04-21 17:48:27]

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:115:
compositor_frame_consumer_->OnCompositorFrameProducerWillDestroy();
On 2016/04/21 17:05:39, boliu wrote:
> This seems redundant. Just do SetCompositorFrameConsumer(nullptr) here and
looks
> things will just work? Don't need the WillDestroy on consumer.

Done.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:148:
ReleaseHardware(compositor_frame_consumer_);
On 2016/04/21 17:05:39, boliu wrote:
> This code smells bad..
> 
> ReleaseHardware "mostly" means set the memory budget to 0. Since the consumer
> potentially holds other resources, it's the consumer to ensure those resources
> are returned in trim. So ReleaseHardware should happen regardless, and
shouldn't
> care whether there is a consumer or not.

Done.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:267:
DCHECK(compositor_frame_consumer_ == compositor_frame_consumer);
On 2016/04/21 17:05:39, boliu wrote:
> there is DCHECK_EQ

Done.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:277: compositor_frame_consumer_
= nullptr;
On 2016/04/21 17:05:39, boliu wrote:
> SetCompositorFrameConsumer(nullptr)

Done.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:425: if
(compositor_frame_consumer_) {
On 2016/04/21 17:05:39, boliu wrote:
> ditto

Done.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/browser_view_renderer.h (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.h:12: #include
"android_webview/browser/compositor_frame_consumer.h"
On 2016/04/21 17:05:39, boliu wrote:
> should only need a forward declaration in header

Done.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/compositor_frame_consumer.h (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/compositor_frame_consumer.h:11: #include
"cc/output/compositor_frame_ack.h"
On 2016/04/21 17:05:39, boliu wrote:
> Only need cc/resources/returned_resource.h for cc::ReturnedResourceArray?

Done.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/compositor_frame_producer.h (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/compositor_frame_producer.h:14: virtual void
OnParentDrawConstraintsUpdated(CompositorFrameConsumer*) = 0;
On 2016/04/21 17:05:39, boliu wrote:
> style: params need names
> 
> So one producer could have many consumers? Otherwise why have the params here
at
> all? That's not really reflected in the producer api?

Removed for the moment. Will add back when it's needed.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/compositor_frame_producer.h:14: virtual void
OnParentDrawConstraintsUpdated(CompositorFrameConsumer*) = 0;
On 2016/04/21 17:05:39, boliu wrote:
> style: params need names
> 
> So one producer could have many consumers? Otherwise why have the params here
at
> all? That's not really reflected in the producer api?

Done.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/render_thread_manager.h (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/render_thread_manager.h:57: 
On 2016/04/21 17:05:40, boliu wrote:
> remove blank line

Done.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/native...
File android_webview/native/aw_gl_functor.h (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/native...
android_webview/native/aw_gl_functor.h:11: #include
"android_webview/browser/compositor_frame_producer.h"
On 2016/04/21 17:05:40, boliu wrote:
> don't need producer

Done.

https://codereview.chromium.org/1911433002/diff/120001/android_webview/native...
android_webview/native/aw_gl_functor.h:35: return
static_cast<CompositorFrameConsumer*>(&render_thread_manager_);
On 2016/04/21 17:05:40, boliu wrote:
> huh? why does this need to be a cast?

Doesn't. Removed.

[boliu, 2016-04-21 18:01:33]

looked at diff

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/1911433002/diff/120001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:203: if
(!compositor_frame_consumer_) {
On 2016/04/21 17:25:46, boliu wrote:
> On 2016/04/21 17:21:12, Tobias Sargeant wrote:
> > On 2016/04/21 17:05:39, boliu wrote:
> > > why did a DCHECK become a null check..? bad code smell again
> > 
> > If RTM is destroyed before BVR, then unless there's something that stops
> > OnDrawHardware from happening, we will need to handle not having a
> > compositor_frame_consumer_.
> 
> hmm, should the gurantee should be that BVRClient knows RTM is gone, and stop
> calling into OnDrawHardware instead?

No change?

For now at least, RTM is guaranteed to outlive BVR, so should be fine to turn
this into a DCHECK.

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:445:
DCHECK(compositor_frame_consumer_->ReturnedResourcesEmptyOnUI());
This at the moment still assumes RTM outlives BVR still I think, because
consumer actually have no way to return resources, say if RTM wants to die first
before BVR.

This is fine now, but just something to be aware of later when the assumption is
no longer true.

Another way we talked in the earlier CL to solve this is in
SetCompositorFrameConsumer, make sure all resources are returned from the
consumer that's no longer active. That might be safer thing to do for now. Then
you can write a unit test that RTM is destroyed first, and verify all resources
are still returned.

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
File android_webview/browser/render_thread_manager.h (right):

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
android_webview/browser/render_thread_manager.h:47: void
SetCompositorFrameProducer(CompositorFrameProducer*) override;
param name

[boliu, 2016-04-21 18:14:11]

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
File android_webview/browser/browser_view_renderer_unittest.cc (right):

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
android_webview/browser/browser_view_renderer_unittest.cc:11: #include
"android_webview/browser/render_thread_manager.h"
needed?

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
File android_webview/browser/test/rendering_test.cc (right):

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
android_webview/browser/test/rendering_test.cc:22: class BrowserViewRendererTest
: public BrowserViewRenderer {
TestFoo, not FooTest. FooTest means it's the test itself.

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
android_webview/browser/test/rendering_test.cc:39: RenderingTest*
rendering_test_;
* const

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
android_webview/browser/test/rendering_test.cc:60:
render_thread_manager_.get());
hmm, this totally gives us the test ability to put in a fake consumer

[boliu, 2016-04-21 18:16:39]

yeah, should add a unit test that RTM is destroyed first, and all resources are
returned

https://codereview.chromium.org/1911433002/diff/160001/android_webview/browse...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/1911433002/diff/160001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:124:
ReturnResourceFromParent(compositor_frame_consumer_);
Basicaclly need to do ReleaseHardwareRenderer here..

[Tobias Sargeant, 2016-04-22 11:42:23]

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
File android_webview/browser/browser_view_renderer_unittest.cc (right):

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
android_webview/browser/browser_view_renderer_unittest.cc:11: #include
"android_webview/browser/render_thread_manager.h"
On 2016/04/21 18:14:10, boliu wrote:
> needed?

Not strictly. Can't just remove it as is, but maybe what's in the next PS is
better?

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
File android_webview/browser/render_thread_manager.h (right):

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
android_webview/browser/render_thread_manager.h:47: void
SetCompositorFrameProducer(CompositorFrameProducer*) override;
On 2016/04/21 18:01:32, boliu wrote:
> param name

Done.

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
File android_webview/browser/test/rendering_test.cc (right):

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
android_webview/browser/test/rendering_test.cc:22: class BrowserViewRendererTest
: public BrowserViewRenderer {
On 2016/04/21 18:14:11, boliu wrote:
> TestFoo, not FooTest. FooTest means it's the test itself.

Done.

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
android_webview/browser/test/rendering_test.cc:39: RenderingTest*
rendering_test_;
On 2016/04/21 18:14:11, boliu wrote:
> * const

Done.

https://codereview.chromium.org/1911433002/diff/140001/android_webview/browse...
android_webview/browser/test/rendering_test.cc:60:
render_thread_manager_.get());
On 2016/04/21 18:14:11, boliu wrote:
> hmm, this totally gives us the test ability to put in a fake consumer

Acknowledged.

https://codereview.chromium.org/1911433002/diff/160001/android_webview/browse...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/1911433002/diff/160001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:124:
ReturnResourceFromParent(compositor_frame_consumer_);
On 2016/04/21 18:16:38, boliu wrote:
> Basicaclly need to do ReleaseHardwareRenderer here..

But then we'd flicker, right?

[boliu, 2016-04-22 15:34:37]

lgtm % style nit

https://codereview.chromium.org/1911433002/diff/160001/android_webview/browse...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/1911433002/diff/160001/android_webview/browse...
android_webview/browser/browser_view_renderer.cc:124:
ReturnResourceFromParent(compositor_frame_consumer_);
On 2016/04/22 11:42:22, Tobias Sargeant wrote:
> On 2016/04/21 18:16:38, boliu wrote:
> > Basicaclly need to do ReleaseHardwareRenderer here..
> 
> But then we'd flicker, right?

Yes, but we are not fixing flicker in this CL. Up to you to either
1) Allow resources to be not returned, in which case you need some other way for
unreturned resources to get back to BVR
2) Release everything here, then fix the flicker issue in a follow up CL

Right now this makes no sense..

So I see you chose 2) in your latest PS?

https://codereview.chromium.org/1911433002/diff/180001/android_webview/browse...
File android_webview/browser/test/rendering_test.h (right):

https://codereview.chromium.org/1911433002/diff/180001/android_webview/browse...
android_webview/browser/test/rendering_test.h:77: CompositorFrameConsumer
*GetCompositorFrameConsumer();
style: Foo* Bar(), ie the asterisk before space

[Tobias Sargeant, 2016-04-22 15:46:17]

The CQ bit was checked by tobiasjs@chromium.org

[Tobias Sargeant, 2016-04-22 15:46:17]

The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/1911433002/#ps200001
(title: "nits")

[commit-bot: I haz the power, 2016-04-22 15:46:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1911433002/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1911433002/200001

[commit-bot: I haz the power, 2016-04-22 16:12:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-22 16:13:00]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[Tobias Sargeant, 2016-04-22 16:20:48]

The CQ bit was checked by tobiasjs@chromium.org

[Tobias Sargeant, 2016-04-22 16:20:49]

The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/1911433002/#ps220001
(title: "mark ~RenderThreadManager override")

[commit-bot: I haz the power, 2016-04-22 16:21:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1911433002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1911433002/220001

[commit-bot: I haz the power, 2016-04-22 17:06:09]

Description was changed from

==========
Refactor BrowserViewRenderer-RenderThreadManager relationship.

Make BrowserViewRenderer a CompositorFrameProducer and
RenderThreadManager a CompositorFrameConsumer, that implicitly
have a 1:many relationship. Each end can notify the other when it
is about to be destroyed, so that the relationship can be cleaned
up appropriately.

Both BVR and RTM must then cope with not having a valid
association with the other.

BUG=597167
==========

to

==========
Refactor BrowserViewRenderer-RenderThreadManager relationship.

Make BrowserViewRenderer a CompositorFrameProducer and
RenderThreadManager a CompositorFrameConsumer, that implicitly
have a 1:many relationship. Each end can notify the other when it
is about to be destroyed, so that the relationship can be cleaned
up appropriately.

Both BVR and RTM must then cope with not having a valid
association with the other.

BUG=597167
==========

[commit-bot: I haz the power, 2016-04-22 17:06:10]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2016-04-22 19:49:05]

Description was changed from

==========
Refactor BrowserViewRenderer-RenderThreadManager relationship.

Make BrowserViewRenderer a CompositorFrameProducer and
RenderThreadManager a CompositorFrameConsumer, that implicitly
have a 1:many relationship. Each end can notify the other when it
is about to be destroyed, so that the relationship can be cleaned
up appropriately.

Both BVR and RTM must then cope with not having a valid
association with the other.

BUG=597167
==========

to

==========
Refactor BrowserViewRenderer-RenderThreadManager relationship.

Make BrowserViewRenderer a CompositorFrameProducer and
RenderThreadManager a CompositorFrameConsumer, that implicitly
have a 1:many relationship. Each end can notify the other when it
is about to be destroyed, so that the relationship can be cleaned
up appropriately.

Both BVR and RTM must then cope with not having a valid
association with the other.

BUG=597167

Committed: https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b
Cr-Commit-Position: refs/heads/master@{#389147}
==========

[commit-bot: I haz the power, 2016-04-22 19:49:07]

Patchset 12 (id:??) landed as
https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b
Cr-Commit-Position: refs/heads/master@{#389147}