Skip to content

Commit

Permalink
Dropping unnecessary SwapPromiseMonitor member.
Browse files Browse the repository at this point in the history 
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}
  • Loading branch information
@tommckee1
tommckee1 authored and Commit Bot committed Apr 4, 2019
1 parent 1b7e6b8 commit ab702c7
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 85 deletions.
26 changes: 0 additions & 26 deletions cc/trees/latency_info_swap_promise_monitor.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<SwapPromise> swap_promise(
new LatencyInfoSwapPromise(new_latency));
host_impl_->QueueSwapPromiseForMainThreadScrollUpdate(
std::move(swap_promise));
}
}

} // namespace cc
1 change: 0 additions & 1 deletion cc/trees/latency_info_swap_promise_monitor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ class CC_EXPORT LatencyInfoSwapPromiseMonitor : public SwapPromiseMonitor {

void OnSetNeedsCommitOnMain() override;
void OnSetNeedsRedrawOnImpl() override;
void OnForwardScrollUpdateToMainThreadOnImpl() override;

private:
ui::LatencyInfo* latency_;
Expand Down
12 changes: 0 additions & 12 deletions cc/trees/layer_tree_host_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion cc/trees/layer_tree_host_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,6 @@ class CC_EXPORT LayerTreeHostImpl
bool lost);

void NotifySwapPromiseMonitorsOfSetNeedsRedraw();
void NotifySwapPromiseMonitorsOfForwardingToMainThread();

void UpdateRootLayerStateForSynchronousInputHandler();

Expand Down
59 changes: 20 additions & 39 deletions cc/trees/layer_tree_host_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10555,89 +10555,76 @@ 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;

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, &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
// 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, &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<SimpleSwapPromiseMonitor> 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<SimpleSwapPromiseMonitor> 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.
Expand All @@ -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.
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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<SimpleSwapPromiseMonitor> 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();
Expand Down Expand Up @@ -11847,19 +11830,17 @@ 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, &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,
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();
Expand Down
4 changes: 0 additions & 4 deletions cc/trees/layer_tree_host_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
};
Expand Down
1 change: 0 additions & 1 deletion cc/trees/swap_promise_manager_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ class MockSwapPromiseMonitor : public SwapPromiseMonitor {

MOCK_METHOD0(OnSetNeedsCommitOnMain, void());
void OnSetNeedsRedrawOnImpl() override {}
void OnForwardScrollUpdateToMainThreadOnImpl() override {}
};

class MockSwapPromise : public SwapPromise {
Expand Down
1 change: 0 additions & 1 deletion cc/trees/swap_promise_monitor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down

0 comments on commit ab702c7

Please sign in to comment.