Surface Sychronization: Clean-up DelegatedFrameHostAndroid

This CL does two things:

1. It moves ParentLocalSurfaceIdAllocator from DelegatedFrameHostAndroid
to RenderWidgetHostViewAndroid.

2. It makes DelegatedFrameHostAndroid::SynchronizeVisualProperties
   and DelegatedFrameHostAndroid::OnFirstSurfaceActivation better
   match their DelegatedFrameHost counterparts.

3. Creating and destroying SurfaceLayers is a relatively heavyweight
   operation. This CL avoids this when surface sync is on.

Change-Id: Ia265489cc95494eb0ae0cf6ab9d5a300a79a53e3
Bug: 672962
Reviewed-on: https://chromium-review.googlesource.com/1067954Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562737}

content/browser/renderer_host/render_widget_host_view_android.cc

@@ -264,8 +264,10 @@ void RenderWidgetHostViewAndroid::InitAsFullscreen(
 
 bool RenderWidgetHostViewAndroid::SynchronizeVisualProperties() {
   if (delegated_frame_host_) {
-    delegated_frame_host_->SynchronizeVisualProperties(
-        GetCompositorViewportPixelSize());
+    delegated_frame_host_->EmbedSurface(
+        local_surface_id_allocator_.GenerateId(),
+        GetCompositorViewportPixelSize(),
+        cc::DeadlinePolicy::UseDefaultDeadline());
 
     // TODO(ericrk): This can be removed once surface synchronization is
     // enabled. https://crbug.com/835102
@@ -962,7 +964,7 @@ void RenderWidgetHostViewAndroid::SubmitCompositorFrame(
 
   viz::BeginFrameAck ack = frame.metadata.begin_frame_ack;
   if (!has_content) {
-    DestroyDelegatedContent();
+    EvictDelegatedFrame();
 
     ack.has_damage = false;
     OnDidNotProduceFrame(ack);
@@ -981,14 +983,14 @@ void RenderWidgetHostViewAndroid::SubmitCompositorFrame(
   OnFrameMetadataUpdated(std::move(metadata), is_transparent);
 }
 
-void RenderWidgetHostViewAndroid::DestroyDelegatedContent() {
+void RenderWidgetHostViewAndroid::EvictDelegatedFrame() {
   if (!delegated_frame_host_)
     return;
 
   DCHECK(delegated_frame_host_->HasDelegatedContent() ==
          frame_evictor_->HasFrame());
 
-  delegated_frame_host_->DestroyDelegatedContent();
+  delegated_frame_host_->EvictDelegatedFrame();
 
   if (frame_evictor_->HasFrame())
     frame_evictor_->DiscardedFrame();
@@ -1017,7 +1019,7 @@ void RenderWidgetHostViewAndroid::AcknowledgeBeginFrame(
 }
 
 void RenderWidgetHostViewAndroid::ClearCompositorFrame() {
-  DestroyDelegatedContent();
+  EvictDelegatedFrame();
 }
 
 bool RenderWidgetHostViewAndroid::RequestRepaintForTesting() {
@@ -1328,7 +1330,7 @@ bool RenderWidgetHostViewAndroid::UpdateControls(
 
 void RenderWidgetHostViewAndroid::OnDidUpdateVisualPropertiesComplete(
     const cc::RenderFrameMetadata& metadata) {
-  if (delegated_frame_host_->GetLocalSurfaceIdAllocator()->UpdateFromChild(
+  if (local_surface_id_allocator_.UpdateFromChild(
           metadata.local_surface_id.value_or(viz::LocalSurfaceId()))) {
     // A synchronization event was initiated by the renderer so let's updated
     // the top/bottom bar controls now.
@@ -1336,6 +1338,12 @@ void RenderWidgetHostViewAndroid::OnDidUpdateVisualPropertiesComplete(
                    metadata.top_controls_shown_ratio,
                    metadata.bottom_controls_height,
                    metadata.bottom_controls_shown_ratio);
+    if (delegated_frame_host_) {
+      delegated_frame_host_->EmbedSurface(
+          local_surface_id_allocator_.GetCurrentLocalSurfaceId(),
+          GetCompositorViewportPixelSize(),
+          cc::DeadlinePolicy::UseDefaultDeadline());
+    }
   }
   host()->SynchronizeVisualProperties();
 }
@@ -1515,10 +1523,6 @@ void RenderWidgetHostViewAndroid::RequestDisallowInterceptTouchEvent() {
     view_.RequestDisallowInterceptTouchEvent();
 }
 
-void RenderWidgetHostViewAndroid::EvictDelegatedFrame() {
-  DestroyDelegatedContent();
-}
-
 gfx::Vector2d RenderWidgetHostViewAndroid::GetOffsetFromRootSurface() {
   return gfx::Vector2d(
       0, DoBrowserControlsShrinkBlinkSize() ? GetTopControlsHeight() : 0);
@@ -1988,9 +1992,9 @@ RenderWidgetHostViewAndroid::GetTouchSelectionControllerClientManager() {
 }
 
 viz::LocalSurfaceId RenderWidgetHostViewAndroid::GetLocalSurfaceId() const {
-  if (delegated_frame_host_)
-    return delegated_frame_host_->GetLocalSurfaceId();
-  return viz::LocalSurfaceId();
+  if (!delegated_frame_host_)
+    return viz::LocalSurfaceId();
+  return local_surface_id_allocator_.GetCurrentLocalSurfaceId();
 }
 
 void RenderWidgetHostViewAndroid::OnRenderWidgetInit() {
@@ -2189,7 +2193,7 @@ void RenderWidgetHostViewAndroid::OnActivityStarted() {
 }
 
 void RenderWidgetHostViewAndroid::OnLostResources() {
-  DestroyDelegatedContent();
+  EvictDelegatedFrame();
   DCHECK(ack_callbacks_.empty());
 }
 
@@ -2334,10 +2338,13 @@ base::Optional<SkColor> RenderWidgetHostViewAndroid::GetBackgroundColor()
 }
 
 void RenderWidgetHostViewAndroid::DidNavigate() {
-  RenderWidgetHostViewBase::DidNavigate();
+  if (!delegated_frame_host_) {
+    RenderWidgetHostViewBase::DidNavigate();
+    return;
+  }
 
-  if (delegated_frame_host_)
-    delegated_frame_host_->DidNavigate();
+  SynchronizeVisualProperties();
+  delegated_frame_host_->DidNavigate();
 }
 
 viz::ScopedSurfaceIdAllocator

content/browser/renderer_host/render_widget_host_view_android.h

@@ -22,6 +22,7 @@
 #include "components/viz/common/frame_sinks/begin_frame_args.h"
 #include "components/viz/common/frame_sinks/begin_frame_source.h"
 #include "components/viz/common/quads/selection.h"
+#include "components/viz/common/surfaces/parent_local_surface_id_allocator.h"
 #include "content/browser/renderer_host/input/mouse_wheel_phase_handler.h"
 #include "content/browser/renderer_host/input/stylus_text_selector.h"
 #include "content/browser/renderer_host/render_widget_host_view_base.h"
@@ -381,7 +382,7 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid
       const gfx::Size& dst_size_in_pixel,
       base::OnceCallback<void(const SkBitmap&)> callback);
 
-  void DestroyDelegatedContent();
+  void EvictDelegatedContent();
   void OnLostResources();
 
   enum BeginFrameRequestType {
@@ -513,6 +514,8 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid
   MouseWheelPhaseHandler mouse_wheel_phase_handler_;
   uint32_t latest_capture_sequence_number_ = 0u;
 
+  viz::ParentLocalSurfaceIdAllocator local_surface_id_allocator_;
+
   base::WeakPtrFactory<RenderWidgetHostViewAndroid> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostViewAndroid);

ui/android/BUILD.gn

@@ -316,6 +316,7 @@ test("ui_android_unittests") {
     "//cc:test_support",
     "//components/viz/host:host",
     "//components/viz/service:service",
+    "//components/viz/test:test_support",
     "//skia",
     "//testing/gmock",
     "//testing/gtest",

ui/android/DEPS

@@ -21,3 +21,9 @@ include_rules = [
   "+ui/events",
   "+ui/gfx",
 ]
+
+specific_include_rules = {
+  ".*test\.(cc|h)": [
+    "+components/viz/test"
+  ]
+}

ui/android/delegated_frame_host_android.h

@@ -7,9 +7,9 @@
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "cc/layers/deadline_policy.h"
 #include "components/viz/common/frame_sinks/copy_output_request.h"
 #include "components/viz/common/resources/returned_resource.h"
-#include "components/viz/common/surfaces/parent_local_surface_id_allocator.h"
 #include "components/viz/common/surfaces/surface_info.h"
 #include "components/viz/host/host_frame_sink_client.h"
 #include "components/viz/service/frame_sinks/compositor_frame_sink_support.h"
@@ -66,7 +66,7 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid
       base::Optional<viz::HitTestRegionList> hit_test_region_list);
   void DidNotProduceFrame(const viz::BeginFrameAck& ack);
 
-  void DestroyDelegatedContent();
+  void EvictDelegatedFrame();
 
   bool HasDelegatedContent() const;
 
@@ -88,7 +88,9 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid
   void AttachToCompositor(WindowAndroidCompositor* compositor);
   void DetachFromCompositor();
 
-  void SynchronizeVisualProperties(gfx::Size size_in_pixels);
+  void EmbedSurface(const viz::LocalSurfaceId& new_pending_local_surface_id,
+                    const gfx::Size& new_pending_size_in_pixels,
+                    cc::DeadlinePolicy deadline_policy);
 
   // Called when we begin a resize operation. Takes the compositor lock until we
   // receive a frame of the expected size.
@@ -98,15 +100,6 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid
   // surface exists (!HasDelegatedContent()).
   const viz::SurfaceId& SurfaceId() const;
 
-  // Returns the local surface ID for this delegated content.
-  const viz::LocalSurfaceId& GetLocalSurfaceId() const;
-
-  // TODO(fsamuel): We should move the viz::ParentLocalSurfaceIdAllocator to
-  // RenderWidgetHostViewAndroid.
-  viz::ParentLocalSurfaceIdAllocator* GetLocalSurfaceIdAllocator() {
-    return &local_surface_id_allocator_;
-  }
-
   void TakeFallbackContentFrom(DelegatedFrameHostAndroid* other);
 
   void DidNavigate();
@@ -148,14 +141,12 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid
   std::unique_ptr<viz::CompositorFrameSinkSupport> support_;
   viz::ExternalBeginFrameSource begin_frame_source_;
 
-  viz::SurfaceInfo surface_info_;
   bool has_transparent_background_ = false;
 
   scoped_refptr<cc::SurfaceLayer> content_layer_;
 
   const bool enable_surface_synchronization_;
   const bool enable_viz_;
-  viz::ParentLocalSurfaceIdAllocator local_surface_id_allocator_;
 
   // The size we are resizing to. Once we receive a frame of this size we can
   // release any resize compositor lock.
@@ -171,9 +162,14 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid
   std::unique_ptr<ui::CompositorLock> compositor_pending_resize_lock_;
 
   // Whether we've received a frame from the renderer since navigating.
-  // Only used in Viz mode.
+  // Only used when surface synchronization is on.
+  uint32_t first_parent_sequence_number_after_navigation_ = 0;
   bool received_frame_after_navigation_ = false;
-  uint32_t parent_sequence_number_at_navigation_ = 0;
+
+  // The local surface id as of the most recent call to
+  // EmbedSurface. This is the surface that we expect future frames to
+  // reference. This will eventually equal the active surface.
+  viz::LocalSurfaceId pending_local_surface_id_;
 
   DISALLOW_COPY_AND_ASSIGN(DelegatedFrameHostAndroid);
 };

ui/android/delegated_frame_host_android_unittest.cc

@@ -8,8 +8,10 @@
 #include "cc/layers/layer.h"
 #include "cc/layers/solid_color_layer.h"
 #include "components/viz/common/hit_test/hit_test_region_list.h"
+#include "components/viz/common/surfaces/parent_local_surface_id_allocator.h"
 #include "components/viz/host/host_frame_sink_manager.h"
 #include "components/viz/service/frame_sinks/frame_sink_manager_impl.h"
+#include "components/viz/test/compositor_frame_helpers.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/android/resources/resource_manager.h"
@@ -77,29 +79,24 @@ class DelegatedFrameHostAndroidTest : public testing::Test {
         task_runner_(new base::TestMockTimeTaskRunner()),
         lock_manager_(task_runner_, &lock_manager_client_) {
     host_frame_sink_manager_.SetLocalManager(&frame_sink_manager_impl_);
+    frame_sink_manager_impl_.SetLocalClient(&host_frame_sink_manager_);
     view_.SetLayer(cc::SolidColorLayer::Create());
     frame_host_ = std::make_unique<DelegatedFrameHostAndroid>(
         &view_, &host_frame_sink_manager_, &client_, frame_sink_id_);
   }
 
-  static viz::LocalSurfaceId GetFakeId() {
-    return viz::LocalSurfaceId(1, 1, base::UnguessableToken::Create());
-  }
-
   ui::CompositorLock* GetLock(CompositorLockClient* client,
                               base::TimeDelta time_delta) {
     return lock_manager_.GetCompositorLock(client, time_delta).release();
   }
 
   void SubmitCompositorFrame(const gfx::Size& frame_size = gfx::Size(10, 10)) {
-    viz::CompositorFrame frame;
-    auto render_pass = viz::RenderPass::Create();
-    render_pass->output_rect = gfx::Rect(frame_size);
-    frame.render_pass_list.push_back(std::move(render_pass));
-    frame.metadata.begin_frame_ack.sequence_number = 1;
-    frame.metadata.device_scale_factor = 1;
-    frame_host_->SubmitCompositorFrame(GetFakeId(), std::move(frame),
-                                       base::nullopt);
+    viz::CompositorFrame frame =
+        viz::CompositorFrameBuilder()
+            .AddRenderPass(gfx::Rect(frame_size), gfx::Rect(frame_size))
+            .Build();
+    frame_host_->SubmitCompositorFrame(allocator_.GenerateId(),
+                                       std::move(frame), base::nullopt);
   }
 
   void SetUpValidFrame(const gfx::Size& frame_size) {
@@ -123,6 +120,7 @@ class DelegatedFrameHostAndroidTest : public testing::Test {
   viz::HostFrameSinkManager host_frame_sink_manager_;
   MockDelegatedFrameHostAndroidClient client_;
   viz::FrameSinkId frame_sink_id_;
+  viz::ParentLocalSurfaceIdAllocator allocator_;
   std::unique_ptr<DelegatedFrameHostAndroid> frame_host_;
   MockCompositorLockManagerClient lock_manager_client_;
   scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;