sync compositor: Add output_surface_id plumbing

The output_surface_id is unique for a particular renderer compositor
instance. It is used to avoid returning resources from lost context to
compositor with new context.

Plumb output_surface_id from SynchronousOutputSurface all the way
to android_webview::HardwareRenderer. To avoid having to send down
a map<id, resource list>, a simpler guarantee is to assume the
output_surface_id is increasing, so only need to return resources
of with highest output surface id and drop resources other resources.

BUG=592744

Review URL: https://codereview.chromium.org/1769913003

Cr-Commit-Position: refs/heads/master@{#382636}

10 files changed, 181 insertions, 37 deletions

diff --git a/android_webview/browser/browser_view_renderer.cc b/android_webview/browser/browser_view_renderer.cc
index 9cc3669..cf6b4b4 100644
--- a/android_webview/browser/browser_view_renderer.cc
+++ b/android_webview/browser/browser_view_renderer.cc
@@ -250,23 +250,24 @@ bool BrowserViewRenderer::OnDrawHardware() {
     viewport_rect_for_tile_priority = last_on_draw_global_visible_rect_;
   }
 
-  scoped_ptr<cc::CompositorFrame> frame =
+  content::SynchronousCompositor::Frame frame =
       compositor_->DemandDrawHw(surface_size,
                                 gfx::Transform(),
                                 viewport,
                                 clip,
                                 viewport_rect_for_tile_priority,
                                 transform_for_tile_priority);
-  if (!frame.get()) {
+  if (!frame.frame.get()) {
     TRACE_EVENT_INSTANT0("android_webview", "NoNewFrame",
                          TRACE_EVENT_SCOPE_THREAD);
     return shared_renderer_state_.HasFrameOnUI();
   }
 
   scoped_ptr<ChildFrame> child_frame = make_scoped_ptr(new ChildFrame(
-      std::move(frame), GetCompositorID(compositor_),
-      viewport_rect_for_tile_priority.IsEmpty(), transform_for_tile_priority,
-      offscreen_pre_raster_, parent_draw_constraints.is_layer));
+      frame.output_surface_id, std::move(frame.frame),
+      GetCompositorID(compositor_), viewport_rect_for_tile_priority.IsEmpty(),
+      transform_for_tile_priority, offscreen_pre_raster_,
+      parent_draw_constraints.is_layer));
 
   ReturnUnusedResource(shared_renderer_state_.PassUncommittedFrameOnUI());
   shared_renderer_state_.SetCompositorFrameOnUI(std::move(child_frame));
@@ -292,21 +293,23 @@ void BrowserViewRenderer::ReturnUnusedResource(
   content::SynchronousCompositor* compositor =
       compositor_map_[child_frame->compositor_id];
   if (compositor && !frame_ack.resources.empty())
-    compositor->ReturnResources(frame_ack);
+    compositor->ReturnResources(child_frame->output_surface_id, frame_ack);
 }
 
 void BrowserViewRenderer::ReturnResourceFromParent() {
-  std::map<uint32_t, cc::ReturnedResourceArray> returned_resource_map;
+  SharedRendererState::ReturnedResourcesMap returned_resource_map;
   shared_renderer_state_.SwapReturnedResourcesOnUI(&returned_resource_map);
   for (auto iterator = returned_resource_map.begin();
        iterator != returned_resource_map.end(); iterator++) {
     uint32_t compositor_id = iterator->first;
     content::SynchronousCompositor* compositor = compositor_map_[compositor_id];
     cc::CompositorFrameAck frame_ack;
-    frame_ack.resources.swap(iterator->second);
+    frame_ack.resources.swap(iterator->second.resources);
 
-    if (compositor && !frame_ack.resources.empty())
-      compositor->ReturnResources(frame_ack);
+    if (compositor && !frame_ack.resources.empty()) {
+      compositor->ReturnResources(iterator->second.output_surface_id,
+                                  frame_ack);
+    }
   }
 }
 
diff --git a/android_webview/browser/browser_view_renderer_unittest.cc b/android_webview/browser/browser_view_renderer_unittest.cc
index 5b2ec24..ffa5742 100644
--- a/android_webview/browser/browser_view_renderer_unittest.cc
+++ b/android_webview/browser/browser_view_renderer_unittest.cc
@@ -2,11 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <map>
+#include <queue>
+#include <utility>
+
 #include "android_webview/browser/browser_view_renderer.h"
 #include "android_webview/browser/child_frame.h"
 #include "android_webview/browser/test/rendering_test.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
+#include "cc/output/compositor_frame.h"
+#include "content/public/test/test_synchronous_compositor_android.h"
 
 namespace android_webview {
 
@@ -156,7 +162,7 @@ class CompositorNoFrameTest : public RenderingTest {
     if (0 == on_draw_count_) {
       // No frame from compositor.
     } else if (1 == on_draw_count_) {
-      SetCompositorFrame();
+      compositor_->SetHardwareFrame(0u, ConstructEmptyFrame());
     } else if (2 == on_draw_count_) {
       // No frame from compositor.
     }
@@ -186,4 +192,95 @@ class CompositorNoFrameTest : public RenderingTest {
 
 RENDERING_TEST_F(CompositorNoFrameTest);
 
+class SwitchOutputSurfaceIdTest : public RenderingTest {
+ public:
+  struct FrameInfo {
+    uint32_t output_surface_id;
+    cc::ResourceId resource_id;  // Each frame contains a single resource.
+  };
+
+  void StartTest() override {
+    last_output_surface_id_ = 0;
+    FrameInfo infos[] = {
+        // First output surface.
+        {0u, 1u}, {0u, 1u}, {0u, 2u}, {0u, 2u}, {0u, 3u}, {0u, 3u}, {0u, 4u},
+        // Second output surface.
+        {1u, 1u}, {1u, 1u}, {1u, 2u}, {1u, 2u}, {1u, 3u}, {1u, 3u}, {1u, 4u},
+    };
+    for (const auto& info : infos) {
+      content::SynchronousCompositor::Frame frame;
+      frame.output_surface_id = info.output_surface_id;
+      frame.frame = ConstructEmptyFrame();
+      cc::TransferableResource resource;
+      resource.id = info.resource_id;
+      frame.frame->delegated_frame_data->resource_list.push_back(resource);
+      frames_.push(std::move(frame));
+
+      // Keep a id -> count map for the last ouptut_surface_id.
+      if (last_output_surface_id_ != info.output_surface_id) {
+        expected_return_count_.clear();
+        last_output_surface_id_ = info.output_surface_id;
+      }
+      if (expected_return_count_.count(info.resource_id)) {
+        expected_return_count_[info.resource_id]++;
+      } else {
+        expected_return_count_[info.resource_id] = 1;
+      }
+    }
+
+    browser_view_renderer_->PostInvalidate();
+  }
+
+  void WillOnDraw() override {
+    if (!frames_.empty()) {
+      compositor_->SetHardwareFrame(frames_.front().output_surface_id,
+                                    std::move(frames_.front().frame));
+    }
+  }
+
+  void DidOnDraw(bool success) override {
+    EXPECT_TRUE(success);
+    if (frames_.empty()) {
+      ui_task_runner_->PostTask(
+          FROM_HERE, base::Bind(&SwitchOutputSurfaceIdTest::CheckResults,
+                                base::Unretained(this)));
+    } else {
+      frames_.pop();
+      browser_view_renderer_->PostInvalidate();
+    }
+  }
+
+  void CheckResults() {
+    window_->Detach();
+    window_.reset();
+
+    // Make sure resources for the last output surface are returned.
+    content::TestSynchronousCompositor::FrameAckArray returned_resources_array;
+    compositor_->SwapReturnedResources(&returned_resources_array);
+    for (const auto& resources : returned_resources_array) {
+      if (resources.output_surface_id != last_output_surface_id_)
+        continue;
+      for (const auto& returned_resource : resources.resources) {
+        EXPECT_TRUE(!!expected_return_count_.count(returned_resource.id));
+        EXPECT_GE(expected_return_count_[returned_resource.id],
+                  returned_resource.count);
+        expected_return_count_[returned_resource.id] -=
+            returned_resource.count;
+        if (!expected_return_count_[returned_resource.id])
+          expected_return_count_.erase(returned_resource.id);
+      }
+    }
+    EXPECT_TRUE(expected_return_count_.empty());
+
+    EndTest();
+  }
+
+ private:
+  std::queue<content::SynchronousCompositor::Frame> frames_;
+  uint32_t last_output_surface_id_;
+  std::map<cc::ResourceId, int> expected_return_count_;
+};
+
+RENDERING_TEST_F(SwitchOutputSurfaceIdTest);
+
 }  // namespace android_webview
diff --git a/android_webview/browser/child_frame.cc b/android_webview/browser/child_frame.cc
index aa6130f..f51b745 100644
--- a/android_webview/browser/child_frame.cc
+++ b/android_webview/browser/child_frame.cc
@@ -10,13 +10,15 @@
 
 namespace android_webview {
 
-ChildFrame::ChildFrame(scoped_ptr<cc::CompositorFrame> frame,
+ChildFrame::ChildFrame(uint32_t output_surface_id,
+                       scoped_ptr<cc::CompositorFrame> frame,
                        uint32_t compositor_id,
                        bool viewport_rect_for_tile_priority_empty,
                        const gfx::Transform& transform_for_tile_priority,
                        bool offscreen_pre_raster,
                        bool is_layer)
-    : frame(std::move(frame)),
+    : output_surface_id(output_surface_id),
+      frame(std::move(frame)),
       compositor_id(compositor_id),
       viewport_rect_for_tile_priority_empty(
           viewport_rect_for_tile_priority_empty),
diff --git a/android_webview/browser/child_frame.h b/android_webview/browser/child_frame.h
index b83e57e..ca42958 100644
--- a/android_webview/browser/child_frame.h
+++ b/android_webview/browser/child_frame.h
@@ -19,7 +19,8 @@ namespace android_webview {
 
 class ChildFrame {
  public:
-  ChildFrame(scoped_ptr<cc::CompositorFrame> frame,
+  ChildFrame(uint32_t output_surface_id,
+             scoped_ptr<cc::CompositorFrame> frame,
              uint32_t compositor_id,
              bool viewport_rect_for_tile_priority_empty,
              const gfx::Transform& transform_for_tile_priority,
@@ -27,6 +28,7 @@ class ChildFrame {
              bool is_layer);
   ~ChildFrame();
 
+  const uint32_t output_surface_id;
   scoped_ptr<cc::CompositorFrame> frame;
   // The id of the compositor this |frame| comes from.
   const uint32_t compositor_id;
diff --git a/android_webview/browser/hardware_renderer.cc b/android_webview/browser/hardware_renderer.cc
index aab7f16..f0a4f85 100644
--- a/android_webview/browser/hardware_renderer.cc
+++ b/android_webview/browser/hardware_renderer.cc
@@ -38,7 +38,9 @@ HardwareRenderer::HardwareRenderer(SharedRendererState* state)
     : shared_renderer_state_(state),
       last_egl_context_(eglGetCurrentContext()),
       gl_surface_(new AwGLSurface),
-      compositor_id_(0),  // Valid compositor id starts at 1.
+      compositor_id_(0u),  // Valid compositor id starts at 1.
+      last_committed_output_surface_id_(0u),
+      last_submitted_output_surface_id_(0u),
       output_surface_(NULL) {
   DCHECK(last_egl_context_);
 
@@ -86,6 +88,7 @@ void HardwareRenderer::CommitFrame() {
   if (!child_frame.get())
     return;
 
+  last_committed_output_surface_id_ = child_frame->output_surface_id;
   ReturnResourcesInChildFrame();
   child_frame_ = std::move(child_frame);
   DCHECK(child_frame_->frame.get());
@@ -110,7 +113,8 @@ void HardwareRenderer::DrawGL(AwDrawGLInfo* draw_info,
   // kModeProcess. Instead, submit the frame in "kModeDraw" stage to avoid
   // unnecessary kModeProcess.
   if (child_frame_.get() && child_frame_->frame.get()) {
-    if (compositor_id_ != child_frame_->compositor_id) {
+    if (compositor_id_ != child_frame_->compositor_id ||
+        last_submitted_output_surface_id_ != child_frame_->output_surface_id) {
       if (!root_id_.is_null())
         surface_factory_->Destroy(root_id_);
       if (!child_id_.is_null())
@@ -122,6 +126,7 @@ void HardwareRenderer::DrawGL(AwDrawGLInfo* draw_info,
       // This will return all the resources to the previous compositor.
       surface_factory_.reset();
       compositor_id_ = child_frame_->compositor_id;
+      last_submitted_output_surface_id_ = child_frame_->output_surface_id;
       surface_factory_.reset(
           new cc::SurfaceFactory(surface_manager_.get(), this));
     }
@@ -223,7 +228,8 @@ void HardwareRenderer::DrawGL(AwDrawGLInfo* draw_info,
 
 void HardwareRenderer::ReturnResources(
     const cc::ReturnedResourceArray& resources) {
-  ReturnResourcesToCompositor(resources, compositor_id_);
+  ReturnResourcesToCompositor(resources, compositor_id_,
+                              last_submitted_output_surface_id_);
 }
 
 void HardwareRenderer::SetBeginFrameSource(
@@ -246,15 +252,20 @@ void HardwareRenderer::ReturnResourcesInChildFrame() {
     // The child frame's compositor id is not necessarily same as
     // compositor_id_.
     ReturnResourcesToCompositor(resources_to_return,
-                                child_frame_->compositor_id);
+                                child_frame_->compositor_id,
+                                child_frame_->output_surface_id);
   }
   child_frame_.reset();
 }
 
 void HardwareRenderer::ReturnResourcesToCompositor(
     const cc::ReturnedResourceArray& resources,
-    uint32_t compositor_id) {
-  shared_renderer_state_->InsertReturnedResourcesOnRT(resources, compositor_id);
+    uint32_t compositor_id,
+    uint32_t output_surface_id) {
+  if (output_surface_id != last_committed_output_surface_id_)
+    return;
+  shared_renderer_state_->InsertReturnedResourcesOnRT(resources, compositor_id,
+                                                      output_surface_id);
 }
 
 }  // namespace android_webview
diff --git a/android_webview/browser/hardware_renderer.h b/android_webview/browser/hardware_renderer.h
index fe842f4..1b5127a 100644
--- a/android_webview/browser/hardware_renderer.h
+++ b/android_webview/browser/hardware_renderer.h
@@ -52,7 +52,8 @@ class HardwareRenderer : public cc::DisplayClient,
 
   void ReturnResourcesInChildFrame();
   void ReturnResourcesToCompositor(const cc::ReturnedResourceArray& resources,
-                                   uint32_t compositor_routing_id);
+                                   uint32_t compositor_routing_id,
+                                   uint32_t output_surface_id);
 
   SharedRendererState* shared_renderer_state_;
 
@@ -79,6 +80,10 @@ class HardwareRenderer : public cc::DisplayClient,
   cc::SurfaceId child_id_;
   cc::SurfaceId root_id_;
   uint32_t compositor_id_;
+  // HardwareRenderer guarantees resources are returned in the order of
+  // output_surface_id, and resources for old output surfaces are dropped.
+  uint32_t last_committed_output_surface_id_;
+  uint32_t last_submitted_output_surface_id_;
 
   // This is owned by |display_|.
   ParentOutputSurface* output_surface_;
diff --git a/android_webview/browser/shared_renderer_state.cc b/android_webview/browser/shared_renderer_state.cc
index e81ab126..488f915 100644
--- a/android_webview/browser/shared_renderer_state.cc
+++ b/android_webview/browser/shared_renderer_state.cc
@@ -213,18 +213,28 @@ bool SharedRendererState::IsInsideHardwareRelease() const {
   return inside_hardware_release_;
 }
 
+SharedRendererState::ReturnedResources::ReturnedResources()
+    : output_surface_id(0u) {}
+
+SharedRendererState::ReturnedResources::~ReturnedResources() {}
+
 void SharedRendererState::InsertReturnedResourcesOnRT(
     const cc::ReturnedResourceArray& resources,
-    uint32_t compositor_id) {
+    uint32_t compositor_id,
+    uint32_t output_surface_id) {
   base::AutoLock lock(lock_);
-  cc::ReturnedResourceArray& returned_resources =
+  ReturnedResources& returned_resources =
       returned_resources_map_[compositor_id];
-  returned_resources.insert(returned_resources.end(), resources.begin(),
-                            resources.end());
+  if (returned_resources.output_surface_id != output_surface_id) {
+    returned_resources.resources.clear();
+  }
+  returned_resources.resources.insert(returned_resources.resources.end(),
+                                      resources.begin(), resources.end());
+  returned_resources.output_surface_id = output_surface_id;
 }
 
 void SharedRendererState::SwapReturnedResourcesOnUI(
-    std::map<uint32_t, cc::ReturnedResourceArray>* returned_resource_map) {
+    ReturnedResourcesMap* returned_resource_map) {
   DCHECK(returned_resource_map->empty());
   base::AutoLock lock(lock_);
   returned_resource_map->swap(returned_resources_map_);
diff --git a/android_webview/browser/shared_renderer_state.h b/android_webview/browser/shared_renderer_state.h
index a6f3e0b..a2313a5 100644
--- a/android_webview/browser/shared_renderer_state.h
+++ b/android_webview/browser/shared_renderer_state.h
@@ -34,6 +34,15 @@ class InsideHardwareReleaseReset;
 // This class is used to pass data between UI thread and RenderThread.
 class SharedRendererState {
  public:
+  struct ReturnedResources {
+    ReturnedResources();
+    ~ReturnedResources();
+
+    uint32_t output_surface_id;
+    cc::ReturnedResourceArray resources;
+  };
+  using ReturnedResourcesMap = std::map<uint32_t, ReturnedResources>;
+
   SharedRendererState(
       const scoped_refptr<base::SingleThreadTaskRunner>& ui_loop,
       BrowserViewRenderer* browser_view_renderer);
@@ -48,8 +57,7 @@ class SharedRendererState {
   void InitializeHardwareDrawIfNeededOnUI();
   void ReleaseHardwareDrawIfNeededOnUI();
   ParentCompositorDrawConstraints GetParentDrawConstraintsOnUI() const;
-  void SwapReturnedResourcesOnUI(
-      std::map<uint32_t, cc::ReturnedResourceArray>* returned_resource_map);
+  void SwapReturnedResourcesOnUI(ReturnedResourcesMap* returned_resource_map);
   bool ReturnedResourcesEmptyOnUI() const;
   scoped_ptr<ChildFrame> PassUncommittedFrameOnUI();
   void DeleteHardwareRendererOnUI();
@@ -62,7 +70,8 @@ class SharedRendererState {
   void PostExternalDrawConstraintsToChildCompositorOnRT(
       const ParentCompositorDrawConstraints& parent_draw_constraints);
   void InsertReturnedResourcesOnRT(const cc::ReturnedResourceArray& resources,
-                                   uint32_t compositor_id);
+                                   uint32_t compositor_id,
+                                   uint32_t output_surface_id);
 
  private:
   friend class internal::RequestDrawGLTracker;
@@ -106,8 +115,7 @@ class SharedRendererState {
   scoped_ptr<ChildFrame> child_frame_;
   bool inside_hardware_release_;
   ParentCompositorDrawConstraints parent_draw_constraints_;
-  // A map from compositor's ID to the resources that belong to the compositor.
-  std::map<uint32_t, cc::ReturnedResourceArray> returned_resources_map_;
+  ReturnedResourcesMap returned_resources_map_;
   base::Closure request_draw_gl_closure_;
 
   base::WeakPtrFactory<SharedRendererState> weak_factory_on_ui_thread_;
diff --git a/android_webview/browser/test/rendering_test.cc b/android_webview/browser/test/rendering_test.cc
index 4f00a20..e51f3aa 100644
--- a/android_webview/browser/test/rendering_test.cc
+++ b/android_webview/browser/test/rendering_test.cc
@@ -67,8 +67,7 @@ void RenderingTest::QuitMessageLoop() {
   message_loop_->QuitWhenIdle();
 }
 
-void RenderingTest::SetCompositorFrame() {
-  DCHECK(compositor_.get());
+scoped_ptr<cc::CompositorFrame> RenderingTest::ConstructEmptyFrame() {
   scoped_ptr<cc::CompositorFrame> compositor_frame(new cc::CompositorFrame);
   scoped_ptr<cc::DelegatedFrameData> frame(new cc::DelegatedFrameData);
   scoped_ptr<cc::RenderPass> root_pass(cc::RenderPass::Create());
@@ -77,11 +76,12 @@ void RenderingTest::SetCompositorFrame() {
                     gfx::Transform());
   frame->render_pass_list.push_back(std::move(root_pass));
   compositor_frame->delegated_frame_data = std::move(frame);
-  compositor_->SetHardwareFrame(std::move(compositor_frame));
+  return compositor_frame;
 }
 
 void RenderingTest::WillOnDraw() {
-  SetCompositorFrame();
+  DCHECK(compositor_);
+  compositor_->SetHardwareFrame(0u, ConstructEmptyFrame());
 }
 
 bool RenderingTest::RequestDrawGL(bool wait_for_completion) {
@@ -103,7 +103,8 @@ void RenderingTest::OnNewPicture() {
 }
 
 void RenderingTest::PostInvalidate() {
-  window_->PostInvalidate();
+  if (window_)
+    window_->PostInvalidate();
 }
 
 void RenderingTest::DetachFunctorFromView() {
diff --git a/android_webview/browser/test/rendering_test.h b/android_webview/browser/test/rendering_test.h
index 0e6fcf1..8dd6cae 100644
--- a/android_webview/browser/test/rendering_test.h
+++ b/android_webview/browser/test/rendering_test.h
@@ -16,6 +16,10 @@ namespace base {
 class MessageLoop;
 }
 
+namespace cc {
+class CompositorFrame;
+}
+
 namespace content {
 class TestSynchronousCompositor;
 }
@@ -58,6 +62,7 @@ class RenderingTest : public testing::Test,
   void DidDrawOnRT(SharedRendererState* functor) override {}
 
  protected:
+
   RenderingTest();
   ~RenderingTest() override;
 
@@ -68,7 +73,7 @@ class RenderingTest : public testing::Test,
   void InitializeCompositor();
   void Attach();
   void EndTest();
-  void SetCompositorFrame();
+  scoped_ptr<cc::CompositorFrame> ConstructEmptyFrame();
 
   scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
   scoped_ptr<BrowserViewRenderer> browser_view_renderer_;