Get rid of virtual Display::CreateScheduler.

cc/surfaces/display.cc

Index: cc/surfaces/display.cc
diff --git a/cc/surfaces/display.cc b/cc/surfaces/display.cc
index 1619092afda73a44963b0b8cec69cd1d8264bcb2..8d41cb2a22a4a04aafddd5454cebcd6bcec258b3 100644
--- a/cc/surfaces/display.cc
+++ b/cc/surfaces/display.cc
@@ -47,16 +47,63 @@ Display::Display(SurfaceManager* manager,
                  const RendererSettings& settings,
                  uint32_t compositor_surface_namespace,
                  base::SingleThreadTaskRunner* task_runner,
+                 BeginFrameSource* begin_frame_source,
                  std::unique_ptr<OutputSurface> output_surface)
     : surface_manager_(manager),
       bitmap_manager_(bitmap_manager),
       gpu_memory_buffer_manager_(gpu_memory_buffer_manager),
       settings_(settings),
       compositor_surface_namespace_(compositor_surface_namespace),
-      task_runner_(task_runner),
       output_surface_(std::move(output_surface)),
       texture_mailbox_deleter_(task_runner) {
   surface_manager_->AddObserver(this);
+
+  if (begin_frame_source) {

[enne (OOO), 2016/06/02 18:15:20]

I'm not sure I understand this logic.  Should this conditional line just be
removed?

[danakj, 2016/06/02 18:27:21]

Oh, this replaces the InitializeSynchronous(), see the comment in the header
file. When this is null, no scheduler is created.

[enne (OOO), 2016/06/02 18:58:09]

Ohhhh ok ok, then can you remove the DCHECKs about begin frame source inside the
block?

[danakj, 2016/06/03 01:28:35]

Soo... why remove 2 DCHECKs when we can change who creates and owns everything?
YAS.

So now the creator of Display makes:
- BeginFrameSource
- DisplayScheduler
- OutputSurface
- TextureMailboxDeleter

They pass ownership of all 4 to Display. Now Display doesn't need to know about
TaskRunner.

A lot of changes in this patchset though, so PTAL everyone.

+    if (!task_runner) {
+      // WebView doesn't have a task runner or a real begin frame source,
+      // so just create something fake here.
+      DCHECK(!begin_frame_source);
+      internal_begin_frame_source_.reset(new EmptyBeginFrameSource());
+      vsync_begin_frame_source_ = internal_begin_frame_source_.get();
+      observed_begin_frame_source_ = vsync_begin_frame_source_;
+    } else {
+      DCHECK(begin_frame_source);
+      vsync_begin_frame_source_ = begin_frame_source;
+      observed_begin_frame_source_ = vsync_begin_frame_source_;
+      if (settings_.disable_display_vsync) {
+        internal_begin_frame_source_.reset(
+            new BackToBackBeginFrameSource(task_runner));
+        observed_begin_frame_source_ = internal_begin_frame_source_.get();
+      }
+    }
+
+    scheduler_.reset(new DisplayScheduler(
+        this, observed_begin_frame_source_, task_runner,
+        output_surface_->capabilities().max_frames_pending));
+    surface_manager_->RegisterBeginFrameSource(observed_begin_frame_source_,
+                                               compositor_surface_namespace_);
+  }
+}
+
+Display::Display(SurfaceManager* manager,
+                 SharedBitmapManager* bitmap_manager,
+                 gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager,
+                 const RendererSettings& settings,
+                 uint32_t compositor_surface_namespace,
+                 base::SingleThreadTaskRunner* task_runner,
+                 BeginFrameSource* begin_frame_source,
+                 std::unique_ptr<OutputSurface> output_surface,
+                 std::unique_ptr<DisplayScheduler> display_scheduler_for_tests)
+    : Display(manager,
+              bitmap_manager,
+              gpu_memory_buffer_manager,
+              settings,
+              compositor_surface_namespace,
+              task_runner,
+              begin_frame_source,
+              std::move(output_surface)) {
+  DCHECK_EQ(!!begin_frame_source, !!display_scheduler_for_tests);
+  scheduler_ = std::move(display_scheduler_for_tests);
 }
 
 Display::~Display() {
@@ -72,45 +119,10 @@ Display::~Display() {
   }
 }
 
-void Display::CreateScheduler() {
-  DCHECK(!scheduler_);
-  if (!task_runner_) {
-    // WebView doesn't have a task runner or a real begin frame source,
-    // so just create something fake here.
-    internal_begin_frame_source_.reset(new EmptyBeginFrameSource());
-    vsync_begin_frame_source_ = internal_begin_frame_source_.get();
-    observed_begin_frame_source_ = vsync_begin_frame_source_;
-  } else {
-    DCHECK(vsync_begin_frame_source_);
-
-    observed_begin_frame_source_ = vsync_begin_frame_source_;
-    if (settings_.disable_display_vsync) {
-      internal_begin_frame_source_.reset(
-          new BackToBackBeginFrameSource(task_runner_));
-      observed_begin_frame_source_ = internal_begin_frame_source_.get();
-    }
-  }
-
-  scheduler_.reset(
-      new DisplayScheduler(this, observed_begin_frame_source_, task_runner_,
-                           output_surface_->capabilities().max_frames_pending));
-  surface_manager_->RegisterBeginFrameSource(observed_begin_frame_source_,
-                                             compositor_surface_namespace_);
-}
-
 bool Display::Initialize(DisplayClient* client) {
   client_ = client;
   if (!output_surface_->BindToClient(this))
     return false;
-  CreateScheduler();
-  return true;
-}
-
-bool Display::InitializeSynchronous(DisplayClient* client) {
-  client_ = client;
-  if (!output_surface_->BindToClient(this))
-    return false;
-  // No scheduler created here.
   return true;
 }
 
@@ -354,15 +366,16 @@ void Display::DidSwapBuffersComplete() {
     renderer_->SwapBuffersComplete();
 }
 
+void Display::CommitVSyncParameters(base::TimeTicks timebase,
+                                    base::TimeDelta interval) {
+  // Display uses a BeginFrameSource instead.
+  NOTREACHED();
+}
+
 void Display::SetBeginFrameSource(BeginFrameSource* source) {
-  // It's expected that there's only a single source from the
-  // BrowserCompositorOutputSurface that corresponds to vsync.  The BFS is
-  // passed BrowserCompositorOutputSurface -> Display -> DisplayScheduler as an
-  // input.  DisplayScheduler makes a decision about which BFS to use and
-  // calls back to Display as DisplaySchedulerClient to register for that
-  // surface id.
-  DCHECK(!vsync_begin_frame_source_);
-  vsync_begin_frame_source_ = source;
+  // The BeginFrameSource is set from the constructor, it doesn't come
+  // from the OutputSurface for the Display.
+  NOTREACHED();
 }
 
 void Display::SetMemoryPolicy(const ManagedMemoryPolicy& policy) {