WIP Handle AwContents needing multiple live functors.

android_webview/browser/browser_view_renderer.cc

Index: android_webview/browser/browser_view_renderer.cc
diff --git a/android_webview/browser/browser_view_renderer.cc b/android_webview/browser/browser_view_renderer.cc
index c53081505d10fae594486f9336a97243de958d51..acef03534ac7d5bcd57d671311cbdb614c57409c 100644
--- a/android_webview/browser/browser_view_renderer.cc
+++ b/android_webview/browser/browser_view_renderer.cc
@@ -92,7 +92,7 @@ BrowserViewRenderer::BrowserViewRenderer(
     const scoped_refptr<base::SingleThreadTaskRunner>& ui_task_runner)
     : client_(client),
       ui_task_runner_(ui_task_runner),
-      compositor_frame_consumer_(nullptr),
+      current_compositor_frame_consumer_(nullptr),
       compositor_(NULL),
       is_paused_(false),
       view_visible_(false),
@@ -110,24 +110,28 @@ BrowserViewRenderer::BrowserViewRenderer(
 
 BrowserViewRenderer::~BrowserViewRenderer() {
   DCHECK(compositor_map_.empty());
-  SetCompositorFrameConsumer(nullptr);
+  while (compositor_frame_consumers_.size()) {
+    RemoveCompositorFrameConsumer(*compositor_frame_consumers_.begin());
+  }
 }
 
-void BrowserViewRenderer::SetCompositorFrameConsumer(
+void BrowserViewRenderer::SetCurrentCompositorFrameConsumer(
     CompositorFrameConsumer* compositor_frame_consumer) {
-  if (compositor_frame_consumer == compositor_frame_consumer_) {
+  if (compositor_frame_consumer == current_compositor_frame_consumer_) {
     return;
   }
-  if (compositor_frame_consumer_) {
-    compositor_frame_consumer_->DeleteHardwareRendererOnUI();
-    ReturnUnusedResource(
-        compositor_frame_consumer_->PassUncommittedFrameOnUI());
-    ReturnResourceFromParent(compositor_frame_consumer_);
-    compositor_frame_consumer_->SetCompositorFrameProducer(nullptr);
-  }
-  compositor_frame_consumer_ = compositor_frame_consumer;
-  if (compositor_frame_consumer_) {
-    compositor_frame_consumer_->SetCompositorFrameProducer(this);
+  current_compositor_frame_consumer_ = compositor_frame_consumer;
+  if (current_compositor_frame_consumer_) {
+    compositor_frame_consumers_.insert(current_compositor_frame_consumer_);
+    if (current_compositor_frame_consumer_->GetCompositorFrameProducer() !=
+        this) {
+      if (current_compositor_frame_consumer_->GetCompositorFrameProducer()) {

[boliu, 2016/05/05 14:09:34]

this can't happen? we destroy the previous BVR before the new BVR is attached in
the pop up flow. just DCHECK somewhere that this can't happen

[Tobias Sargeant, 2016/05/05 19:06:46]

You're right. I've cleaned this up.

+        current_compositor_frame_consumer_->GetCompositorFrameProducer()
+            ->RemoveCompositorFrameConsumer(current_compositor_frame_consumer_);
+      }
+      current_compositor_frame_consumer_->SetCompositorFrameProducer(this);
+    }
+    OnParentDrawConstraintsUpdated(current_compositor_frame_consumer_);
   }
 }
 
@@ -200,22 +204,23 @@ bool BrowserViewRenderer::CanOnDraw() {
 }
 
 bool BrowserViewRenderer::OnDrawHardware() {
-  DCHECK(compositor_frame_consumer_);
+  DCHECK(current_compositor_frame_consumer_);
   TRACE_EVENT0("android_webview", "BrowserViewRenderer::OnDrawHardware");
 
-  compositor_frame_consumer_->InitializeHardwareDrawIfNeededOnUI();
+  current_compositor_frame_consumer_->InitializeHardwareDrawIfNeededOnUI();
 
   if (!CanOnDraw()) {
     return false;
   }
 
-  compositor_frame_consumer_->SetScrollOffsetOnUI(last_on_draw_scroll_offset_);
+  current_compositor_frame_consumer_->SetScrollOffsetOnUI(
+      last_on_draw_scroll_offset_);
   hardware_enabled_ = true;
 
   external_draw_constraints_ =
-      compositor_frame_consumer_->GetParentDrawConstraintsOnUI();
+      current_compositor_frame_consumer_->GetParentDrawConstraintsOnUI();
 
-  ReturnResourceFromParent(compositor_frame_consumer_);
+  ReturnResourceFromParent(current_compositor_frame_consumer_);
   UpdateMemoryPolicy();
 
   gfx::Size surface_size(size_);
@@ -244,7 +249,7 @@ bool BrowserViewRenderer::OnDrawHardware() {
   if (!frame.frame.get()) {
     TRACE_EVENT_INSTANT0("android_webview", "NoNewFrame",
                          TRACE_EVENT_SCOPE_THREAD);
-    hardware_enabled_ = compositor_frame_consumer_->HasFrameOnUI();
+    hardware_enabled_ = current_compositor_frame_consumer_->HasFrameOnUI();
     if (!hardware_enabled_)
       UpdateMemoryPolicy();
     return hardware_enabled_;
@@ -256,22 +261,37 @@ bool BrowserViewRenderer::OnDrawHardware() {
       transform_for_tile_priority, offscreen_pre_raster_,
       external_draw_constraints_.is_layer));
 
-  ReturnUnusedResource(compositor_frame_consumer_->PassUncommittedFrameOnUI());
-  compositor_frame_consumer_->SetFrameOnUI(std::move(child_frame));
+  ReturnUnusedResource(
+      current_compositor_frame_consumer_->PassUncommittedFrameOnUI());
+  current_compositor_frame_consumer_->SetFrameOnUI(std::move(child_frame));
   return true;
 }
 
-void BrowserViewRenderer::OnParentDrawConstraintsUpdated() {
-  DCHECK(compositor_frame_consumer_);
-  PostInvalidate();
-  external_draw_constraints_ =
-      compositor_frame_consumer_->GetParentDrawConstraintsOnUI();
-  UpdateMemoryPolicy();
+void BrowserViewRenderer::OnParentDrawConstraintsUpdated(
+    CompositorFrameConsumer* compositor_frame_consumer) {
+  DCHECK(compositor_frame_consumer);

[boliu, 2016/05/05 14:09:34]

DCHECK compositor frame is in set

[Tobias Sargeant, 2016/05/05 19:06:45]

Don't need to, because being equal to the current consumer implies that.

[boliu, 2016/05/06 15:53:13]

Well, you are DCHECK-ing that guarantee.

+  if (compositor_frame_consumer == current_compositor_frame_consumer_) {

[boliu, 2016/05/05 14:09:34]

nit: we usually prefer

if (foo)
  return;

since that avoids indenting the whole method

[Tobias Sargeant, 2016/05/05 19:06:46]

Done.

+    PostInvalidate();
+    external_draw_constraints_ =
+        current_compositor_frame_consumer_->GetParentDrawConstraintsOnUI();
+    UpdateMemoryPolicy();
+  }
 }
 
-void BrowserViewRenderer::OnCompositorFrameConsumerWillDestroy() {
-  DCHECK(compositor_frame_consumer_);
-  SetCompositorFrameConsumer(nullptr);
+void BrowserViewRenderer::RemoveCompositorFrameConsumer(
+    CompositorFrameConsumer* compositor_frame_consumer) {
+  DCHECK(compositor_frame_consumers_.count(compositor_frame_consumer));
+  compositor_frame_consumers_.erase(compositor_frame_consumer);
+  if (current_compositor_frame_consumer_ == compositor_frame_consumer) {
+    SetCurrentCompositorFrameConsumer(nullptr);
+  }
+
+  // At this point the compositor frame consumer has to hand back all resources
+  // to the child compositor.
+  compositor_frame_consumer->DeleteHardwareRendererOnUI();
+  ReturnUnusedResource(compositor_frame_consumer->PassUncommittedFrameOnUI());
+  ReturnResourceFromParent(compositor_frame_consumer);
+  compositor_frame_consumer->SetCompositorFrameProducer(nullptr);
 }
 
 void BrowserViewRenderer::ReturnUnusedResource(
@@ -438,7 +458,7 @@ void BrowserViewRenderer::OnComputeScroll(base::TimeTicks animation_time) {
 }
 
 void BrowserViewRenderer::ReleaseHardware() {
-  if (compositor_frame_consumer_) {
+  for (auto compositor_frame_consumer_ : compositor_frame_consumers_) {
     ReturnUnusedResource(
         compositor_frame_consumer_->PassUncommittedFrameOnUI());
     ReturnResourceFromParent(compositor_frame_consumer_);