From ab702c7308960f680f6ad2edfd014236d0ebd3f1 Mon Sep 17 00:00:00 2001 From: Tom McKee Date: Thu, 4 Apr 2019 15:29:51 +0000 Subject: [PATCH] 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 Reviewed-by: Timothy Dresser Commit-Queue: Tom McKee Cr-Commit-Position: refs/heads/master@{#647752} --- cc/trees/latency_info_swap_promise_monitor.cc | 26 -------- cc/trees/latency_info_swap_promise_monitor.h | 1 - cc/trees/layer_tree_host_impl.cc | 12 ---- cc/trees/layer_tree_host_impl.h | 1 - cc/trees/layer_tree_host_impl_unittest.cc | 59 +++++++------------ cc/trees/layer_tree_host_unittest.cc | 4 -- cc/trees/swap_promise_manager_unittest.cc | 1 - cc/trees/swap_promise_monitor.h | 1 - 8 files changed, 20 insertions(+), 85 deletions(-) diff --git a/cc/trees/latency_info_swap_promise_monitor.cc b/cc/trees/latency_info_swap_promise_monitor.cc index 5da4b6895b382e..a69055c986dcc2 100644 --- a/cc/trees/latency_info_swap_promise_monitor.cc +++ b/cc/trees/latency_info_swap_promise_monitor.cc @@ -25,16 +25,6 @@ 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 { @@ -67,20 +57,4 @@ 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 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 index c127b966018079..eb1d8f4a63589b 100644 --- a/cc/trees/latency_info_swap_promise_monitor.h +++ b/cc/trees/latency_info_swap_promise_monitor.h @@ -26,7 +26,6 @@ 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 index 927ca0a27a95e8..9c29cbe691223f 100644 --- a/cc/trees/layer_tree_host_impl.cc +++ b/cc/trees/layer_tree_host_impl.cc @@ -4484,12 +4484,6 @@ 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(); @@ -5555,12 +5549,6 @@ 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 index 8e760536879f75..2f72eaea5f5c32 100644 --- a/cc/trees/layer_tree_host_impl.h +++ b/cc/trees/layer_tree_host_impl.h @@ -862,7 +862,6 @@ 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 index 6602005a9e7dea..125a5caa663005 100644 --- a/cc/trees/layer_tree_host_impl_unittest.cc +++ b/cc/trees/layer_tree_host_impl_unittest.cc @@ -10555,15 +10555,13 @@ 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* forward_to_main_count) + int* set_needs_redraw_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), - forward_to_main_count_(forward_to_main_count) {} + set_needs_redraw_count_(set_needs_redraw_count) {} ~SimpleSwapPromiseMonitor() override = default; @@ -10571,30 +10569,23 @@ class SimpleSwapPromiseMonitor : public SwapPromiseMonitor { 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 swap_promise_monitor( - new SimpleSwapPromiseMonitor( - nullptr, host_impl_.get(), &set_needs_commit_count, - &set_needs_redraw_count, &forward_to_main_count)); + new SimpleSwapPromiseMonitor(nullptr, host_impl_.get(), + &set_needs_commit_count, + &set_needs_redraw_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 @@ -10602,42 +10593,38 @@ TEST_F(LayerTreeHostImplTest, SimpleSwapPromiseMonitor) { 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 swap_promise_monitor( - new SimpleSwapPromiseMonitor( - nullptr, host_impl_.get(), &set_needs_commit_count, - &set_needs_redraw_count, &forward_to_main_count)); + new SimpleSwapPromiseMonitor(nullptr, host_impl_.get(), + &set_needs_commit_count, + &set_needs_redraw_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 swap_promise_monitor( - new SimpleSwapPromiseMonitor( - nullptr, host_impl_.get(), &set_needs_commit_count, - &set_needs_redraw_count, &forward_to_main_count)); + new SimpleSwapPromiseMonitor(nullptr, host_impl_.get(), + &set_needs_commit_count, + &set_needs_redraw_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 swap_promise_monitor( - new SimpleSwapPromiseMonitor( - nullptr, host_impl_.get(), &set_needs_commit_count, - &set_needs_redraw_count, &forward_to_main_count)); + new SimpleSwapPromiseMonitor(nullptr, host_impl_.get(), + &set_needs_commit_count, + &set_needs_redraw_count)); SetupScrollAndContentsLayers(gfx::Size(100, 100)); // Scrolling normally should not trigger any forwarding. @@ -10654,7 +10641,6 @@ 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. @@ -10672,7 +10658,6 @@ 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); } } @@ -11805,18 +11790,16 @@ 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 swap_promise_monitor( - new SimpleSwapPromiseMonitor( - nullptr, host_impl_.get(), &set_needs_commit_count, - &set_needs_redraw_count, &forward_to_main_count)); + new SimpleSwapPromiseMonitor(nullptr, host_impl_.get(), + &set_needs_commit_count, + &set_needs_redraw_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(); @@ -11847,11 +11830,10 @@ 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 swap_promise_monitor( - new SimpleSwapPromiseMonitor( - nullptr, host_impl_.get(), &set_needs_commit_count, - &set_needs_redraw_count, &forward_to_main_count)); + new SimpleSwapPromiseMonitor(nullptr, host_impl_.get(), + &set_needs_commit_count, + &set_needs_redraw_count)); // Update target. EXPECT_EQ( InputHandler::SCROLL_ON_IMPL_THREAD, @@ -11859,7 +11841,6 @@ TEST_F(LayerTreeHostImplTest, ScrollAnimated) { 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 index 6afc9c90f1721e..8787743e322d5a 100644 --- a/cc/trees/layer_tree_host_unittest.cc +++ b/cc/trees/layer_tree_host_unittest.cc @@ -6108,10 +6108,6 @@ 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 index d88818e90e21ef..32d818defcaa50 100644 --- a/cc/trees/swap_promise_manager_unittest.cc +++ b/cc/trees/swap_promise_manager_unittest.cc @@ -21,7 +21,6 @@ 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 index 7efe2e281a5a30..3350ca1ffd9dbf 100644 --- a/cc/trees/swap_promise_monitor.h +++ b/cc/trees/swap_promise_monitor.h @@ -33,7 +33,6 @@ class CC_EXPORT SwapPromiseMonitor { virtual void OnSetNeedsCommitOnMain() = 0; virtual void OnSetNeedsRedrawOnImpl() = 0; - virtual void OnForwardScrollUpdateToMainThreadOnImpl() = 0; protected: SwapPromiseManager* swap_promise_manager_;