From 0f081c459a326c76da497645ae97c4d7d7bfd750 Mon Sep 17 00:00:00 2001 From: khushalsagar Date: Tue, 15 Dec 2015 23:04:20 -0800 Subject: cc: Fix error in smoothness_priority_expiration_notifier for ProxyImpl The error was introduced by https://codereview.chromium.org/1417053005/ where using TimeDelta::FromSeconds for a double value caused the DelayedUniqueNotifier to be initialized with zero delay. This also fixes the flakiness for BenchmarkSmokeTest.thread_times on Win7 bots, caused by an increase in the number of trace events from the DelayedUniqueNotifier running with zero delay. BUG=568120, 569658 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review URL: https://codereview.chromium.org/1527893002 Cr-Commit-Position: refs/heads/master@{#365492} --- cc/BUILD.gn | 3 ++ cc/base/delayed_unique_notifier.h | 2 ++ cc/cc_tests.gyp | 3 ++ cc/test/fake_channel_impl.cc | 11 +++++++ cc/test/fake_channel_impl.h | 41 ++++++++++++++++++++++++++ cc/test/proxy_impl_for_test.cc | 2 +- cc/test/proxy_impl_for_test.h | 5 +++- cc/trees/proxy_impl.cc | 2 +- cc/trees/proxy_impl_unittest.cc | 60 +++++++++++++++++++++++++++++++++++++++ 9 files changed, 126 insertions(+), 3 deletions(-) create mode 100644 cc/test/fake_channel_impl.cc create mode 100644 cc/test/fake_channel_impl.h create mode 100644 cc/trees/proxy_impl_unittest.cc diff --git a/cc/BUILD.gn b/cc/BUILD.gn index fb77e98..0b5a4af 100644 --- a/cc/BUILD.gn +++ b/cc/BUILD.gn @@ -570,6 +570,8 @@ source_set("test_support") { "test/begin_frame_source_test.h", "test/failure_output_surface.cc", "test/failure_output_surface.h", + "test/fake_channel_impl.cc", + "test/fake_channel_impl.h", "test/fake_content_layer_client.cc", "test/fake_content_layer_client.h", "test/fake_delegated_renderer_layer.cc", @@ -891,6 +893,7 @@ test("cc_unittests") { "trees/occlusion_tracker_unittest.cc", "trees/occlusion_unittest.cc", "trees/property_tree_unittest.cc", + "trees/proxy_impl_unittest.cc", "trees/threaded_channel_unittest.cc", "trees/tree_synchronizer_unittest.cc", diff --git a/cc/base/delayed_unique_notifier.h b/cc/base/delayed_unique_notifier.h index 02051be..0e461fe 100644 --- a/cc/base/delayed_unique_notifier.h +++ b/cc/base/delayed_unique_notifier.h @@ -45,6 +45,8 @@ class CC_EXPORT DelayedUniqueNotifier { // Returns true if a notification is currently scheduled to run. bool HasPendingNotification() const; + base::TimeDelta delay() const { return delay_; } + protected: // Virtual for testing. virtual base::TimeTicks Now() const; diff --git a/cc/cc_tests.gyp b/cc/cc_tests.gyp index d98439c..ddf3e8d 100644 --- a/cc/cc_tests.gyp +++ b/cc/cc_tests.gyp @@ -149,6 +149,7 @@ 'trees/occlusion_tracker_unittest.cc', 'trees/occlusion_unittest.cc', 'trees/property_tree_unittest.cc', + 'trees/proxy_impl_unittest.cc', 'trees/threaded_channel_unittest.cc', 'trees/tree_synchronizer_unittest.cc', ], @@ -173,6 +174,8 @@ 'test/begin_frame_source_test.h', 'test/failure_output_surface.cc', 'test/failure_output_surface.h', + 'test/fake_channel_impl.cc', + 'test/fake_channel_impl.h', 'test/fake_content_layer_client.cc', 'test/fake_content_layer_client.h', 'test/fake_delegated_renderer_layer.cc', diff --git a/cc/test/fake_channel_impl.cc b/cc/test/fake_channel_impl.cc new file mode 100644 index 0000000..8b609d4 --- /dev/null +++ b/cc/test/fake_channel_impl.cc @@ -0,0 +1,11 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "cc/test/fake_channel_impl.h" + +namespace cc { + +FakeChannelImpl::FakeChannelImpl() {} + +} // namespace cc diff --git a/cc/test/fake_channel_impl.h b/cc/test/fake_channel_impl.h new file mode 100644 index 0000000..32716ea --- /dev/null +++ b/cc/test/fake_channel_impl.h @@ -0,0 +1,41 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CC_TEST_FAKE_CHANNEL_IMPL_H_ +#define CC_TEST_FAKE_CHANNEL_IMPL_H_ + +#include "base/macros.h" +#include "cc/trees/channel_impl.h" + +namespace cc { + +class FakeChannelImpl : public ChannelImpl { + public: + FakeChannelImpl(); + + ~FakeChannelImpl() override {} + + void DidCompleteSwapBuffers() override {} + void SetRendererCapabilitiesMainCopy( + const RendererCapabilities& capabilities) override {} + void BeginMainFrameNotExpectedSoon() override {} + void DidCommitAndDrawFrame() override {} + void SetAnimationEvents(scoped_ptr queue) override {} + void DidLoseOutputSurface() override {} + void RequestNewOutputSurface() override {} + void DidInitializeOutputSurface( + bool success, + const RendererCapabilities& capabilities) override {} + void DidCompletePageScaleAnimation() override {} + void PostFrameTimingEventsOnMain( + scoped_ptr composite_events, + scoped_ptr main_frame_events) + override {} + void BeginMainFrame(scoped_ptr + begin_main_frame_state) override {} +}; + +} // namespace cc + +#endif // CC_TEST_FAKE_CHANNEL_IMPL_H_ diff --git a/cc/test/proxy_impl_for_test.cc b/cc/test/proxy_impl_for_test.cc index 3cd542b..457bb32 100644 --- a/cc/test/proxy_impl_for_test.cc +++ b/cc/test/proxy_impl_for_test.cc @@ -5,7 +5,7 @@ #include "cc/test/proxy_impl_for_test.h" namespace cc { -scoped_ptr ProxyImplForTest::Create( +scoped_ptr ProxyImplForTest::Create( TestHooks* test_hooks, ChannelImpl* channel_impl, LayerTreeHost* layer_tree_host, diff --git a/cc/test/proxy_impl_for_test.h b/cc/test/proxy_impl_for_test.h index b81ece5..ce96154 100644 --- a/cc/test/proxy_impl_for_test.h +++ b/cc/test/proxy_impl_for_test.h @@ -14,7 +14,7 @@ namespace cc { // actions. class ProxyImplForTest : public ProxyImpl { public: - static scoped_ptr Create( + static scoped_ptr Create( TestHooks* test_hooks, ChannelImpl* channel_impl, LayerTreeHost* layer_tree_host, @@ -28,6 +28,9 @@ class ProxyImplForTest : public ProxyImpl { bool HasCommitCompletionEvent() const; bool GetNextCommitWaitsForActivation() const; + const DelayedUniqueNotifier& smoothness_priority_expiration_notifier() const { + return smoothness_priority_expiration_notifier_; + } ProxyImplForTest(TestHooks* test_hooks, ChannelImpl* channel_impl, diff --git a/cc/trees/proxy_impl.cc b/cc/trees/proxy_impl.cc index 810e73e..0ba4cc2 100644 --- a/cc/trees/proxy_impl.cc +++ b/cc/trees/proxy_impl.cc @@ -57,7 +57,7 @@ ProxyImpl::ProxyImpl(ChannelImpl* channel_impl, smoothness_priority_expiration_notifier_( task_runner_provider->ImplThreadTaskRunner(), base::Bind(&ProxyImpl::RenewTreePriority, base::Unretained(this)), - base::TimeDelta::FromSeconds( + base::TimeDelta::FromSecondsD( kSmoothnessTakesPriorityExpirationDelay)), external_begin_frame_source_(std::move(external_begin_frame_source)), rendering_stats_instrumentation_( diff --git a/cc/trees/proxy_impl_unittest.cc b/cc/trees/proxy_impl_unittest.cc new file mode 100644 index 0000000..ad99d53 --- /dev/null +++ b/cc/trees/proxy_impl_unittest.cc @@ -0,0 +1,60 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "cc/test/fake_channel_impl.h" +#include "cc/test/fake_layer_tree_host.h" +#include "cc/test/fake_layer_tree_host_client.h" +#include "cc/test/proxy_impl_for_test.h" +#include "cc/test/test_hooks.h" +#include "cc/test/test_task_graph_runner.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace cc { + +class ProxyImplTest : public testing::Test, public TestHooks { + public: + ~ProxyImplTest() override { + DebugScopedSetImplThreadAndMainThreadBlocked impl_and_main_blocked( + task_runner_provider_); + proxy_impl_.reset(); + } + + void RequestNewOutputSurface() override {} + + protected: + ProxyImplTest() + : host_client_(FakeLayerTreeHostClient::DIRECT_3D), + task_runner_provider_(nullptr) {} + + void Initialize(CompositorMode mode) { + layer_tree_host_ = FakeLayerTreeHost::Create( + &host_client_, &task_graph_runner_, settings_, mode); + task_runner_provider_ = layer_tree_host_->task_runner_provider(); + { + DebugScopedSetImplThreadAndMainThreadBlocked impl_and_main_blocked( + task_runner_provider_); + proxy_impl_ = + ProxyImplForTest::Create(this, &channel_impl_, layer_tree_host_.get(), + task_runner_provider_, nullptr); + } + } + + TestTaskGraphRunner task_graph_runner_; + LayerTreeSettings settings_; + FakeLayerTreeHostClient host_client_; + FakeChannelImpl channel_impl_; + TaskRunnerProvider* task_runner_provider_; + scoped_ptr proxy_impl_; + scoped_ptr layer_tree_host_; +}; + +// This is a regression test. See crbug/568120. +TEST_F(ProxyImplTest, NonZeroSmoothnessPriorityExpiration) { + Initialize(CompositorMode::Threaded); + DebugScopedSetImplThread impl_thread(task_runner_provider_); + EXPECT_FALSE( + proxy_impl_->smoothness_priority_expiration_notifier().delay().is_zero()); +} + +} // namespace cc -- cgit v1.1