Get rid of virtual Display::CreateScheduler.

cc/surfaces/display.cc

 // Copyright 2014 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/surfaces/display.h"
 
 #include <stddef.h>
 
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/trace_event.h"
@@ skipping 29 matching lines @@
 }  // namespace
 
 namespace cc {
 
 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)
     : 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() {
   if (observed_begin_frame_source_)
     surface_manager_->UnregisterBeginFrameSource(observed_begin_frame_source_);
   surface_manager_->RemoveObserver(this);
   if (aggregator_) {
     for (const auto& id_entry : aggregator_->previous_contained_surfaces()) {
       Surface* surface = surface_manager_->GetSurfaceForId(id_entry.first);
       if (surface)
         surface->RunDrawCallbacks(SurfaceDrawStatus::DRAW_SKIPPED);
     }
   }
 }
 
-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;
-}
-
 void Display::SetSurfaceId(SurfaceId id, float device_scale_factor) {
   DCHECK_EQ(id.id_namespace(), compositor_surface_namespace_);
   if (current_surface_id_ == id && device_scale_factor_ == device_scale_factor)
     return;
 
   TRACE_EVENT0("cc", "Display::SetSurfaceId");
   current_surface_id_ = id;
   device_scale_factor_ = device_scale_factor;
 
   UpdateRootSurfaceResourcesLocked();
@@ skipping 220 matching lines @@
     scheduler_->DidSwapBuffers();
 }
 
 void Display::DidSwapBuffersComplete() {
   if (scheduler_)
     scheduler_->DidSwapBuffersComplete();
   if (renderer_)
     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) {
   client_->DisplaySetMemoryPolicy(policy);
 }
 
 void Display::OnDraw(const gfx::Transform& transform,
                      const gfx::Rect& viewport,
                      const gfx::Rect& clip,
                      bool resourceless_software_draw) {
@@ skipping 47 matching lines @@
 
   if (surface_id == current_surface_id_)
     UpdateRootSurfaceResourcesLocked();
 }
 
 SurfaceId Display::CurrentSurfaceId() {
   return current_surface_id_;
 }
 
 }  // namespace cc