Surface synchronization: Improve input throttling on Chrome OS

Input throttling with surface synchronization didn't always work well.

Occasionally a huge number of input events from Ash would be processed
between the time a resize request is first initiated and the time the
surface synchronization event began. This caused a lot of unnecessary
jank. Traces showed that the browser UI process was busy doing a lot of input
processing work but the resize in the renderer consumed very little
time so there was definitely an opportunity to improve input throttling.

This CL takes a different (simpler/better) approach. As soon as a
synchronization event is detected in DelegatedFrameHost (we know we
will produce a UI CompositorFrame that will synchronize with the
renderer), we throttle input events until a browser UI commit
(OnChildResizing guarantees an upcoming commit).

Bug: 672962
Change-Id: I75c8328bb846eff2817a805e63a6367ebb0714a2
Reviewed-on: https://chromium-review.googlesource.com/822658
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523788}

ash/extended_desktop_unittest.cc

@@ -336,6 +336,10 @@ TEST_F(ExtendedDesktopTest, GetRootWindowMatching) {
 }
 
 TEST_F(ExtendedDesktopTest, Capture) {
+  // This test deals with input events but not visuals so don't throttle input
+  // on visuals.
+  aura::Env::GetInstance()->set_throttle_input_on_resize_for_testing(false);
+
   UpdateDisplay("1000x600,600x400");
   aura::Window::Windows root_windows = Shell::GetAllRootWindows();
 
@@ -819,6 +823,9 @@ TEST_F(ExtendedDesktopTest, KeyEventsOnLockScreen) {
 }
 
 TEST_F(ExtendedDesktopTest, PassiveGrab) {
+  // This test deals with input events but not visuals so don't throttle input
+  // on visuals.
+  aura::Env::GetInstance()->set_throttle_input_on_resize_for_testing(false);
   EventLocationRecordingEventHandler event_handler;
   ash::Shell::Get()->AddPreTargetHandler(&event_handler);
 

ash/wm/overview/window_selector_unittest.cc

@@ -2113,6 +2113,8 @@ TEST_F(SplitViewWindowSelectorTest, DragOverviewWindowToSnap) {
 }
 
 TEST_F(SplitViewWindowSelectorTest, Dragging) {
+  aura::Env::GetInstance()->set_throttle_input_on_resize_for_testing(false);
+
   ui::test::EventGenerator& generator = GetEventGenerator();
 
   std::unique_ptr<aura::Window> right_window = CreateTestWindow();

content/browser/renderer_host/delegated_frame_host.cc

@@ -299,8 +299,12 @@ void DelegatedFrameHost::OnAggregatedSurfaceDamage(
 }
 
 void DelegatedFrameHost::WasResized() {
+  const viz::SurfaceId* primary_surface_id =
+      client_->DelegatedFrameHostGetLayer()->GetPrimarySurfaceId();
   if (enable_surface_synchronization_ &&
-      client_->DelegatedFrameHostIsVisible()) {
+      client_->DelegatedFrameHostIsVisible() &&
+      (!primary_surface_id || primary_surface_id->local_surface_id() !=
+                                  client_->GetLocalSurfaceId())) {
     current_frame_size_in_dip_ = client_->DelegatedFrameHostDesiredSizeInDIP();
 
     viz::SurfaceId surface_id(frame_sink_id_, client_->GetLocalSurfaceId());

ui/aura/env.h

@@ -118,6 +118,11 @@ class AURA_EXPORT Env : public ui::EventTarget,
   }
   ui::ContextFactory* context_factory() { return context_factory_; }
 
+  void set_throttle_input_on_resize_for_testing(bool throttle_input) {
+    throttle_input_on_resize_ = throttle_input;
+  }
+  bool throttle_input_on_resize() const { return throttle_input_on_resize_; }
+
   void set_context_factory_private(
       ui::ContextFactoryPrivate* context_factory_private) {
     context_factory_private_ = context_factory_private;
@@ -222,6 +227,8 @@ class AURA_EXPORT Env : public ui::EventTarget,
   // creating a different WindowPort implementation.
   bool in_mus_shutdown_ = false;
 
+  bool throttle_input_on_resize_ = true;
+
   DISALLOW_COPY_AND_ASSIGN(Env);
 };
 

ui/aura/window_tree_host.cc

@@ -420,21 +420,10 @@ void WindowTreeHost::MoveCursorToInternal(const gfx::Point& root_location,
   dispatcher()->OnCursorMovedToRootLocation(root_location);
 }
 
-void WindowTreeHost::OnCompositingDidCommit(ui::Compositor* compositor) {}
-
-void WindowTreeHost::OnCompositingStarted(ui::Compositor* compositor,
-                                          base::TimeTicks start_time) {
-  if (!synchronizing_with_child_on_next_frame_)
-    return;
-  synchronizing_with_child_on_next_frame_ = false;
-  synchronization_start_time_ = base::TimeTicks::Now();
-  dispatcher_->HoldPointerMoves();
-  holding_pointer_moves_ = true;
-}
-
-void WindowTreeHost::OnCompositingEnded(ui::Compositor* compositor) {
+void WindowTreeHost::OnCompositingDidCommit(ui::Compositor* compositor) {
   if (!holding_pointer_moves_)
     return;
+
   dispatcher_->ReleasePointerMoves();
   holding_pointer_moves_ = false;
   DCHECK(!synchronization_start_time_.is_null());
@@ -442,11 +431,20 @@ void WindowTreeHost::OnCompositingEnded(ui::Compositor* compositor) {
                       base::TimeTicks::Now() - synchronization_start_time_);
 }
 
+void WindowTreeHost::OnCompositingStarted(ui::Compositor* compositor,
+                                          base::TimeTicks start_time) {}
+
+void WindowTreeHost::OnCompositingEnded(ui::Compositor* compositor) {}
+
 void WindowTreeHost::OnCompositingLockStateChanged(ui::Compositor* compositor) {
 }
 
 void WindowTreeHost::OnCompositingChildResizing(ui::Compositor* compositor) {
-  synchronizing_with_child_on_next_frame_ = true;
+  if (!Env::GetInstance()->throttle_input_on_resize() || holding_pointer_moves_)
+    return;
+  synchronization_start_time_ = base::TimeTicks::Now();
+  dispatcher_->HoldPointerMoves();
+  holding_pointer_moves_ = true;
 }
 
 void WindowTreeHost::OnCompositingShuttingDown(ui::Compositor* compositor) {

ui/aura/window_tree_host.h

@@ -302,9 +302,6 @@ class AURA_EXPORT WindowTreeHost : public ui::internal::InputMethodDelegate,
 
   gfx::Insets output_surface_padding_in_pixels_;
 
-  // Set to true if the next CompositorFrame will block on a new child surface.
-  bool synchronizing_with_child_on_next_frame_ = false;
-
   // Set to the time the synchronization event began.
   base::TimeTicks synchronization_start_time_;
 

ui/aura/window_tree_host_unittest.cc

@@ -78,26 +78,24 @@ TEST_F(WindowTreeHostTest, DPIWindowSize) {
 }
 
 TEST_F(WindowTreeHostTest, HoldPointerMovesOnChildResizing) {
-  // Signal to the ui::Compositor that a child is resizing. This will
-  // trigger input throttling on the next BeginFrame.
-  host()->compositor()->OnChildResizing();
-
-  // Wait for a CompositorFrame to be submitted.
-  ui::DrawWaiterForTest::WaitForCompositingStarted(host()->compositor());
   aura::WindowEventDispatcher* dispatcher = host()->dispatcher();
 
   aura::test::WindowEventDispatcherTestApi dispatcher_api(dispatcher);
 
-  // Pointer moves should be throttled until Viz ACKs. If surface
-  // synchronization is on, this may happen several BeginFrames later.
-  // This rate limits further resizing while Viz tries to synchronize
-  // the visuals of multiple clients.
+  EXPECT_FALSE(dispatcher_api.HoldingPointerMoves());
+
+  // Signal to the ui::Compositor that a child is resizing. This will
+  // immediately trigger input throttling.
+  host()->compositor()->OnChildResizing();
+
+  // Pointer moves should be throttled until the next commit. This has the
+  // effect of prioritizing the resize event above other operations in aura.
   EXPECT_TRUE(dispatcher_api.HoldingPointerMoves());
 
-  // Wait until Viz ACKs the submitted CompositorFrame.
-  ui::DrawWaiterForTest::WaitForCompositingEnded(host()->compositor());
+  // Wait for a CompositorFrame to be submitted.
+  ui::DrawWaiterForTest::WaitForCompositingStarted(host()->compositor());
 
-  // Pointer moves should be routed normally after the ACK.
+  // Pointer moves should be routed normally after commit.
   EXPECT_FALSE(dispatcher_api.HoldingPointerMoves());
 }