Surface Synchronization: Skip activation messages from Viz on Android

Android use cases do not need to worry about
OnRenderFrameMetadataChangedAfterActivation notifications because
changes either do not require synchronization or are synchronized
directly via surface synchronization. This CL saves a few IPCs and should
improve scrolling performance on Android when surface synchronization is
enabled.

Bug: 672962
Change-Id: I3fe26c56dde75394677ee2dee53f50f79e99f8f9
Reviewed-on: https://chromium-review.googlesource.com/1117198
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571003}

content/browser/renderer_host/render_frame_metadata_provider_impl.cc

@@ -78,6 +78,9 @@ void RenderFrameMetadataProviderImpl::OnRenderFrameMetadataChanged(
       observer.OnLocalSurfaceIdChanged(metadata);
   }
 
+  if (!frame_token)
+    return;
+
   // Both RenderFrameMetadataProviderImpl and FrameTokenMessageQueue are owned
   // by the same RenderWidgetHostImpl. During shutdown the queue is cleared
   // without running the callbacks.

content/renderer/render_frame_metadata_observer_impl.cc

@@ -39,6 +39,7 @@ void RenderFrameMetadataObserverImpl::OnRenderFrameSubmission(
   // frequency of change. However if there are changes in high frequency
   // fields these can be reported while testing is enabled.
   bool send_metadata = false;
+  bool needs_activation_notification = true;
   if (render_frame_metadata_observer_client_) {
     if (report_all_frame_submissions_for_testing_enabled_) {
       last_frame_token_ = compositor_frame_metadata->frame_token;
@@ -50,7 +51,8 @@ void RenderFrameMetadataObserverImpl::OnRenderFrameSubmission(
     } else {
       send_metadata = !last_render_frame_metadata_ ||
                       ShouldSendRenderFrameMetadata(
-                          *last_render_frame_metadata_, render_frame_metadata);
+                          *last_render_frame_metadata_, render_frame_metadata,
+                          &needs_activation_notification);
     }
   }
 
@@ -73,16 +75,18 @@ void RenderFrameMetadataObserverImpl::OnRenderFrameSubmission(
 #endif
 
     last_frame_token_ = compositor_frame_metadata->frame_token;
-    compositor_frame_metadata->send_frame_token_to_embedder = true;
+    compositor_frame_metadata->send_frame_token_to_embedder =
+        needs_activation_notification;
     render_frame_metadata_observer_client_->OnRenderFrameMetadataChanged(
-        last_frame_token_, metadata_copy);
+        needs_activation_notification ? last_frame_token_ : 0u, metadata_copy);
   }
 
   // Always cache the initial frame token, so that if a test connects later on
   // it can be notified of the initial state.
   if (!last_frame_token_) {
     last_frame_token_ = compositor_frame_metadata->frame_token;
-    compositor_frame_metadata->send_frame_token_to_embedder = true;
+    compositor_frame_metadata->send_frame_token_to_embedder =
+        needs_activation_notification;
   }
 }
 
@@ -103,7 +107,8 @@ void RenderFrameMetadataObserverImpl::ReportAllFrameSubmissionsForTesting(
 // static
 bool RenderFrameMetadataObserverImpl::ShouldSendRenderFrameMetadata(
     const cc::RenderFrameMetadata& rfm1,
-    const cc::RenderFrameMetadata& rfm2) {
+    const cc::RenderFrameMetadata& rfm2,
+    bool* needs_activation_notification) {
   if (rfm1.root_background_color != rfm2.root_background_color ||
       rfm1.is_scroll_offset_at_top != rfm2.is_scroll_offset_at_top ||
       rfm1.selection != rfm2.selection ||
@@ -112,6 +117,7 @@ bool RenderFrameMetadataObserverImpl::ShouldSendRenderFrameMetadata(
       rfm1.device_scale_factor != rfm2.device_scale_factor ||
       rfm1.viewport_size_in_pixels != rfm2.viewport_size_in_pixels ||
       rfm1.local_surface_id != rfm2.local_surface_id) {
+    *needs_activation_notification = true;
     return true;
   }
 
@@ -126,6 +132,7 @@ bool RenderFrameMetadataObserverImpl::ShouldSendRenderFrameMetadata(
       rfm1.scrollable_viewport_size != rfm2.scrollable_viewport_size ||
       rfm1.root_layer_size != rfm2.root_layer_size ||
       rfm1.has_transparent_background != rfm2.has_transparent_background) {
+    *needs_activation_notification = true;
     return true;
   }
 
@@ -156,10 +163,12 @@ bool RenderFrameMetadataObserverImpl::ShouldSendRenderFrameMetadata(
 
   if (old_viewport_rect != new_viewport_rect &&
       (at_left_or_right_edge || at_top_or_bottom_edge)) {
+    *needs_activation_notification = false;
     return true;
   }
 #endif
 
+  *needs_activation_notification = false;
   return false;
 }
 

content/renderer/render_frame_metadata_observer_impl.h

@@ -7,6 +7,7 @@
 
 #include "cc/trees/render_frame_metadata.h"
 #include "cc/trees/render_frame_metadata_observer.h"
+#include "content/common/content_export.h"
 #include "content/common/render_frame_metadata.mojom.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
@@ -21,7 +22,7 @@ namespace content {
 // Mojo pipe is properly bound.
 //
 // Subsequent usage should only be from the Compositor thread.
-class RenderFrameMetadataObserverImpl
+class CONTENT_EXPORT RenderFrameMetadataObserverImpl
     : public cc::RenderFrameMetadataObserver,
       public mojom::RenderFrameMetadataObserver {
  public:
@@ -40,12 +41,18 @@ class RenderFrameMetadataObserverImpl
   void ReportAllFrameSubmissionsForTesting(bool enabled) override;
 
  private:
+  friend class RenderFrameMetadataObserverImplTest;
+
   // Certain fields should always have their changes reported. This will return
   // true when there is a difference between |rfm1| and |rfm2| for those fields.
   // These fields have a low frequency rate of change.
+  // |needs_activation_notification| indicates whether the browser process
+  // expects notification of activation of the assoicated CompositorFrame from
+  // Viz.
   static bool ShouldSendRenderFrameMetadata(
       const cc::RenderFrameMetadata& rfm1,
-      const cc::RenderFrameMetadata& rfm2);
+      const cc::RenderFrameMetadata& rfm2,
+      bool* needs_activation_notification);
 
   // When true this will notifiy |render_frame_metadata_observer_client_| of all
   // frame submissions.

content/renderer/render_frame_metadata_observer_impl_unittest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/renderer/render_frame_metadata_observer_impl.h"
+
+#include "base/run_loop.h"
+#include "base/test/scoped_task_environment.h"
+#include "build/build_config.h"
+#include "cc/trees/render_frame_metadata.h"
+#include "components/viz/common/quads/compositor_frame_metadata.h"
+#include "content/common/render_frame_metadata.mojom.h"
+#include "mojo/public/cpp/bindings/binding.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+namespace {
+
+ACTION_P(InvokeClosure, closure) {
+  closure.Run();
+}
+
+}  // namespace
+
+class MockRenderFrameMetadataObserverClient
+    : public mojom::RenderFrameMetadataObserverClient {
+ public:
+  MockRenderFrameMetadataObserverClient(
+      mojom::RenderFrameMetadataObserverClientRequest client_request,
+      mojom::RenderFrameMetadataObserverPtr observer)
+      : render_frame_metadata_observer_client_binding_(
+            this,
+            std::move(client_request)),
+        render_frame_metadata_observer_ptr_(std::move(observer)) {}
+
+  MOCK_METHOD2(OnRenderFrameMetadataChanged,
+               void(uint32_t frame_token,
+                    const cc::RenderFrameMetadata& metadata));
+  MOCK_METHOD1(OnFrameSubmissionForTesting, void(uint32_t frame_token));
+
+ private:
+  mojo::Binding<mojom::RenderFrameMetadataObserverClient>
+      render_frame_metadata_observer_client_binding_;
+  mojom::RenderFrameMetadataObserverPtr render_frame_metadata_observer_ptr_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockRenderFrameMetadataObserverClient);
+};
+
+class RenderFrameMetadataObserverImplTest : public testing::Test {
+ public:
+  RenderFrameMetadataObserverImplTest() = default;
+  ~RenderFrameMetadataObserverImplTest() override = default;
+
+  RenderFrameMetadataObserverImpl& observer_impl() { return *observer_impl_; }
+
+  MockRenderFrameMetadataObserverClient& client() { return *client_; }
+
+  // testing::Test:
+  void SetUp() override {
+    mojom::RenderFrameMetadataObserverPtr ptr;
+    mojom::RenderFrameMetadataObserverRequest request = mojo::MakeRequest(&ptr);
+    mojom::RenderFrameMetadataObserverClientPtrInfo client_info;
+    mojom::RenderFrameMetadataObserverClientRequest client_request =
+        mojo::MakeRequest(&client_info);
+
+    client_ = std::make_unique<
+        testing::NiceMock<MockRenderFrameMetadataObserverClient>>(
+        std::move(client_request), std::move(ptr));
+    observer_impl_ = std::make_unique<RenderFrameMetadataObserverImpl>(
+        std::move(request), std::move(client_info));
+    observer_impl_->BindToCurrentThread();
+  }
+
+  void TearDown() override {
+    observer_impl_.reset();
+    client_.reset();
+    scoped_task_environment_.RunUntilIdle();
+  }
+
+ private:
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  std::unique_ptr<testing::NiceMock<MockRenderFrameMetadataObserverClient>>
+      client_;
+  std::unique_ptr<RenderFrameMetadataObserverImpl> observer_impl_;
+
+  DISALLOW_COPY_AND_ASSIGN(RenderFrameMetadataObserverImplTest);
+};
+
+// This test verifies that the RenderFrameMetadataObserverImpl picks up
+// the frame token from CompositorFrameMetadata and passes it along to the
+// client. This test also verifies that the RenderFrameMetadata object is
+// passed along to the client.
+TEST_F(RenderFrameMetadataObserverImplTest, ShouldSendFrameToken) {
+  viz::CompositorFrameMetadata compositor_frame_metadata;
+  compositor_frame_metadata.send_frame_token_to_embedder = false;
+  compositor_frame_metadata.frame_token = 1337;
+  cc::RenderFrameMetadata render_frame_metadata;
+  render_frame_metadata.is_mobile_optimized = true;
+  observer_impl().OnRenderFrameSubmission(render_frame_metadata,
+                                          &compositor_frame_metadata);
+  // |is_mobile_optimized| should be synchronized with frame activation so
+  // RenderFrameMetadataObserverImpl should ask for the frame token from
+  // Viz.
+  EXPECT_TRUE(compositor_frame_metadata.send_frame_token_to_embedder);
+  {
+    base::RunLoop run_loop;
+    EXPECT_CALL(client(),
+                OnRenderFrameMetadataChanged(1337, render_frame_metadata))
+        .WillOnce(InvokeClosure(run_loop.QuitClosure()));
+    run_loop.Run();
+  }
+}
+
+// This test verifies that a frame token is not requested from viz when
+// the root scroll offset changes on Android.
+#if defined(OS_ANDROID)
+TEST_F(RenderFrameMetadataObserverImplTest, ShouldSendFrameTokenOnAndroid) {
+  viz::CompositorFrameMetadata compositor_frame_metadata;
+  compositor_frame_metadata.send_frame_token_to_embedder = false;
+  compositor_frame_metadata.frame_token = 1337;
+  cc::RenderFrameMetadata render_frame_metadata;
+  render_frame_metadata.root_scroll_offset = gfx::Vector2dF(0.f, 1.f);
+  render_frame_metadata.root_layer_size = gfx::SizeF(100.f, 100.f);
+  render_frame_metadata.scrollable_viewport_size = gfx::SizeF(100.f, 50.f);
+  observer_impl().OnRenderFrameSubmission(render_frame_metadata,
+                                          &compositor_frame_metadata);
+  // The first RenderFrameMetadata will always get a corresponding frame token
+  // from Viz because this is the first frame.
+  EXPECT_TRUE(compositor_frame_metadata.send_frame_token_to_embedder);
+  {
+    base::RunLoop run_loop;
+    EXPECT_CALL(client(),
+                OnRenderFrameMetadataChanged(1337, render_frame_metadata))
+        .WillOnce(InvokeClosure(run_loop.QuitClosure()));
+    run_loop.Run();
+  }
+
+  // Scroll back to the top.
+  render_frame_metadata.root_scroll_offset = gfx::Vector2dF(0.f, 0.f);
+
+  observer_impl().OnRenderFrameSubmission(render_frame_metadata,
+                                          &compositor_frame_metadata);
+  // Android does not need a corresponding frame token.
+  EXPECT_FALSE(compositor_frame_metadata.send_frame_token_to_embedder);
+  {
+    base::RunLoop run_loop;
+    // The 0u frame token indicates that the client should not expect
+    // a corresponding frame token from Viz.
+    EXPECT_CALL(client(),
+                OnRenderFrameMetadataChanged(0u, render_frame_metadata))
+        .WillOnce(InvokeClosure(run_loop.QuitClosure()));
+    run_loop.Run();
+  }
+}
+#endif
+
+}  // namespace content

content/test/BUILD.gn

@@ -1714,6 +1714,7 @@ test("content_unittests") {
     "../renderer/p2p/filtering_network_manager_unittest.cc",
     "../renderer/p2p/ipc_network_manager_unittest.cc",
     "../renderer/peripheral_content_heuristic_unittest.cc",
+    "../renderer/render_frame_metadata_observer_impl_unittest.cc",
     "../renderer/render_thread_impl_unittest.cc",
     "../renderer/render_widget_unittest.cc",
     "../renderer/service_worker/service_worker_context_client_unittest.cc",