Revert "viz: Make GpuTaskSchedulerHelper a unique_ptr with Single Own…

…ership"

This reverts commit 65cdf4d.

Reason for revert: crash w/ GLRendererCopier Bug: 1096487

Original change's description:
> viz: Make GpuTaskSchedulerHelper a unique_ptr with Single Ownership
> 
> |gpu::GpuTaskSchedulerHelper| is the interface to post tasks to the gpu
> thread that is shared by the output surface and the overlay processor.
> The ownership used to be managered by a scopred_refptr. Since
> viz::Display owns both the output surface and the overlay processor,
> move the shared gpu task scheduler to be owned by display as well.
> This will make |viz::Display| the single owner of this class, and
> this will make the lifetime of this class easier to understand.
> 
> 
> R=​rjkroege
> TBR=kylechar
> 
> Bug: 1093440
> Change-Id: Ie5f3760ca6c7ee61e9adf6ce56eb3904f1b8d1c4
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2237996
> Commit-Queue: weiliangc <weiliangc@chromium.org>
> Reviewed-by: Bo <boliu@chromium.org>
> Reviewed-by: Khushal <khushalsagar@chromium.org>
> Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#779176}

TBR=rjkroege@chromium.org,boliu@chromium.org,weiliangc@chromium.org,khushalsagar@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1093440
Change-Id: I4eb125e9f75ef38336a395a8c288d3f3ed94f911
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2256005
Reviewed-by: weiliangc <weiliangc@chromium.org>
Commit-Queue: weiliangc <weiliangc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780667}

android_webview/browser/gfx/hardware_renderer_viz.cc

@@ -117,14 +117,11 @@ HardwareRendererViz::OnViz::OnViz(
       std::make_unique<DisplaySchedulerWebView>(without_gpu_.get());
   auto overlay_processor = std::make_unique<viz::OverlayProcessorStub>();
 
-  // Android WebView has no overlay processor, and does not need to share
-  // gpu_task_scheduler, so it is passed in as nullptr.
   display_ = std::make_unique<viz::Display>(
       nullptr /* shared_bitmap_manager */,
       output_surface_provider->renderer_settings(), frame_sink_id_,
-      nullptr /*gpu_task_scheduler */, std::move(output_surface),
-      std::move(overlay_processor), std::move(scheduler),
-      nullptr /* current_task_runner */);
+      std::move(output_surface), std::move(overlay_processor),
+      std::move(scheduler), nullptr /* current_task_runner */);
   display_->Initialize(this, GetFrameSinkManager()->surface_manager(),
                        output_surface_provider->enable_shared_image());
 

android_webview/browser/gfx/output_surface_provider_webview.cc

@@ -20,7 +20,6 @@
 #include "components/viz/common/features.h"
 #include "components/viz/service/display_embedder/skia_output_surface_impl.h"
 #include "gpu/config/gpu_finch_features.h"
-#include "gpu/ipc/single_task_sequence.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_share_group.h"
 #include "ui/gl/init/gl_factory.h"
@@ -126,12 +125,8 @@ OutputSurfaceProviderWebview::CreateOutputSurface() {
     auto skia_dependency = std::make_unique<SkiaOutputSurfaceDependencyWebView>(
         TaskQueueWebView::GetInstance(), GpuServiceWebView::GetInstance(),
         shared_context_state_.get(), gl_surface_.get());
-    // We are not passing in a gpu_task_scheduler here, so SkiaOutputSurface
-    // will create one from its skia_dependency. This is because ANdroid WebView
-    // does not have overlay and do not need to share the gpu_task_scheduler
-    // with OverlayProcessor.
     return viz::SkiaOutputSurfaceImpl::Create(std::move(skia_dependency),
-                                              nullptr, renderer_settings_);
+                                              renderer_settings_);
   } else {
     auto context_provider = AwRenderThreadContextProvider::Create(
         gl_surface_, DeferredGpuCommandService::GetInstance());

android_webview/browser/gfx/output_surface_provider_webview.h

@@ -11,7 +11,6 @@
 #include "base/memory/ref_counted.h"
 #include "components/viz/common/display/renderer_settings.h"
 #include "gpu/command_buffer/service/shared_context_state.h"
-#include "gpu/ipc/gpu_task_scheduler_helper.h"
 
 namespace gpu {
 class SharedContextState;

android_webview/browser/gfx/parent_output_surface.cc

@@ -132,6 +132,11 @@ gfx::OverlayTransform ParentOutputSurface::GetDisplayTransform() {
   return gfx::OVERLAY_TRANSFORM_NONE;
 }
 
+scoped_refptr<gpu::GpuTaskSchedulerHelper>
+ParentOutputSurface::GetGpuTaskSchedulerHelper() {
+  return nullptr;
+}
+
 gpu::MemoryTracker* ParentOutputSurface::GetMemoryTracker() {
   return nullptr;
 }

android_webview/browser/gfx/parent_output_surface.h

@@ -51,6 +51,8 @@ class ParentOutputSurface : public viz::OutputSurface {
       viz::UpdateVSyncParametersCallback callback) override;
   void SetDisplayTransformHint(gfx::OverlayTransform transform) override {}
   gfx::OverlayTransform GetDisplayTransform() override;
+  scoped_refptr<gpu::GpuTaskSchedulerHelper> GetGpuTaskSchedulerHelper()
+      override;
   gpu::MemoryTracker* GetMemoryTracker() override;
 
  private:

android_webview/browser/gfx/surfaces_instance.cc

@@ -81,14 +81,11 @@ SurfacesInstance::SurfacesInstance()
       begin_frame_source_.get(), nullptr /* current_task_runner */,
       output_surface->capabilities().max_frames_pending);
   auto overlay_processor = std::make_unique<viz::OverlayProcessorStub>();
-  // Android WebView has no overlay processor, and does not need to share
-  // gpu_task_scheduler, so it is passed in as nullptr.
   display_ = std::make_unique<viz::Display>(
       nullptr /* shared_bitmap_manager */,
       output_surface_provider_.renderer_settings(), frame_sink_id_,
-      nullptr /* gpu_task_scheduler */, std::move(output_surface),
-      std::move(overlay_processor), std::move(scheduler),
-      nullptr /* current_task_runner */);
+      std::move(output_surface), std::move(overlay_processor),
+      std::move(scheduler), nullptr /* current_task_runner */);
   display_->Initialize(this, frame_sink_manager_->surface_manager(),
                        output_surface_provider_.enable_shared_image());
   frame_sink_manager_->RegisterBeginFrameSource(begin_frame_source_.get(),

cc/test/layer_tree_pixel_test.cc

@@ -114,7 +114,7 @@ LayerTreePixelTest::CreateDisplaySkiaOutputSurfaceOnThread() {
       std::make_unique<viz::SkiaOutputSurfaceDependencyImpl>(
           viz::TestGpuServiceHolder::GetInstance()->gpu_service(),
           gpu::kNullSurfaceHandle),
-      nullptr, viz::RendererSettings());
+      viz::RendererSettings());
   return output_surface;
 }
 

cc/test/pixel_test.cc

@@ -291,13 +291,11 @@ void PixelTest::SetUpSkiaRenderer(gfx::SurfaceOrigin output_surface_origin) {
   // Set up the GPU service.
   gpu_service_holder_ = viz::TestGpuServiceHolder::GetInstance();
 
-  gpu_task_scheduler_ = std::make_unique<gpu::GpuTaskSchedulerHelper>(
-      gpu_service()->GetGpuScheduler());
   // Set up the skia renderer.
   output_surface_ = viz::SkiaOutputSurfaceImpl::Create(
       std::make_unique<viz::SkiaOutputSurfaceDependencyImpl>(
           gpu_service(), gpu::kNullSurfaceHandle),
-      gpu_task_scheduler_.get(), renderer_settings_);
+      renderer_settings_);
   output_surface_->BindToClient(output_surface_client_.get());
   static_cast<viz::SkiaOutputSurfaceImpl*>(output_surface_.get())
       ->SetCapabilitiesForTesting(output_surface_origin);

cc/test/pixel_test.h

@@ -115,7 +115,6 @@ class PixelTest : public testing::Test {
   gfx::Size device_viewport_size_;
   gfx::DisplayColorSpaces display_color_spaces_;
   bool disable_picture_quad_image_filtering_;
-  std::unique_ptr<gpu::GpuTaskSchedulerHelper> gpu_task_scheduler_;
   std::unique_ptr<FakeOutputSurfaceClient> output_surface_client_;
   std::unique_ptr<viz::OutputSurface> output_surface_;
   std::unique_ptr<viz::TestSharedBitmapManager> shared_bitmap_manager_;

cc/test/pixel_test_output_surface.cc

@@ -112,6 +112,11 @@ gfx::OverlayTransform PixelTestOutputSurface::GetDisplayTransform() {
   return gfx::OVERLAY_TRANSFORM_NONE;
 }
 
+scoped_refptr<gpu::GpuTaskSchedulerHelper>
+PixelTestOutputSurface::GetGpuTaskSchedulerHelper() {
+  return nullptr;
+}
+
 gpu::MemoryTracker* PixelTestOutputSurface::GetMemoryTracker() {
   return nullptr;
 }

cc/test/pixel_test_output_surface.h

@@ -41,6 +41,8 @@ class PixelTestOutputSurface : public viz::OutputSurface {
       viz::UpdateVSyncParametersCallback callback) override;
   void SetDisplayTransformHint(gfx::OverlayTransform transform) override {}
   gfx::OverlayTransform GetDisplayTransform() override;
+  scoped_refptr<gpu::GpuTaskSchedulerHelper> GetGpuTaskSchedulerHelper()
+      override;
   gpu::MemoryTracker* GetMemoryTracker() override;
 
   void set_has_external_stencil_test(bool has_test) {

cc/test/test_layer_tree_frame_sink.cc

@@ -102,15 +102,8 @@ bool TestLayerTreeFrameSink::BindToClient(LayerTreeFrameSinkClient* client) {
   }
 
   auto overlay_processor = std::make_unique<viz::OverlayProcessorStub>();
-  // Normally display will need to take ownership of a
-  // gpu::GpuTaskschedulerhelper in order to keep it alive to share between the
-  // output surface and the overlay processor. In this case the overlay
-  // processor is only a stub, and viz::TestGpuServiceHolder will keep a
-  // gpu::GpuTaskSchedulerHelper alive for output surface to use, so there is no
-  // need to pass in an gpu::GpuTaskSchedulerHelper here.
   display_ = std::make_unique<viz::Display>(
       shared_bitmap_manager_.get(), renderer_settings_, frame_sink_id_,
-      nullptr /* gpu::GpuTaskSchedulerHelper */,
       std::move(display_output_surface), std::move(overlay_processor),
       std::move(scheduler), compositor_task_runner_);
 

components/viz/service/compositor_frame_fuzzer/fuzzer_software_output_surface_provider.cc

@@ -86,19 +86,11 @@ FuzzerSoftwareOutputSurfaceProvider::FuzzerSoftwareOutputSurfaceProvider(
 FuzzerSoftwareOutputSurfaceProvider::~FuzzerSoftwareOutputSurfaceProvider() =
     default;
 
-std::unique_ptr<gpu::GpuTaskSchedulerHelper>
-FuzzerSoftwareOutputSurfaceProvider::CreateGpuTaskScheduler(
-    bool gpu_compositing,
-    const RendererSettings& renderer_settings) {
-  return nullptr;
-}
-
 std::unique_ptr<OutputSurface>
 FuzzerSoftwareOutputSurfaceProvider::CreateOutputSurface(
     gpu::SurfaceHandle surface_handle,
     bool gpu_compositing,
     mojom::DisplayClient* display_client,
-    gpu::GpuTaskSchedulerHelper* gpu_task_scheduler,
     const RendererSettings& renderer_settings) {
   std::unique_ptr<SoftwareOutputDevice> software_output_device =
       png_dir_path_ ? std::make_unique<PNGSoftwareOutputDevice>(*png_dir_path_)

components/viz/service/compositor_frame_fuzzer/fuzzer_software_output_surface_provider.h

@@ -22,14 +22,10 @@ class FuzzerSoftwareOutputSurfaceProvider : public OutputSurfaceProvider {
   ~FuzzerSoftwareOutputSurfaceProvider() override;
 
   // OutputSurfaceProvider implementation.
-  std::unique_ptr<gpu::GpuTaskSchedulerHelper> CreateGpuTaskScheduler(
-      bool gpu_compositing,
-      const RendererSettings& renderer_settings) override;
   std::unique_ptr<OutputSurface> CreateOutputSurface(
       gpu::SurfaceHandle surface_handle,
       bool gpu_compositing,
       mojom::DisplayClient* display_client,
-      gpu::GpuTaskSchedulerHelper* gpu_task_scheduler,
       const RendererSettings& renderer_settings) override;
 
   gpu::SharedImageManager* GetSharedImageManager() override;

components/viz/service/display/display.cc

@@ -261,15 +261,13 @@ Display::Display(
     SharedBitmapManager* bitmap_manager,
     const RendererSettings& settings,
     const FrameSinkId& frame_sink_id,
-    std::unique_ptr<gpu::GpuTaskSchedulerHelper> gpu_task_scheduler,
     std::unique_ptr<OutputSurface> output_surface,
     std::unique_ptr<OverlayProcessorInterface> overlay_processor,
     std::unique_ptr<DisplaySchedulerBase> scheduler,
     scoped_refptr<base::SingleThreadTaskRunner> current_task_runner)
     : bitmap_manager_(bitmap_manager),
       settings_(settings),
       frame_sink_id_(frame_sink_id),
-      gpu_task_scheduler_(std::move(gpu_task_scheduler)),
       output_surface_(std::move(output_surface)),
       skia_output_surface_(output_surface_->AsSkiaOutputSurface()),
       scheduler_(std::move(scheduler)),

components/viz/service/display/display.h

@@ -41,7 +41,6 @@ class Size;
 }
 
 namespace gpu {
-class GpuTaskSchedulerHelper;
 class ScopedAllowScheduleGpuTask;
 }
 
@@ -82,7 +81,6 @@ class VIZ_SERVICE_EXPORT Display : public DisplaySchedulerClient,
   Display(SharedBitmapManager* bitmap_manager,
           const RendererSettings& settings,
           const FrameSinkId& frame_sink_id,
-          std::unique_ptr<gpu::GpuTaskSchedulerHelper> gpu_task_scheduler,
           std::unique_ptr<OutputSurface> output_surface,
           std::unique_ptr<OverlayProcessorInterface> overlay_processor,
           std::unique_ptr<DisplaySchedulerBase> scheduler,
@@ -241,7 +239,6 @@ class VIZ_SERVICE_EXPORT Display : public DisplaySchedulerClient,
   std::unique_ptr<gpu::ScopedAllowScheduleGpuTask>
       allow_schedule_gpu_task_during_destruction_;
 #endif
-  std::unique_ptr<gpu::GpuTaskSchedulerHelper> gpu_task_scheduler_;
   std::unique_ptr<OutputSurface> output_surface_;
   SkiaOutputSurface* const skia_output_surface_;
   std::unique_ptr<DisplayDamageTracker> damage_tracker_;

components/viz/service/display/display_perftest.cc

@@ -70,15 +70,10 @@ class RemoveOverdrawQuadPerfTest : public testing::Test {
         FakeOutputSurface::Create3d();
 
     auto overlay_processor = std::make_unique<OverlayProcessorStub>();
-    // Normally display will need to take ownership of a
-    // gpu::GpuTaskschedulerhelper in order to keep it alive to share between
-    // the output surface and the overlay processor. In this case the overlay
-    // processor is a stub and the output surface is test only as well, so there
-    // is no need to pass in a real gpu::GpuTaskSchedulerHelper.
     auto display = std::make_unique<Display>(
         &bitmap_manager_, RendererSettings(), frame_sink_id,
-        nullptr /* gpu::GpuTaskSchedulerHelper */, std::move(output_surface),
-        std::move(overlay_processor), std::move(scheduler), task_runner_.get());
+        std::move(output_surface), std::move(overlay_processor),
+        std::move(scheduler), task_runner_.get());
     return display;
   }
 

components/viz/service/display/display_unittest.cc

@@ -180,15 +180,10 @@ class DisplayTest : public testing::Test {
       std::unique_ptr<DisplayScheduler> scheduler,
       std::unique_ptr<OutputSurface> output_surface) {
     auto overlay_processor = std::make_unique<OverlayProcessorStub>();
-    // Normally display will need to take ownership of a
-    // gpu::GpuTaskschedulerhelper in order to keep it alive to share between
-    // the output surface and the overlay processor. In this case the overlay
-    // processor is a stub and the output surface is test only as well, so there
-    // is no need to pass in a real gpu::GpuTaskSchedulerHelper.
     auto display = std::make_unique<Display>(
         &shared_bitmap_manager_, settings, frame_sink_id,
-        nullptr /* gpu::GpuTaskSchedulerHelper */, std::move(output_surface),
-        std::move(overlay_processor), std::move(scheduler), task_runner_);
+        std::move(output_surface), std::move(overlay_processor),
+        std::move(scheduler), task_runner_);
     display->SetVisible(true);
     return display;
   }

components/viz/service/display/gl_renderer_unittest.cc

@@ -2268,6 +2268,8 @@ class MockOutputSurface : public OutputSurface {
   MOCK_METHOD1(SetUpdateVSyncParametersCallback,
                void(UpdateVSyncParametersCallback));
   MOCK_METHOD1(SetDisplayTransformHint, void(gfx::OverlayTransform));
+  MOCK_METHOD0(GetGpuTaskSchedulerHelper,
+               scoped_refptr<gpu::GpuTaskSchedulerHelper>());
   MOCK_METHOD0(GetMemoryTracker, gpu::MemoryTracker*());
 
   gfx::OverlayTransform GetDisplayTransform() override {

components/viz/service/display/output_surface.h

@@ -246,6 +246,8 @@ class VIZ_SERVICE_EXPORT OutputSurface {
   // between the output surface and the overlay processor.
   // TODO(weiliangc): Consider making this outside of output surface and pass in
   // instead of passing it out here.
+  virtual scoped_refptr<gpu::GpuTaskSchedulerHelper>
+  GetGpuTaskSchedulerHelper() = 0;
   virtual gpu::MemoryTracker* GetMemoryTracker() = 0;
 
   // Notifies the OutputSurface of rate of content updates in frames per second.

components/viz/service/display/overlay_ca_unittest.cc

@@ -84,6 +84,10 @@ class OverlayOutputSurface : public OutputSurface {
   gfx::OverlayTransform GetDisplayTransform() override {
     return gfx::OVERLAY_TRANSFORM_NONE;
   }
+  scoped_refptr<gpu::GpuTaskSchedulerHelper> GetGpuTaskSchedulerHelper()
+      override {
+    return nullptr;
+  }
   gpu::MemoryTracker* GetMemoryTracker() override { return nullptr; }
 
   unsigned bind_framebuffer_count() const { return bind_framebuffer_count_; }

components/viz/service/display/overlay_dc_unittest.cc

@@ -86,6 +86,10 @@ class OverlayOutputSurface : public OutputSurface {
   gfx::OverlayTransform GetDisplayTransform() override {
     return gfx::OVERLAY_TRANSFORM_NONE;
   }
+  scoped_refptr<gpu::GpuTaskSchedulerHelper> GetGpuTaskSchedulerHelper()
+      override {
+    return nullptr;
+  }
   gpu::MemoryTracker* GetMemoryTracker() override { return nullptr; }
 
   void set_is_displayed_as_overlay_plane(bool value) {

components/viz/service/display/overlay_processor_android.cc

@@ -16,10 +16,10 @@ namespace viz {
 OverlayProcessorAndroid::OverlayProcessorAndroid(
     gpu::SharedImageManager* shared_image_manager,
     gpu::MemoryTracker* memory_tracker,
-    gpu::GpuTaskSchedulerHelper* gpu_task_scheduler,
+    scoped_refptr<gpu::GpuTaskSchedulerHelper> gpu_task_scheduler,
     bool enable_overlay)
     : OverlayProcessorUsingStrategy(),
-      gpu_task_scheduler_(gpu_task_scheduler),
+      gpu_task_scheduler_(std::move(gpu_task_scheduler)),
       overlay_enabled_(enable_overlay) {
   if (!overlay_enabled_)
     return;

components/viz/service/display/overlay_processor_android.h

@@ -28,10 +28,11 @@ class OverlayProcessorOnGpu;
 class VIZ_SERVICE_EXPORT OverlayProcessorAndroid
     : public OverlayProcessorUsingStrategy {
  public:
-  OverlayProcessorAndroid(gpu::SharedImageManager* shared_image_manager,
-                          gpu::MemoryTracker* memory_tracker,
-                          gpu::GpuTaskSchedulerHelper* gpu_task_scheduler,
-                          bool enable_overlay);
+  OverlayProcessorAndroid(
+      gpu::SharedImageManager* shared_image_manager,
+      gpu::MemoryTracker* memory_tracker,
+      scoped_refptr<gpu::GpuTaskSchedulerHelper> gpu_task_scheduler,
+      bool enable_overlay);
   ~OverlayProcessorAndroid() override;
 
   bool IsOverlaySupported() const override;
@@ -74,7 +75,7 @@ class VIZ_SERVICE_EXPORT OverlayProcessorAndroid
   // overlay, if one backs them with a SurfaceView.
   PromotionHintInfoMap promotion_hint_info_map_;
 
-  gpu::GpuTaskSchedulerHelper* gpu_task_scheduler_;
+  scoped_refptr<gpu::GpuTaskSchedulerHelper> gpu_task_scheduler_;
   const bool overlay_enabled_;
   // This class is created, accessed, and destroyed on the gpu thread.
   std::unique_ptr<OverlayProcessorOnGpu> processor_on_gpu_;

components/viz/service/display/overlay_processor_interface.cc

@@ -81,7 +81,7 @@ OverlayProcessorInterface::CreateOverlayProcessor(
     const RendererSettings& renderer_settings,
     gpu::SharedImageManager* shared_image_manager,
     gpu::MemoryTracker* memory_tracker,
-    gpu::GpuTaskSchedulerHelper* gpu_task_scheduler,
+    scoped_refptr<gpu::GpuTaskSchedulerHelper> gpu_task_scheduler,
     gpu::SharedImageInterface* shared_image_interface) {
 #if defined(OS_MACOSX)
   bool could_overlay = surface_handle != gpu::kNullSurfaceHandle;

components/viz/service/display/overlay_processor_interface.h

@@ -101,7 +101,7 @@ class VIZ_SERVICE_EXPORT OverlayProcessorInterface {
       const RendererSettings& renderer_settings,
       gpu::SharedImageManager* shared_image_manager,
       gpu::MemoryTracker* memory_tracker,
-      gpu::GpuTaskSchedulerHelper* gpu_task_scheduler,
+      scoped_refptr<gpu::GpuTaskSchedulerHelper> gpu_task_scheduler,
       gpu::SharedImageInterface* shared_image_interface);
 
   virtual ~OverlayProcessorInterface() {}

components/viz/service/display/overlay_unittest.cc

@@ -211,6 +211,10 @@ class OverlayOutputSurface : public OutputSurface {
   gfx::OverlayTransform GetDisplayTransform() override {
     return gfx::OVERLAY_TRANSFORM_NONE;
   }
+  scoped_refptr<gpu::GpuTaskSchedulerHelper> GetGpuTaskSchedulerHelper()
+      override {
+    return nullptr;
+  }
   gpu::MemoryTracker* GetMemoryTracker() override { return nullptr; }
 
   void set_is_displayed_as_overlay_plane(bool value) {