Surface Synchronization: Don't use CompositorLock if enabled

Don't use the CompositorLock if surface synchronization is enabled.
This was causing issues on telemetry bots where the CompositorLock
was preventing the UI from submitting CompositorFrames for a long
period of time and so input latency was very high.

Before:
https://pinpoint-dot-chromeperf.appspot.com/job/11675b9b240000
After:
https://pinpoint-dot-chromeperf.appspot.com/job/152a7367240000

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I95407531216b543fb4c192387bd67fe690bee0b4
Bug: 672962
Reviewed-on: https://chromium-review.googlesource.com/1119191
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571414}

content/browser/renderer_host/render_widget_host_view_android.cc

@@ -198,7 +198,7 @@ RenderWidgetHostViewAndroid::RenderWidgetHostViewAndroid(
   if (using_browser_compositor_) {
     delegated_frame_host_ = std::make_unique<ui::DelegatedFrameHostAndroid>(
         &view_, CompositorImpl::GetHostFrameSinkManager(), this,
-        host()->GetFrameSinkId());
+        host()->GetFrameSinkId(), features::IsSurfaceSynchronizationEnabled());
     if (is_showing_) {
       delegated_frame_host_->WasShown(
           local_surface_id_allocator_.GetCurrentLocalSurfaceId(),

ui/android/delegated_frame_host_android.cc

@@ -60,14 +60,14 @@ DelegatedFrameHostAndroid::DelegatedFrameHostAndroid(
     ui::ViewAndroid* view,
     viz::HostFrameSinkManager* host_frame_sink_manager,
     Client* client,
-    const viz::FrameSinkId& frame_sink_id)
+    const viz::FrameSinkId& frame_sink_id,
+    bool enable_surface_synchronization)
     : frame_sink_id_(frame_sink_id),
       view_(view),
       host_frame_sink_manager_(host_frame_sink_manager),
       client_(client),
       begin_frame_source_(this),
-      enable_surface_synchronization_(
-          features::IsSurfaceSynchronizationEnabled()),
+      enable_surface_synchronization_(enable_surface_synchronization),
       enable_viz_(
           base::FeatureList::IsEnabled(features::kVizDisplayCompositor)),
       frame_evictor_(std::make_unique<viz::FrameEvictor>(this)) {
@@ -221,8 +221,10 @@ void DelegatedFrameHostAndroid::AttachToCompositor(
   // browser in cases where the renderer hangs or another factor prevents a
   // frame from being produced. If we already have delegated content, no need
   // to take the lock.
-  if (!enable_viz_ && compositor->IsDrawingFirstVisibleFrame() &&
-      !HasDelegatedContent()) {
+  // If surface synchronization is enabled, then it will block browser UI until
+  // a renderer frame is available instead.
+  if (!enable_surface_synchronization_ &&
+      compositor->IsDrawingFirstVisibleFrame() && !HasDelegatedContent()) {
     compositor_attach_until_frame_lock_ = compositor->GetCompositorLock(
         this, base::TimeDelta::FromSeconds(kFirstFrameTimeoutSeconds));
   }

ui/android/delegated_frame_host_android.h

@@ -57,7 +57,8 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid
   DelegatedFrameHostAndroid(ViewAndroid* view,
                             viz::HostFrameSinkManager* host_frame_sink_manager,
                             Client* client,
-                            const viz::FrameSinkId& frame_sink_id);
+                            const viz::FrameSinkId& frame_sink_id,
+                            bool enable_surface_synchronization);
 
   ~DelegatedFrameHostAndroid() override;
 

ui/android/delegated_frame_host_android_unittest.cc

@@ -82,11 +82,19 @@ class DelegatedFrameHostAndroidTest : public testing::Test {
         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_);
+  }
+
+  void SetUp() override {
     view_.SetLayer(cc::SolidColorLayer::Create());
     frame_host_ = std::make_unique<DelegatedFrameHostAndroid>(
-        &view_, &host_frame_sink_manager_, &client_, frame_sink_id_);
+        &view_, &host_frame_sink_manager_, &client_, frame_sink_id_,
+        ShouldEnableSurfaceSynchronization());
   }
 
+  void TearDown() override { frame_host_.reset(); }
+
+  virtual bool ShouldEnableSurfaceSynchronization() const { return false; }
+
   ui::CompositorLock* GetLock(CompositorLockClient* client,
                               base::TimeDelta time_delta) {
     return lock_manager_.GetCompositorLock(client, time_delta).release();
@@ -130,6 +138,24 @@ class DelegatedFrameHostAndroidTest : public testing::Test {
   CompositorLockManager lock_manager_;
 };
 
+class DelegatedFrameHostAndroidSurfaceSynchronizationTest
+    : public DelegatedFrameHostAndroidTest {
+ public:
+  DelegatedFrameHostAndroidSurfaceSynchronizationTest() = default;
+  ~DelegatedFrameHostAndroidSurfaceSynchronizationTest() override = default;
+
+  bool ShouldEnableSurfaceSynchronization() const override { return true; }
+};
+
+// If surface synchronization is enabled then we should not be acquiring a
+// compositor lock on attach.
+TEST_F(DelegatedFrameHostAndroidSurfaceSynchronizationTest,
+       NoCompositorLockOnAttach) {
+  EXPECT_CALL(compositor_, IsDrawingFirstVisibleFrame()).Times(0);
+  EXPECT_CALL(compositor_, DoGetCompositorLock(_, _)).Times(0);
+  frame_host_->AttachToCompositor(&compositor_);
+}
+
 TEST_F(DelegatedFrameHostAndroidTest, CompositorLockDuringFirstFrame) {
   // Attach during the first frame, lock will be taken.
   EXPECT_CALL(compositor_, IsDrawingFirstVisibleFrame()).WillOnce(Return(true));