cc: Make SingleThreadProxy ignore commit requests inside Layout().

This makes the SingleThreadProxy match the behaviour of ThreadProxy and
ensures that when the embedder is doing Layout() if it changes things
that would cause a commit, those things will already happen in the
upcoming commit so it does not schedule a second commit afterward.

R=enne,vmpstr
BUG=466426

Review URL: https://codereview.chromium.org/1061993009

Cr-Commit-Position: refs/heads/master@{#326884}

cc/trees/layer_tree_host_unittest.cc

@@ -86,6 +86,46 @@ class LayerTreeHostTest : public LayerTreeTest {
   PrioritizedResourceManager* contents_texture_manager_;
 };
 
+class LayerTreeHostTestSetNeedsCommitInsideLayout : public LayerTreeHostTest {
+ protected:
+  void BeginTest() override { PostSetNeedsCommitToMainThread(); }
+
+  void Layout() override {
+    // This shouldn't cause a second commit to happen.
+    layer_tree_host()->SetNeedsCommit();
+  }
+
+  void DidCommit() override {
+    EXPECT_EQ(1, layer_tree_host()->source_frame_number());
+    EndTest();
+  }
+
+  void AfterTest() override {}
+};
+
+SINGLE_AND_MULTI_THREAD_IMPL_TEST_F(
+    LayerTreeHostTestSetNeedsCommitInsideLayout);
+
+class LayerTreeHostTestSetNeedsUpdateInsideLayout : public LayerTreeHostTest {
+ protected:
+  void BeginTest() override { PostSetNeedsCommitToMainThread(); }
+
+  void Layout() override {
+    // This shouldn't cause a second commit to happen.
+    layer_tree_host()->SetNeedsUpdateLayers();
+  }
+
+  void DidCommit() override {
+    EXPECT_EQ(1, layer_tree_host()->source_frame_number());
+    EndTest();
+  }
+
+  void AfterTest() override {}
+};
+
+SINGLE_AND_MULTI_THREAD_IMPL_TEST_F(
+    LayerTreeHostTestSetNeedsUpdateInsideLayout);
+
 // Test if the LTHI receives ReadyToActivate notifications from the TileManager
 // when no raster tasks get scheduled.
 class LayerTreeHostTestReadyToActivateEmpty : public LayerTreeHostTest {

cc/trees/single_thread_proxy.cc

@@ -50,11 +50,21 @@ SingleThreadProxy::SingleThreadProxy(
       commit_requested_(false),
       inside_synchronous_composite_(false),
       output_surface_creation_requested_(false),
-      external_begin_frame_source_(external_begin_frame_source.Pass()),
       weak_factory_(this) {
   TRACE_EVENT0("cc", "SingleThreadProxy::SingleThreadProxy");
   DCHECK(Proxy::IsMainThread());
   DCHECK(layer_tree_host);
+
+  if (layer_tree_host->settings().single_thread_proxy_scheduler &&
+      !scheduler_on_impl_thread_) {
+    SchedulerSettings scheduler_settings(
+        layer_tree_host->settings().ToSchedulerSettings());
+    // SingleThreadProxy should run in main thread low latency mode.
+    scheduler_settings.main_thread_should_always_be_low_latency = true;
+    scheduler_on_impl_thread_ = Scheduler::Create(
+        this, scheduler_settings, layer_tree_host_->id(),
+        MainThreadTaskRunner(), external_begin_frame_source.Pass());
+  }
 }
 
 void SingleThreadProxy::Start() {
@@ -95,18 +105,7 @@ void SingleThreadProxy::SetLayerTreeHostClientReady() {
   // nothing to do.
   DCHECK(Proxy::IsMainThread());
   DebugScopedSetImplThread impl(this);
-  if (layer_tree_host_->settings().single_thread_proxy_scheduler &&
-      !scheduler_on_impl_thread_) {
-    SchedulerSettings scheduler_settings(
-        layer_tree_host_->settings().ToSchedulerSettings());
-    // SingleThreadProxy should run in main thread low latency mode.
-    scheduler_settings.main_thread_should_always_be_low_latency = true;
-    scheduler_on_impl_thread_ =
-        Scheduler::Create(this,
-                          scheduler_settings,
-                          layer_tree_host_->id(),
-                          MainThreadTaskRunner(),
-                          external_begin_frame_source_.Pass());
+  if (scheduler_on_impl_thread_) {
     scheduler_on_impl_thread_->SetCanStart();
     scheduler_on_impl_thread_->SetVisible(layer_tree_host_impl_->visible());
   }
@@ -214,7 +213,6 @@ void SingleThreadProxy::DoCommit() {
   // fixed.
   tracked_objects::ScopedTracker tracking_profile1(
       FROM_HERE_WITH_EXPLICIT_FUNCTION("461509 SingleThreadProxy::DoCommit1"));
-  commit_requested_ = false;
   layer_tree_host_->WillCommit();
   devtools_instrumentation::ScopedCommitTrace commit_task(
       layer_tree_host_->id());
@@ -334,8 +332,10 @@ void SingleThreadProxy::CommitComplete() {
 
 void SingleThreadProxy::SetNeedsCommit() {
   DCHECK(Proxy::IsMainThread());
-  DebugScopedSetImplThread impl(this);
   client_->ScheduleComposite();
+  if (commit_requested_)
+    return;
+  DebugScopedSetImplThread impl(this);
   if (scheduler_on_impl_thread_)
     scheduler_on_impl_thread_->SetNeedsCommit();
   commit_requested_ = true;
@@ -766,7 +766,11 @@ void SingleThreadProxy::DidCommitAndDrawFrame() {
 }
 
 bool SingleThreadProxy::MainFrameWillHappenForTesting() {
-  return false;
+  if (layer_tree_host_->output_surface_lost())
+    return false;
+  if (!scheduler_on_impl_thread_)
+    return false;
+  return scheduler_on_impl_thread_->MainFrameForTestingWillHappen();
 }
 
 void SingleThreadProxy::SetChildrenNeedBeginFrames(
@@ -804,6 +808,8 @@ void SingleThreadProxy::SendBeginMainFrameNotExpectedSoon() {
 }
 
 void SingleThreadProxy::BeginMainFrame() {
+  commit_requested_ = false;
+
   if (defer_commits_) {
     TRACE_EVENT_INSTANT0("cc", "EarlyOut_DeferCommit",
                          TRACE_EVENT_SCOPE_THREAD);
@@ -831,6 +837,9 @@ void SingleThreadProxy::BeginMainFrame() {
     return;
   }
 
+  // Prevent new commits from being requested inside DoBeginMainFrame.
+  commit_requested_ = true;
+
   const BeginFrameArgs& begin_frame_args =
       layer_tree_host_impl_->CurrentBeginFrameArgs();
   DoBeginMainFrame(begin_frame_args);
@@ -855,6 +864,9 @@ void SingleThreadProxy::DoBeginMainFrame(
   DCHECK(!queue_for_commit_);
   queue_for_commit_ = make_scoped_ptr(new ResourceUpdateQueue);
 
+  // New commits requested inside UpdateLayers should be respected.
+  commit_requested_ = false;
+
   layer_tree_host_->UpdateLayers(queue_for_commit_.get());
 
   timing_history_.DidBeginMainFrame();

cc/trees/single_thread_proxy.h

@@ -172,8 +172,6 @@ class CC_EXPORT SingleThreadProxy : public Proxy,
   // This is the callback for the scheduled RequestNewOutputSurface.
   base::CancelableClosure output_surface_creation_callback_;
 
-  scoped_ptr<BeginFrameSource> external_begin_frame_source_;
-
   base::WeakPtrFactory<SingleThreadProxy> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SingleThreadProxy);

content/renderer/gpu/render_widget_compositor_unittest.cc

@@ -57,6 +57,7 @@ class RenderWidgetCompositorTest : public testing::Test {
   ~RenderWidgetCompositorTest() override {}
 
  protected:
+  base::MessageLoop loop_;
   MockRenderThread render_thread_;
   scoped_refptr<TestRenderWidget> render_widget_;
   scoped_ptr<FakeCompositorDependencies> compositor_deps_;