Revert "Dropping unnecessary SwapPromiseMonitor member."

This reverts commit ab702c7. Reason for revert: Although the underlying metrics don't seem to be used by chromium itself, it looks like these things are used in catapult. Reverting this for now. Going to sync with catapult folks to see if they're still using this stuff. If they aren't, we can reland this. Original change's description: > Dropping unnecessary SwapPromiseMonitor member. > > OnForwardScrollUpdateToMainThreadOnImpl is used to record latency > components that aren't used downstream. > > There is still some follow-on work to remove the unused latency > components definitions from LatencyInfo. > > Change-Id: I7d54a3fcabecd63ddabfc4705924a3c37d2ad480 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1551063 > Reviewed-by: David Bokan <bokan@chromium.org> > Reviewed-by: Timothy Dresser <tdresser@chromium.org> > Commit-Queue: Tom McKee <tommckee@chromium.org> > Cr-Commit-Position: refs/heads/master@{#647752} TBR=bokan@chromium.org,tdresser@chromium.org,tommckee@chromium.org Change-Id: Ied4ca2e876c3ecf13e145777a4826c497ce1d582 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1553650 Reviewed-by: Timothy Dresser <tdresser@chromium.org> Commit-Queue: Tom McKee <tommckee@chromium.org> Cr-Commit-Position: refs/heads/master@{#647855}
marcosholgado · Apr 4, 2019 · 78615ab · 78615ab
1 parent fab66ba
commit 78615ab
Show file tree

Hide file tree

Showing 8 changed files with 85 additions and 20 deletions.
diff --git a/cc/trees/latency_info_swap_promise_monitor.cc b/cc/trees/latency_info_swap_promise_monitor.cc
@@ -25,6 +25,16 @@ bool AddRenderingScheduledComponent(ui::LatencyInfo* latency_info,
   return true;
 }
 
+bool AddForwardingScrollUpdateToMainComponent(ui::LatencyInfo* latency_info) {
+  if (latency_info->FindLatency(
+          ui::INPUT_EVENT_LATENCY_FORWARD_SCROLL_UPDATE_TO_MAIN_COMPONENT,
+          nullptr))
+    return false;
+  latency_info->AddLatencyNumber(
+      ui::INPUT_EVENT_LATENCY_FORWARD_SCROLL_UPDATE_TO_MAIN_COMPONENT);
+  return true;
+}
+
 }  // namespace
 
 namespace cc {
@@ -57,4 +67,20 @@ void LatencyInfoSwapPromiseMonitor::OnSetNeedsRedrawOnImpl() {
   }
 }
 
+void LatencyInfoSwapPromiseMonitor::OnForwardScrollUpdateToMainThreadOnImpl() {
+  if (AddForwardingScrollUpdateToMainComponent(latency_)) {
+    ui::LatencyInfo new_latency;
+    new_latency.CopyLatencyFrom(
+        *latency_,
+        ui::INPUT_EVENT_LATENCY_FORWARD_SCROLL_UPDATE_TO_MAIN_COMPONENT);
+    new_latency.AddLatencyNumberWithTraceName(
+        ui::LATENCY_BEGIN_SCROLL_LISTENER_UPDATE_MAIN_COMPONENT,
+        "ScrollUpdate");
+    std::unique_ptr<SwapPromise> swap_promise(
+        new LatencyInfoSwapPromise(new_latency));
+    host_impl_->QueueSwapPromiseForMainThreadScrollUpdate(
+        std::move(swap_promise));
+  }
+}
+
 }  // namespace cc
diff --git a/cc/trees/latency_info_swap_promise_monitor.h b/cc/trees/latency_info_swap_promise_monitor.h
@@ -26,6 +26,7 @@ class CC_EXPORT LatencyInfoSwapPromiseMonitor : public SwapPromiseMonitor {
 
   void OnSetNeedsCommitOnMain() override;
   void OnSetNeedsRedrawOnImpl() override;
+  void OnForwardScrollUpdateToMainThreadOnImpl() override;
 
  private:
   ui::LatencyInfo* latency_;

diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
@@ -4484,6 +4484,12 @@ InputHandlerScrollResult LayerTreeHostImpl::ScrollBy(
   bool did_scroll_content = did_scroll_x || did_scroll_y;
   if (did_scroll_content) {
     ShowScrollbarsForImplScroll(current_scrolling_node->element_id);
+
+    // If we are scrolling with an active scroll handler, forward latency
+    // tracking information to the main thread so the delay introduced by the
+    // handler is accounted for.
+    if (scroll_affects_scroll_handler())
+      NotifySwapPromiseMonitorsOfForwardingToMainThread();
     client_->SetNeedsCommitOnImplThread();
     SetNeedsRedraw();
     client_->RenewTreePriority();
@@ -5549,6 +5555,12 @@ void LayerTreeHostImpl::NotifySwapPromiseMonitorsOfSetNeedsRedraw() {
     (*it)->OnSetNeedsRedrawOnImpl();
 }
 
+void LayerTreeHostImpl::NotifySwapPromiseMonitorsOfForwardingToMainThread() {
+  auto it = swap_promise_monitor_.begin();
+  for (; it != swap_promise_monitor_.end(); it++)
+    (*it)->OnForwardScrollUpdateToMainThreadOnImpl();
+}
+
 void LayerTreeHostImpl::UpdateRootLayerStateForSynchronousInputHandler() {
   if (!input_handler_client_)
     return;

diff --git a/cc/trees/layer_tree_host_impl.h b/cc/trees/layer_tree_host_impl.h
@@ -862,6 +862,7 @@ class CC_EXPORT LayerTreeHostImpl
                             bool lost);
 
   void NotifySwapPromiseMonitorsOfSetNeedsRedraw();
+  void NotifySwapPromiseMonitorsOfForwardingToMainThread();
 
   void UpdateRootLayerStateForSynchronousInputHandler();
 

diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -10555,76 +10555,89 @@ class SimpleSwapPromiseMonitor : public SwapPromiseMonitor {
   SimpleSwapPromiseMonitor(LayerTreeHost* layer_tree_host,
                            LayerTreeHostImpl* layer_tree_host_impl,
                            int* set_needs_commit_count,
-                           int* set_needs_redraw_count)
+                           int* set_needs_redraw_count,
+                           int* forward_to_main_count)
       : SwapPromiseMonitor(
             (layer_tree_host ? layer_tree_host->GetSwapPromiseManager()
                              : nullptr),
             layer_tree_host_impl),
         set_needs_commit_count_(set_needs_commit_count),
-        set_needs_redraw_count_(set_needs_redraw_count) {}
+        set_needs_redraw_count_(set_needs_redraw_count),
+        forward_to_main_count_(forward_to_main_count) {}
 
   ~SimpleSwapPromiseMonitor() override = default;
 
   void OnSetNeedsCommitOnMain() override { (*set_needs_commit_count_)++; }
 
   void OnSetNeedsRedrawOnImpl() override { (*set_needs_redraw_count_)++; }
 
+  void OnForwardScrollUpdateToMainThreadOnImpl() override {
+    (*forward_to_main_count_)++;
+  }
+
  private:
   int* set_needs_commit_count_;
   int* set_needs_redraw_count_;
+  int* forward_to_main_count_;
 };
 
 TEST_F(LayerTreeHostImplTest, SimpleSwapPromiseMonitor) {
   int set_needs_commit_count = 0;
   int set_needs_redraw_count = 0;
+  int forward_to_main_count = 0;
 
   {
     std::unique_ptr<SimpleSwapPromiseMonitor> swap_promise_monitor(
-        new SimpleSwapPromiseMonitor(nullptr, host_impl_.get(),
-                                     &set_needs_commit_count,
-                                     &set_needs_redraw_count));
+        new SimpleSwapPromiseMonitor(
+            nullptr, host_impl_.get(), &set_needs_commit_count,
+            &set_needs_redraw_count, &forward_to_main_count));
     host_impl_->SetNeedsRedraw();
     EXPECT_EQ(0, set_needs_commit_count);
     EXPECT_EQ(1, set_needs_redraw_count);
+    EXPECT_EQ(0, forward_to_main_count);
   }
 
   // Now the monitor is destroyed, SetNeedsRedraw() is no longer being
   // monitored.
   host_impl_->SetNeedsRedraw();
   EXPECT_EQ(0, set_needs_commit_count);
   EXPECT_EQ(1, set_needs_redraw_count);
+  EXPECT_EQ(0, forward_to_main_count);
 
   {
     std::unique_ptr<SimpleSwapPromiseMonitor> swap_promise_monitor(
-        new SimpleSwapPromiseMonitor(nullptr, host_impl_.get(),
-                                     &set_needs_commit_count,
-                                     &set_needs_redraw_count));
+        new SimpleSwapPromiseMonitor(
+            nullptr, host_impl_.get(), &set_needs_commit_count,
+            &set_needs_redraw_count, &forward_to_main_count));
     // Redraw with damage.
     host_impl_->SetFullViewportDamage();
     host_impl_->SetNeedsRedraw();
     EXPECT_EQ(0, set_needs_commit_count);
     EXPECT_EQ(2, set_needs_redraw_count);
+    EXPECT_EQ(0, forward_to_main_count);
   }
 
   {
     std::unique_ptr<SimpleSwapPromiseMonitor> swap_promise_monitor(
-        new SimpleSwapPromiseMonitor(nullptr, host_impl_.get(),
-                                     &set_needs_commit_count,
-                                     &set_needs_redraw_count));
+        new SimpleSwapPromiseMonitor(
+            nullptr, host_impl_.get(), &set_needs_commit_count,
+            &set_needs_redraw_count, &forward_to_main_count));
     // Redraw without damage.
     host_impl_->SetNeedsRedraw();
     EXPECT_EQ(0, set_needs_commit_count);
     EXPECT_EQ(3, set_needs_redraw_count);
+    EXPECT_EQ(0, forward_to_main_count);
   }
 
   set_needs_commit_count = 0;
   set_needs_redraw_count = 0;
+  forward_to_main_count = 0;
 
   {
     std::unique_ptr<SimpleSwapPromiseMonitor> swap_promise_monitor(
-        new SimpleSwapPromiseMonitor(nullptr, host_impl_.get(),
-                                     &set_needs_commit_count,
-                                     &set_needs_redraw_count));
+        new SimpleSwapPromiseMonitor(
+            nullptr, host_impl_.get(), &set_needs_commit_count,
+            &set_needs_redraw_count, &forward_to_main_count));
     SetupScrollAndContentsLayers(gfx::Size(100, 100));
 
     // Scrolling normally should not trigger any forwarding.
@@ -10641,6 +10654,7 @@ TEST_F(LayerTreeHostImplTest, SimpleSwapPromiseMonitor) {
 
     EXPECT_EQ(0, set_needs_commit_count);
     EXPECT_EQ(1, set_needs_redraw_count);
+    EXPECT_EQ(0, forward_to_main_count);
 
     // Scrolling with a scroll handler should defer the swap to the main
     // thread.
@@ -10658,6 +10672,7 @@ TEST_F(LayerTreeHostImplTest, SimpleSwapPromiseMonitor) {
 
     EXPECT_EQ(0, set_needs_commit_count);
     EXPECT_EQ(2, set_needs_redraw_count);
+    EXPECT_EQ(1, forward_to_main_count);
   }
 }
 
@@ -11790,16 +11805,18 @@ TEST_F(LayerTreeHostImplTest, ScrollAnimated) {
     // for LatencyInfo's to be propagated along with the CompositorFrame
     int set_needs_commit_count = 0;
     int set_needs_redraw_count = 0;
+    int forward_to_main_count = 0;
     std::unique_ptr<SimpleSwapPromiseMonitor> swap_promise_monitor(
-        new SimpleSwapPromiseMonitor(nullptr, host_impl_.get(),
-                                     &set_needs_commit_count,
-                                     &set_needs_redraw_count));
+        new SimpleSwapPromiseMonitor(
+            nullptr, host_impl_.get(), &set_needs_commit_count,
+            &set_needs_redraw_count, &forward_to_main_count));
     EXPECT_EQ(
         InputHandler::SCROLL_ON_IMPL_THREAD,
         host_impl_->ScrollAnimated(gfx::Point(), gfx::Vector2d(0, 50)).thread);
 
     EXPECT_EQ(0, set_needs_commit_count);
     EXPECT_EQ(1, set_needs_redraw_count);
+    EXPECT_EQ(0, forward_to_main_count);
   }
 
   LayerImpl* scrolling_layer = host_impl_->OuterViewportScrollLayer();
@@ -11830,17 +11847,19 @@ TEST_F(LayerTreeHostImplTest, ScrollAnimated) {
     // for LatencyInfo's to be propagated along with the CompositorFrame
     int set_needs_commit_count = 0;
     int set_needs_redraw_count = 0;
+    int forward_to_main_count = 0;
     std::unique_ptr<SimpleSwapPromiseMonitor> swap_promise_monitor(
-        new SimpleSwapPromiseMonitor(nullptr, host_impl_.get(),
-                                     &set_needs_commit_count,
-                                     &set_needs_redraw_count));
+        new SimpleSwapPromiseMonitor(
+            nullptr, host_impl_.get(), &set_needs_commit_count,
+            &set_needs_redraw_count, &forward_to_main_count));
     // Update target.
     EXPECT_EQ(
         InputHandler::SCROLL_ON_IMPL_THREAD,
         host_impl_->ScrollAnimated(gfx::Point(), gfx::Vector2d(0, 50)).thread);
 
     EXPECT_EQ(0, set_needs_commit_count);
     EXPECT_EQ(1, set_needs_redraw_count);
+    EXPECT_EQ(0, forward_to_main_count);
   }
 
   host_impl_->DidFinishImplFrame();

diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc
@@ -6108,6 +6108,10 @@ class SimpleSwapPromiseMonitor : public SwapPromiseMonitor {
     ADD_FAILURE() << "Should not get called on main thread.";
   }
 
+  void OnForwardScrollUpdateToMainThreadOnImpl() override {
+    ADD_FAILURE() << "Should not get called on main thread.";
+  }
+
  private:
   int* set_needs_commit_count_;
 };

diff --git a/cc/trees/swap_promise_manager_unittest.cc b/cc/trees/swap_promise_manager_unittest.cc
@@ -21,6 +21,7 @@ class MockSwapPromiseMonitor : public SwapPromiseMonitor {
 
   MOCK_METHOD0(OnSetNeedsCommitOnMain, void());
   void OnSetNeedsRedrawOnImpl() override {}
+  void OnForwardScrollUpdateToMainThreadOnImpl() override {}
 };
 
 class MockSwapPromise : public SwapPromise {

diff --git a/cc/trees/swap_promise_monitor.h b/cc/trees/swap_promise_monitor.h
@@ -33,6 +33,7 @@ class CC_EXPORT SwapPromiseMonitor {
 
   virtual void OnSetNeedsCommitOnMain() = 0;
   virtual void OnSetNeedsRedrawOnImpl() = 0;
+  virtual void OnForwardScrollUpdateToMainThreadOnImpl() = 0;
 
  protected:
   SwapPromiseManager* swap_promise_manager_;