Skip to content

Commit

Permalink
latency: Remove unused components from LatencyInfo.
Browse files Browse the repository at this point in the history
INPUT_EVENT_LATENCY_FORWARD_SCROLL_UPDATE_TO_MAIN_COMPONENT and
LATENCY_BEGIN_SCROLL_LISTENER_UPDATE_MAIN_COMPONENT components were only
being set on ui::LatencyInfo objects, but were never being used for
anything (especially after the 'main_thread_scroll_latency' metric was
removed from telemetry). So remove these components.

BUG=849719, 1000697

Change-Id: Ia264d06598970fb32a0f21cbb7b2d15f25ffc28c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1800727
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Mohsen Izadi <mohsen@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699520}
  • Loading branch information
sadrulhc authored and Commit Bot committed Sep 24, 2019
1 parent c68785e commit 9f31191
Show file tree
Hide file tree
Showing 13 changed files with 24 additions and 125 deletions.
26 changes: 0 additions & 26 deletions cc/trees/latency_info_swap_promise_monitor.cc
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
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
11 changes: 0 additions & 11 deletions cc/trees/layer_tree_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4711,11 +4711,6 @@ InputHandlerScrollResult LayerTreeHostImpl::ScrollBy(
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 @@ -5855,12 +5850,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
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,6 @@ class CC_EXPORT LayerTreeHostImpl : public InputHandler,
bool lost);

void NotifySwapPromiseMonitorsOfSetNeedsRedraw();
void NotifySwapPromiseMonitorsOfForwardingToMainThread();

void UpdateRootLayerStateForSynchronousInputHandler();

Expand Down
59 changes: 20 additions & 39 deletions cc/trees/layer_tree_host_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9640,89 +9640,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));
SetupViewportLayersInnerScrolls(gfx::Size(50, 50), gfx::Size(100, 100));

// Scrolling normally should not trigger any forwarding.
Expand All @@ -9739,7 +9726,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 @@ -9757,7 +9743,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 @@ -10773,18 +10758,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 @@ -10815,19 +10798,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
Original file line number Diff line number Diff line change
Expand Up @@ -5941,10 +5941,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
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
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
5 changes: 2 additions & 3 deletions services/viz/public/cpp/compositing/mojom_traits_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -624,8 +624,7 @@ TEST_F(StructTraitsTest, CompositorFrameMetadata) {
const uint32_t root_background_color = 1337;
ui::LatencyInfo latency_info;
latency_info.set_trace_id(5);
latency_info.AddLatencyNumber(
ui::LATENCY_BEGIN_SCROLL_LISTENER_UPDATE_MAIN_COMPONENT);
latency_info.AddLatencyNumber(ui::INPUT_EVENT_LATENCY_BEGIN_RWH_COMPONENT);
std::vector<ui::LatencyInfo> latency_infos = {latency_info};
std::vector<SurfaceRange> referenced_surfaces;
SurfaceId id(FrameSinkId(1234, 4321),
Expand Down Expand Up @@ -677,7 +676,7 @@ TEST_F(StructTraitsTest, CompositorFrameMetadata) {
EXPECT_EQ(root_background_color, output.root_background_color);
EXPECT_EQ(latency_infos.size(), output.latency_info.size());
EXPECT_TRUE(output.latency_info[0].FindLatency(
ui::LATENCY_BEGIN_SCROLL_LISTENER_UPDATE_MAIN_COMPONENT, nullptr));
ui::INPUT_EVENT_LATENCY_BEGIN_RWH_COMPONENT, nullptr));
EXPECT_EQ(referenced_surfaces.size(), output.referenced_surfaces.size());
for (uint32_t i = 0; i < referenced_surfaces.size(); ++i)
EXPECT_EQ(referenced_surfaces[i], output.referenced_surfaces[i]);
Expand Down
14 changes: 2 additions & 12 deletions ui/latency/latency_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,12 @@ const char* GetComponentName(ui::LatencyComponentType type) {
#define CASE_TYPE(t) case ui::t: return #t
switch (type) {
CASE_TYPE(INPUT_EVENT_LATENCY_BEGIN_RWH_COMPONENT);
CASE_TYPE(LATENCY_BEGIN_SCROLL_LISTENER_UPDATE_MAIN_COMPONENT);
CASE_TYPE(INPUT_EVENT_LATENCY_SCROLL_UPDATE_ORIGINAL_COMPONENT);
CASE_TYPE(INPUT_EVENT_LATENCY_FIRST_SCROLL_UPDATE_ORIGINAL_COMPONENT);
CASE_TYPE(INPUT_EVENT_LATENCY_ORIGINAL_COMPONENT);
CASE_TYPE(INPUT_EVENT_LATENCY_UI_COMPONENT);
CASE_TYPE(INPUT_EVENT_LATENCY_RENDERING_SCHEDULED_MAIN_COMPONENT);
CASE_TYPE(INPUT_EVENT_LATENCY_RENDERING_SCHEDULED_IMPL_COMPONENT);
CASE_TYPE(INPUT_EVENT_LATENCY_FORWARD_SCROLL_UPDATE_TO_MAIN_COMPONENT);
CASE_TYPE(INPUT_EVENT_LATENCY_SCROLL_UPDATE_LAST_EVENT_COMPONENT);
CASE_TYPE(INPUT_EVENT_LATENCY_ACK_RWH_COMPONENT);
CASE_TYPE(INPUT_EVENT_LATENCY_RENDERER_MAIN_COMPONENT);
Expand All @@ -50,11 +48,6 @@ bool IsInputLatencyBeginComponent(ui::LatencyComponentType type) {
return type == ui::INPUT_EVENT_LATENCY_BEGIN_RWH_COMPONENT;
}

bool IsTraceBeginComponent(ui::LatencyComponentType type) {
return (IsInputLatencyBeginComponent(type) ||
type == ui::LATENCY_BEGIN_SCROLL_LISTENER_UPDATE_MAIN_COMPONENT);
}

// This class is for converting latency info to trace buffer friendly format.
class LatencyInfoTracedValue
: public base::trace_event::ConvertableToTraceFormat {
Expand Down Expand Up @@ -239,7 +232,7 @@ void LatencyInfo::AddLatencyNumberWithTimestampImpl(
const unsigned char* latency_info_enabled =
g_latency_info_enabled.Get().latency_info_enabled;

if (IsTraceBeginComponent(component)) {
if (IsInputLatencyBeginComponent(component)) {
// Should only ever add begin component once.
CHECK(!began_);
began_ = true;
Expand All @@ -263,10 +256,7 @@ void LatencyInfo::AddLatencyNumberWithTimestampImpl(
}

if (trace_name_str) {
if (IsInputLatencyBeginComponent(component))
trace_name_ = std::string("InputLatency::") + trace_name_str;
else
trace_name_ = std::string("Latency::") + trace_name_str;
trace_name_ = std::string("InputLatency::") + trace_name_str;
}

TRACE_EVENT_COPY_ASYNC_BEGIN_WITH_TIMESTAMP0(
Expand Down
6 changes: 0 additions & 6 deletions ui/latency/latency_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,6 @@ enum LatencyComponentType {
// BEGIN COMPONENT is when we show the latency begin in chrome://tracing.
// Timestamp when the input event is sent from RenderWidgetHost to renderer.
INPUT_EVENT_LATENCY_BEGIN_RWH_COMPONENT,
// In threaded scrolling, main thread scroll listener update is async to
// scroll processing in impl thread. This is the timestamp when we consider
// the main thread scroll listener update is begun.
LATENCY_BEGIN_SCROLL_LISTENER_UPDATE_MAIN_COMPONENT,
// ---------------------------NORMAL COMPONENT-------------------------------
// The original timestamp of the touch event which converts to scroll update.
INPUT_EVENT_LATENCY_SCROLL_UPDATE_ORIGINAL_COMPONENT,
Expand All @@ -66,8 +62,6 @@ enum LatencyComponentType {
// This is special component indicating there is rendering scheduled for
// the event associated with this LatencyInfo on impl thread.
INPUT_EVENT_LATENCY_RENDERING_SCHEDULED_IMPL_COMPONENT,
// Timestamp when a scroll update is forwarded to the main thread.
INPUT_EVENT_LATENCY_FORWARD_SCROLL_UPDATE_TO_MAIN_COMPONENT,
// Original timestamp of the last event that has been coalesced into this one.
INPUT_EVENT_LATENCY_SCROLL_UPDATE_LAST_EVENT_COMPONENT,
// Timestamp when the event's ack is received by the RWH.
Expand Down
6 changes: 0 additions & 6 deletions ui/latency/mojom/latency_info.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ enum LatencyComponentType {
// BEGIN COMPONENT is when we show the latency begin in chrome://tracing.
// Timestamp when the input event is sent from RenderWidgetHost to renderer.
INPUT_EVENT_LATENCY_BEGIN_RWH_COMPONENT,
// In threaded scrolling, main thread scroll listener update is async to
// scroll processing in impl thread. This is the timestamp when we consider
// the main thread scroll listener update is begun.
LATENCY_BEGIN_SCROLL_LISTENER_UPDATE_MAIN_COMPONENT,
// ---------------------------NORMAL COMPONENT-------------------------------
// The original timestamp of the touch event which converts to scroll update.
INPUT_EVENT_LATENCY_SCROLL_UPDATE_ORIGINAL_COMPONENT,
Expand All @@ -33,8 +29,6 @@ enum LatencyComponentType {
// This is special component indicating there is rendering scheduled for
// the event associated with this LatencyInfo on impl thread.
INPUT_EVENT_LATENCY_RENDERING_SCHEDULED_IMPL_COMPONENT,
// Timestamp when a scroll update is forwarded to the main thread.
INPUT_EVENT_LATENCY_FORWARD_SCROLL_UPDATE_TO_MAIN_COMPONENT,
// Timestamp for last event that has been coalesced into this one.
INPUT_EVENT_LATENCY_SCROLL_UPDATE_LAST_EVENT_COMPONENT,
// Timestamp when the event's ack is received by the RWH.
Expand Down
Loading

0 comments on commit 9f31191

Please sign in to comment.