Revert 287747 "Make SingleThreadProxy a SchedulerClient"

This is a speculative revert to see if issues on Mac bots get resolved.

BrowserTest.WindowOpenClose is e.g. failing on this assert:
ASSERTION FAILED: !active || m_layerTreeView

> Make SingleThreadProxy a SchedulerClient
> 
> This makes ui::Compositor no longer in charge of
> scheduling commits and draws, deferring it to cc::Scheduler.
> 
> Other compositors that use SingleThreadProxy are left calling composite
> synchronously and now pass a flag to indicate that this is their
> intention.  This patch doesn't remove synchronous composite, but now
> makes it mutually exclusive with scheduling.
> 
> BUG=329552, 287250
> 
> Review URL: https://codereview.chromium.org/134623005

TBR=enne@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287766 0039d316-1c4b-4281-b951-d872f2087c98

android_webview/browser/hardware_renderer.cc

@@ -93,11 +93,8 @@ HardwareRenderer::HardwareRenderer(SharedRendererState* state)
   // Webview does not own the surface so should not clear it.
   settings.should_clear_root_render_pass = false;
 
-  // TODO(enne): Update this this compositor to use a synchronous scheduler.
-  settings.single_thread_proxy_scheduler = false;
-
-  layer_tree_host_ =
-      cc::LayerTreeHost::CreateSingleThreaded(this, this, NULL, settings, NULL);
+  layer_tree_host_ = cc::LayerTreeHost::CreateSingleThreaded(
+      this, this, NULL, settings, NULL);
   layer_tree_host_->SetRootLayer(root_layer_);
   layer_tree_host_->SetLayerTreeHostClientReady();
   layer_tree_host_->set_has_transparent_background(true);

android_webview/browser/hardware_renderer.h

@@ -54,6 +54,8 @@ class HardwareRenderer : public cc::LayerTreeHostClient,
   virtual void DidCompleteSwapBuffers() OVERRIDE {}
 
   // cc::LayerTreeHostSingleThreadClient overrides.
+  virtual void ScheduleComposite() OVERRIDE {}
+  virtual void ScheduleAnimation() OVERRIDE {}
   virtual void DidPostSwapBuffers() OVERRIDE {}
   virtual void DidAbortSwapBuffers() OVERRIDE {}
 

cc/scheduler/scheduler.cc

@@ -79,11 +79,11 @@ Scheduler::Scheduler(
     SchedulerClient* client,
     const SchedulerSettings& scheduler_settings,
     int layer_tree_host_id,
-    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner)
+    const scoped_refptr<base::SingleThreadTaskRunner>& impl_task_runner)
     : settings_(scheduler_settings),
       client_(client),
       layer_tree_host_id_(layer_tree_host_id),
-      task_runner_(task_runner),
+      impl_task_runner_(impl_task_runner),
       vsync_interval_(BeginFrameArgs::DefaultInterval()),
       last_set_needs_begin_frame_(false),
       begin_unthrottled_frame_posted_(false),
@@ -128,7 +128,7 @@ Scheduler::~Scheduler() {
 void Scheduler::SetupSyntheticBeginFrames() {
   DCHECK(!synthetic_begin_frame_source_);
   synthetic_begin_frame_source_.reset(
-      new SyntheticBeginFrameSource(this, task_runner_.get()));
+      new SyntheticBeginFrameSource(this, impl_task_runner_.get()));
 }
 
 void Scheduler::CommitVSyncParameters(base::TimeTicks timebase,
@@ -274,9 +274,6 @@ base::TimeTicks Scheduler::LastBeginImplFrameTime() {
 }
 
 void Scheduler::SetupNextBeginFrameIfNeeded() {
-  if (!task_runner_)
-    return;
-
   bool needs_begin_frame = state_machine_.BeginFrameNeeded();
 
   if (settings_.throttle_frame_production) {
@@ -331,7 +328,7 @@ void Scheduler::SetupNextBeginFrameWhenVSyncThrottlingDisabled(
   }
 
   begin_unthrottled_frame_posted_ = true;
-  task_runner_->PostTask(FROM_HERE, begin_unthrottled_frame_closure_);
+  impl_task_runner_->PostTask(FROM_HERE, begin_unthrottled_frame_closure_);
 }
 
 // BeginUnthrottledFrame is used when we aren't throttling frame production.
@@ -365,7 +362,7 @@ void Scheduler::SetupPollingMechanisms(bool needs_begin_frame) {
       base::TimeDelta delay = begin_impl_frame_args_.IsValid()
                                   ? begin_impl_frame_args_.interval
                                   : BeginFrameArgs::DefaultInterval();
-      task_runner_->PostDelayedTask(
+      impl_task_runner_->PostDelayedTask(
           FROM_HERE, poll_for_draw_triggers_task_.callback(), delay);
     }
   } else {
@@ -390,9 +387,9 @@ void Scheduler::SetupPollingMechanisms(bool needs_begin_frame) {
       // Since we'd rather get a BeginImplFrame by the normal mechanism, we
       // set the interval to twice the interval from the previous frame.
       advance_commit_state_task_.Reset(advance_commit_state_closure_);
-      task_runner_->PostDelayedTask(FROM_HERE,
-                                    advance_commit_state_task_.callback(),
-                                    begin_impl_frame_args_.interval * 2);
+      impl_task_runner_->PostDelayedTask(FROM_HERE,
+                                         advance_commit_state_task_.callback(),
+                                         begin_impl_frame_args_.interval * 2);
     }
   } else {
     advance_commit_state_task_.Cancel();
@@ -494,16 +491,16 @@ void Scheduler::PostBeginRetroFrameIfNeeded() {
     return;
 
   begin_retro_frame_posted_ = true;
-  task_runner_->PostTask(FROM_HERE, begin_retro_frame_closure_);
+  impl_task_runner_->PostTask(FROM_HERE, begin_retro_frame_closure_);
 }
 
 // BeginImplFrame starts a compositor frame that will wait up until a deadline
 // for a BeginMainFrame+activation to complete before it times out and draws
 // any asynchronous animation and scroll/pinch updates.
 void Scheduler::BeginImplFrame(const BeginFrameArgs& args) {
   TRACE_EVENT1("cc", "Scheduler::BeginImplFrame", "args", args.AsValue());
-  DCHECK_EQ(state_machine_.begin_impl_frame_state(),
-            SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE);
+  DCHECK(state_machine_.begin_impl_frame_state() ==
+         SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE);
   DCHECK(state_machine_.HasInitializedOutputSurface());
 
   advance_commit_state_task_.Cancel();
@@ -570,7 +567,7 @@ void Scheduler::ScheduleBeginImplFrameDeadline(base::TimeTicks deadline) {
   base::TimeDelta delta = deadline - gfx::FrameTime::Now();
   if (delta <= base::TimeDelta())
     delta = base::TimeDelta();
-  task_runner_->PostDelayedTask(
+  impl_task_runner_->PostDelayedTask(
       FROM_HERE, begin_impl_frame_deadline_task_.callback(), delta);
 }
 

cc/scheduler/scheduler.h

@@ -57,9 +57,9 @@ class CC_EXPORT Scheduler {
       SchedulerClient* client,
       const SchedulerSettings& scheduler_settings,
       int layer_tree_host_id,
-      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
+      const scoped_refptr<base::SingleThreadTaskRunner>& impl_task_runner) {
     return make_scoped_ptr(new Scheduler(
-        client, scheduler_settings, layer_tree_host_id, task_runner));
+        client, scheduler_settings, layer_tree_host_id, impl_task_runner));
   }
 
   virtual ~Scheduler();
@@ -173,15 +173,16 @@ class CC_EXPORT Scheduler {
     scoped_refptr<DelayBasedTimeSource> time_source_;
   };
 
-  Scheduler(SchedulerClient* client,
-            const SchedulerSettings& scheduler_settings,
-            int layer_tree_host_id,
-            const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
+  Scheduler(
+      SchedulerClient* client,
+      const SchedulerSettings& scheduler_settings,
+      int layer_tree_host_id,
+      const scoped_refptr<base::SingleThreadTaskRunner>& impl_task_runner);
 
   const SchedulerSettings settings_;
   SchedulerClient* client_;
   int layer_tree_host_id_;
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+  scoped_refptr<base::SingleThreadTaskRunner> impl_task_runner_;
 
   base::TimeDelta vsync_interval_;
   base::TimeDelta estimated_parent_draw_time_;

cc/scheduler/scheduler_state_machine.cc

@@ -1006,9 +1006,7 @@ void SchedulerStateMachine::DidDrawIfPossibleCompleted(DrawResult result) {
   }
 }
 
-void SchedulerStateMachine::SetNeedsCommit() {
-  needs_commit_ = true;
-}
+void SchedulerStateMachine::SetNeedsCommit() { needs_commit_ = true; }
 
 void SchedulerStateMachine::NotifyReadyToCommit() {
   DCHECK(commit_state_ == COMMIT_STATE_BEGIN_MAIN_FRAME_STARTED)

cc/test/fake_layer_tree_host_client.h

@@ -42,6 +42,8 @@ class FakeLayerTreeHostClient : public LayerTreeHostClient,
   virtual void DidCompleteSwapBuffers() OVERRIDE {}
 
   // LayerTreeHostSingleThreadClient implementation.
+  virtual void ScheduleComposite() OVERRIDE {}
+  virtual void ScheduleAnimation() OVERRIDE {}
   virtual void DidPostSwapBuffers() OVERRIDE {}
   virtual void DidAbortSwapBuffers() OVERRIDE {}
 

cc/test/layer_tree_test.cc

@@ -312,6 +312,14 @@ class LayerTreeHostClientForTesting : public LayerTreeHostClient,
     test_hooks_->DidCompleteSwapBuffers();
   }
 
+  virtual void ScheduleComposite() OVERRIDE {
+    test_hooks_->ScheduleComposite();
+  }
+
+  virtual void ScheduleAnimation() OVERRIDE {
+    test_hooks_->ScheduleAnimation();
+  }
+
   virtual void DidPostSwapBuffers() OVERRIDE {}
   virtual void DidAbortSwapBuffers() OVERRIDE {}
 
@@ -386,6 +394,7 @@ LayerTreeTest::LayerTreeTest()
       end_when_begin_returns_(false),
       timed_out_(false),
       scheduled_(false),
+      schedule_when_set_visible_true_(false),
       started_(false),
       ended_(false),
       delegating_renderer_(false),
@@ -549,6 +558,15 @@ void LayerTreeTest::Timeout() {
   EndTest();
 }
 
+void LayerTreeTest::ScheduleComposite() {
+  if (!started_ || scheduled_)
+    return;
+  scheduled_ = true;
+  main_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&LayerTreeTest::DispatchComposite, main_thread_weak_ptr_));
+}
+
 void LayerTreeTest::RealEndTest() {
   if (layer_tree_host_ && proxy()->CommitPendingForTesting()) {
     main_task_runner_->PostTask(
@@ -600,8 +618,16 @@ void LayerTreeTest::DispatchSetNeedsRedrawRect(const gfx::Rect& damage_rect) {
 
 void LayerTreeTest::DispatchSetVisible(bool visible) {
   DCHECK(!proxy() || proxy()->IsMainThread());
-  if (layer_tree_host_)
-    layer_tree_host_->SetVisible(visible);
+
+  if (!layer_tree_host_)
+    return;
+
+  layer_tree_host_->SetVisible(visible);
+
+  // If the LTH is being made visible and a previous ScheduleComposite() was
+  // deferred because the LTH was not visible, re-schedule the composite now.
+  if (layer_tree_host_->visible() && schedule_when_set_visible_true_)
+    ScheduleComposite();
 }
 
 void LayerTreeTest::DispatchSetNextCommitForcesRedraw() {
@@ -611,6 +637,24 @@ void LayerTreeTest::DispatchSetNextCommitForcesRedraw() {
     layer_tree_host_->SetNextCommitForcesRedraw();
 }
 
+void LayerTreeTest::DispatchComposite() {
+  scheduled_ = false;
+
+  if (!layer_tree_host_)
+    return;
+
+  // If the LTH is not visible, defer the composite until the LTH is made
+  // visible.
+  if (!layer_tree_host_->visible()) {
+    schedule_when_set_visible_true_ = true;
+    return;
+  }
+
+  schedule_when_set_visible_true_ = false;
+  base::TimeTicks now = gfx::FrameTime::Now();
+  layer_tree_host_->Composite(now);
+}
+
 void LayerTreeTest::RunTest(bool threaded,
                             bool delegating_renderer,
                             bool impl_side_painting) {

cc/test/layer_tree_test.h

@@ -71,6 +71,8 @@ class TestHooks : public AnimationDelegate {
   virtual void DidCommit() {}
   virtual void DidCommitAndDrawFrame() {}
   virtual void DidCompleteSwapBuffers() {}
+  virtual void ScheduleComposite() {}
+  virtual void ScheduleAnimation() {}
   virtual void DidDeferCommit() {}
   virtual void DidSetVisibleOnImplTree(LayerTreeHostImpl* host_impl,
                                        bool visible) {}
@@ -137,6 +139,8 @@ class LayerTreeTest : public testing::Test, public TestHooks {
 
   virtual void InitializeSettings(LayerTreeSettings* settings) {}
 
+  virtual void ScheduleComposite() OVERRIDE;
+
   void RealEndTest();
 
   virtual void DispatchAddAnimation(Layer* layer_to_receive_animation,
@@ -147,6 +151,7 @@ class LayerTreeTest : public testing::Test, public TestHooks {
   void DispatchSetNeedsRedrawRect(const gfx::Rect& damage_rect);
   void DispatchSetVisible(bool visible);
   void DispatchSetNextCommitForcesRedraw();
+  void DispatchComposite();
   void DispatchDidAddAnimation();
 
   virtual void AfterTest() = 0;
@@ -199,6 +204,7 @@ class LayerTreeTest : public testing::Test, public TestHooks {
   bool end_when_begin_returns_;
   bool timed_out_;
   bool scheduled_;
+  bool schedule_when_set_visible_true_;
   bool started_;
   bool ended_;
   bool delegating_renderer_;

cc/trees/layer_tree_host.cc

@@ -697,12 +697,8 @@ void LayerTreeHost::NotifyInputThrottledUntilCommit() {
 
 void LayerTreeHost::Composite(base::TimeTicks frame_begin_time) {
   DCHECK(!proxy_->HasImplThread());
-  // This function is only valid when not using the scheduler.
-  DCHECK(!settings_.single_thread_proxy_scheduler);
   SingleThreadProxy* proxy = static_cast<SingleThreadProxy*>(proxy_.get());
 
-  SetLayerTreeHostClientReady();
-
   if (output_surface_lost_)
     proxy->CreateAndInitializeOutputSurface();
   if (output_surface_lost_)

cc/trees/layer_tree_host_single_thread_client.h

@@ -10,10 +10,10 @@ namespace cc {
 class LayerTreeHostSingleThreadClient {
  public:
   // Request that the client schedule a composite.
-  virtual void ScheduleComposite() {}
+  virtual void ScheduleComposite() = 0;
   // Request that the client schedule a composite now, and calculate appropriate
   // delay for potential future frame.
-  virtual void ScheduleAnimation() {}
+  virtual void ScheduleAnimation() = 0;
 
   // Called whenever the compositor posts a SwapBuffers (either full or
   // partial). After DidPostSwapBuffers(), exactly one of

cc/trees/layer_tree_host_unittest.cc

@@ -91,8 +91,8 @@ class LayerTreeHostTestSetNeedsCommit1 : public LayerTreeHostTest {
   }
 
   virtual void AfterTest() OVERRIDE {
-    EXPECT_LE(1, num_commits_);
-    EXPECT_LE(1, num_draws_);
+    EXPECT_GE(1, num_commits_);
+    EXPECT_GE(1, num_draws_);
   }
 
  private:
@@ -1967,7 +1967,7 @@ class LayerTreeHostTestDeferCommits : public LayerTreeHostTest {
   int num_complete_commits_;
 };
 
-SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestDeferCommits);
+MULTI_THREAD_TEST_F(LayerTreeHostTestDeferCommits);
 
 class LayerTreeHostWithProxy : public LayerTreeHost {
  public:
@@ -2040,7 +2040,6 @@ TEST(LayerTreeHostTest, PartialUpdatesWithGLRenderer) {
 
   LayerTreeSettings settings;
   settings.max_partial_texture_updates = 4;
-  settings.single_thread_proxy_scheduler = false;
 
   scoped_ptr<SharedBitmapManager> shared_bitmap_manager(
       new TestSharedBitmapManager());
@@ -2060,7 +2059,6 @@ TEST(LayerTreeHostTest, PartialUpdatesWithSoftwareRenderer) {
 
   LayerTreeSettings settings;
   settings.max_partial_texture_updates = 4;
-  settings.single_thread_proxy_scheduler = false;
 
   scoped_ptr<SharedBitmapManager> shared_bitmap_manager(
       new TestSharedBitmapManager());
@@ -2080,7 +2078,6 @@ TEST(LayerTreeHostTest, PartialUpdatesWithDelegatingRendererAndGLContent) {
 
   LayerTreeSettings settings;
   settings.max_partial_texture_updates = 4;
-  settings.single_thread_proxy_scheduler = false;
 
   scoped_ptr<SharedBitmapManager> shared_bitmap_manager(
       new TestSharedBitmapManager());
@@ -2101,7 +2098,6 @@ TEST(LayerTreeHostTest,
 
   LayerTreeSettings settings;
   settings.max_partial_texture_updates = 4;
-  settings.single_thread_proxy_scheduler = false;
 
   scoped_ptr<SharedBitmapManager> shared_bitmap_manager(
       new TestSharedBitmapManager());