Revert "Change FrameSinkManager task runner."

This reverts commit a1037c1c.

Reason for revert: https://crbug.com/657959#c47

Original change's description:
> Change FrameSinkManager task runner.
> 
> On Mac there is a special task runner that runs when the main task
> runner is blocked on resize. The main task runner blocks until a
> CompositorFrame arrives, and we need HostFrameSinkManager and
> FrameSinkManagerImpl to continue processing messages during this time.
> 
> Bug: 657959
> Change-Id: I3d8bb8bb22b5a4d36d1fe83e1144d11bbe1fd59d
> Reviewed-on: https://chromium-review.googlesource.com/563860
> Commit-Queue: kylechar <kylechar@chromium.org>
> Reviewed-by: Saman Sami <samans@chromium.org>
> Reviewed-by: Fady Samuel <fsamuel@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#486070}

TBR=fsamuel@chromium.org,kylechar@chromium.org,piman@chromium.org,samans@chromium.org

Change-Id: I540b79a8fc55ec260268bf437903e05020dc398f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 657959
Reviewed-on: https://chromium-review.googlesource.com/568879Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486088}

components/viz/host/host_frame_sink_manager.cc

@@ -6,7 +6,6 @@
 
 #include <utility>
 
-#include "base/sequenced_task_runner.h"
 #include "cc/surfaces/surface_info.h"
 #include "cc/surfaces/surface_manager.h"
 
@@ -16,12 +15,11 @@ HostFrameSinkManager::HostFrameSinkManager() : binding_(this) {}
 
 HostFrameSinkManager::~HostFrameSinkManager() = default;
 
-void HostFrameSinkManager::BindAndSetManager(
+void HostFrameSinkManager::BindManagerClientAndSetManagerPtr(
     cc::mojom::FrameSinkManagerClientRequest request,
-    scoped_refptr<base::SequencedTaskRunner> task_runner,
     cc::mojom::FrameSinkManagerPtr ptr) {
   DCHECK(!binding_.is_bound());
-  binding_.Bind(std::move(request), std::move(task_runner));
+  binding_.Bind(std::move(request));
   frame_sink_manager_ptr_ = std::move(ptr);
 }
 

components/viz/host/host_frame_sink_manager.h

@@ -8,7 +8,6 @@
 #include "base/compiler_specific.h"
 #include "base/containers/flat_map.h"
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
 #include "base/observer_list.h"
 #include "base/optional.h"
 #include "cc/ipc/frame_sink_manager.mojom.h"
@@ -17,10 +16,6 @@
 #include "components/viz/host/viz_host_export.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
-namespace base {
-class SequencedTaskRunner;
-}
-
 namespace cc {
 class SurfaceInfo;
 class SurfaceManager;
@@ -36,12 +31,11 @@ class VIZ_HOST_EXPORT HostFrameSinkManager
   HostFrameSinkManager();
   ~HostFrameSinkManager() override;
 
-  // Binds |this| as a FrameSinkManagerClient for |request| on |task_runner|. On
-  // Mac |task_runner| will be the resize helper task runner. May only be called
-  // once.
-  void BindAndSetManager(cc::mojom::FrameSinkManagerClientRequest request,
-                         scoped_refptr<base::SequencedTaskRunner> task_runner,
-                         cc::mojom::FrameSinkManagerPtr ptr);
+  // Binds |this| as a FrameSinkManagerClient to the |request|. May only be
+  // called once.
+  void BindManagerClientAndSetManagerPtr(
+      cc::mojom::FrameSinkManagerClientRequest request,
+      cc::mojom::FrameSinkManagerPtr ptr);
 
   void AddObserver(FrameSinkObserver* observer);
   void RemoveObserver(FrameSinkObserver* observer);

components/viz/host/host_frame_sink_manager_unittests.cc

@@ -116,9 +116,8 @@ class HostFrameSinkManagerTest : public testing::Test {
         mojo::MakeRequest(&manager_mojo);
     manager_impl_->BindAndSetClient(std::move(manager_impl_request),
                                     std::move(host_mojo));
-    host_manager_->BindAndSetManager(std::move(host_mojo_request),
-                                     message_loop_.task_runner(),
-                                     std::move(manager_mojo));
+    host_manager_->BindManagerClientAndSetManagerPtr(
+        std::move(host_mojo_request), std::move(manager_mojo));
   }
 
  private:

components/viz/service/frame_sinks/frame_sink_manager_impl.cc

@@ -8,7 +8,6 @@
 
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
-#include "base/sequenced_task_runner.h"
 #include "cc/base/switches.h"
 #include "cc/scheduler/begin_frame_source.h"
 #include "components/viz/service/display/display.h"
@@ -33,12 +32,11 @@ FrameSinkManagerImpl::~FrameSinkManagerImpl() {
   manager_.surface_manager()->RemoveObserver(this);
 }
 
-void FrameSinkManagerImpl::BindAndSetClient(
+void FrameSinkManagerImpl::BindPtrAndSetClient(
     cc::mojom::FrameSinkManagerRequest request,
-    scoped_refptr<base::SequencedTaskRunner> task_runner,
     cc::mojom::FrameSinkManagerClientPtr client) {
   DCHECK(!binding_.is_bound());
-  binding_.Bind(std::move(request), std::move(task_runner));
+  binding_.Bind(std::move(request));
   client_ = std::move(client);
 }
 

components/viz/service/frame_sinks/frame_sink_manager_impl.h

@@ -11,7 +11,6 @@
 #include <unordered_map>
 
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
 #include "base/threading/thread_checker.h"
 #include "cc/ipc/frame_sink_manager.mojom.h"
 #include "cc/surfaces/frame_sink_manager.h"
@@ -22,10 +21,6 @@
 #include "gpu/ipc/common/surface_handle.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
-namespace base {
-class SequencedTaskRunner;
-}
-
 namespace viz {
 
 class DisplayProvider;
@@ -49,12 +44,10 @@ class VIZ_SERVICE_EXPORT FrameSinkManagerImpl
 
   cc::FrameSinkManager* frame_sink_manager() { return &manager_; }
 
-  // Binds |this| as a FrameSinkManager for |request| on |task_runner|. On Mac
-  // |task_runner| will be the resize helper task runner. May only be called
-  // once.
-  void BindAndSetClient(cc::mojom::FrameSinkManagerRequest request,
-                        scoped_refptr<base::SequencedTaskRunner> task_runner,
-                        cc::mojom::FrameSinkManagerClientPtr client);
+  // Binds |this| as a FrameSinkManager for a given |request|. This may
+  // only be called once.
+  void BindPtrAndSetClient(cc::mojom::FrameSinkManagerRequest request,
+                           cc::mojom::FrameSinkManagerClientPtr client);
 
   // cc::mojom::FrameSinkManager implementation:
   void CreateRootCompositorFrameSink(

content/browser/browser_main_loop.cc

@@ -926,18 +926,6 @@ void BrowserMainLoop::CreateStartupTasks() {
 #endif
 }
 
-scoped_refptr<base::SingleThreadTaskRunner>
-BrowserMainLoop::GetResizeTaskRunner() {
-#if defined(OS_MACOSX)
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner =
-      ui::WindowResizeHelperMac::Get()->task_runner();
-  // In tests, WindowResizeHelperMac task runner might not be initialized.
-  return task_runner ? task_runner : base::ThreadTaskRunnerHandle::Get();
-#else
-  return base::ThreadTaskRunnerHandle::Get();
-#endif
-}
-
 gpu::GpuChannelEstablishFactory*
 BrowserMainLoop::gpu_channel_establish_factory() const {
   return BrowserGpuChannelHostFactory::instance();
@@ -1472,13 +1460,12 @@ int BrowserMainLoop::BrowserThreadsStarted() {
     // TODO(danakj): Don't make a FrameSinkManagerImpl when display is in the
     // Gpu process, instead get the mojo pointer from the Gpu process.
     surface_utils::ConnectWithInProcessFrameSinkManager(
-        host_frame_sink_manager_.get(), frame_sink_manager_impl_.get(),
-        GetResizeTaskRunner());
+        host_frame_sink_manager_.get(), frame_sink_manager_impl_.get());
   }
 #endif
 
   DCHECK(factory);
-  ImageTransportFactory::Initialize(GetResizeTaskRunner());
+  ImageTransportFactory::Initialize();
   ImageTransportFactory::GetInstance()->SetGpuChannelEstablishFactory(factory);
 #if defined(USE_AURA)
   if (env_->mode() == aura::Env::Mode::LOCAL) {

content/browser/browser_main_loop.h

@@ -29,7 +29,6 @@ class HighResolutionTimerManager;
 class MemoryPressureMonitor;
 class MessageLoop;
 class PowerMonitor;
-class SingleThreadTaskRunner;
 class SystemMonitor;
 namespace trace_event {
 class TraceEventSystemStatsMonitor;
@@ -175,12 +174,6 @@ class CONTENT_EXPORT BrowserMainLoop {
     return startup_trace_file_;
   }
 
-  // Returns the task runner for tasks that that are critical to producing a new
-  // CompositorFrame on resize. On Mac this will be the task runner provided by
-  // WindowResizeHelperMac, on other platforms it will just be the thread task
-  // runner.
-  scoped_refptr<base::SingleThreadTaskRunner> GetResizeTaskRunner();
-
   gpu::GpuChannelEstablishFactory* gpu_channel_establish_factory() const;
 
 #if defined(OS_ANDROID)

content/browser/compositor/gpu_process_transport_factory.cc

@@ -91,6 +91,7 @@
 #include "content/browser/compositor/gpu_output_surface_mac.h"
 #include "content/browser/compositor/software_output_device_mac.h"
 #include "gpu/config/gpu_driver_bug_workaround_type.h"
+#include "ui/accelerated_widget_mac/window_resize_helper_mac.h"
 #include "ui/base/cocoa/remote_layer_api.h"
 #include "ui/base/ui_base_switches.h"
 #elif defined(OS_ANDROID)
@@ -198,12 +199,10 @@ struct GpuProcessTransportFactory::PerCompositorData {
   bool output_is_secure = false;
 };
 
-GpuProcessTransportFactory::GpuProcessTransportFactory(
-    scoped_refptr<base::SingleThreadTaskRunner> resize_task_runner)
+GpuProcessTransportFactory::GpuProcessTransportFactory()
     : frame_sink_id_allocator_(kDefaultClientId),
       renderer_settings_(
           ui::CreateRendererSettings(CreateBufferToTextureTargetMap())),
-      resize_task_runner_(std::move(resize_task_runner)),
       task_graph_runner_(new cc::SingleThreadTaskGraphRunner),
       callback_factory_(this) {
   cc::SetClientNameForMetrics("Browser");
@@ -458,9 +457,12 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
                 "125248"
                 " GpuProcessTransportFactory::EstablishedGpuChannel"
                 "::Compositor"));
+#if defined(OS_MACOSX)
         // On Mac, GpuCommandBufferMsg_SwapBuffersCompleted must be handled in
         // a nested run loop during resize.
-        context_provider->SetDefaultTaskRunner(resize_task_runner_);
+        context_provider->SetDefaultTaskRunner(
+            ui::WindowResizeHelperMac::Get()->task_runner());
+#endif
         if (!context_provider->BindToCurrentThread())
           context_provider = nullptr;
       }

content/browser/compositor/gpu_process_transport_factory.h

@@ -23,10 +23,6 @@
 #include "gpu/ipc/client/gpu_channel_host.h"
 #include "ui/compositor/compositor.h"
 
-namespace base {
-class SingleThreadTaskRunner;
-}
-
 namespace cc {
 class ResourceSettings;
 class SingleThreadTaskGraphRunner;
@@ -46,8 +42,7 @@ class GpuProcessTransportFactory : public ui::ContextFactory,
                                    public ui::ContextFactoryPrivate,
                                    public ImageTransportFactory {
  public:
-  explicit GpuProcessTransportFactory(
-      scoped_refptr<base::SingleThreadTaskRunner> resize_task_runner);
+  GpuProcessTransportFactory();
 
   ~GpuProcessTransportFactory() override;
 
@@ -128,7 +123,6 @@ class GpuProcessTransportFactory : public ui::ContextFactory,
   scoped_refptr<ui::ContextProviderCommandBuffer> shared_main_thread_contexts_;
   std::unique_ptr<viz::GLHelper> gl_helper_;
   base::ObserverList<ui::ContextFactoryObserver> observer_list_;
-  scoped_refptr<base::SingleThreadTaskRunner> resize_task_runner_;
   std::unique_ptr<cc::SingleThreadTaskGraphRunner> task_graph_runner_;
   scoped_refptr<ui::ContextProviderCommandBuffer>
       shared_worker_context_provider_;

content/browser/compositor/image_transport_factory.cc

@@ -5,7 +5,6 @@
 #include "content/browser/compositor/image_transport_factory.h"
 
 #include "base/command_line.h"
-#include "base/single_thread_task_runner.h"
 #include "content/browser/compositor/gpu_process_transport_factory.h"
 #include "ui/compositor/compositor.h"
 #include "ui/compositor/compositor_switches.h"
@@ -25,12 +24,11 @@ void SetFactory(ImageTransportFactory* factory) {
 }
 
 // static
-void ImageTransportFactory::Initialize(
-    scoped_refptr<base::SingleThreadTaskRunner> resize_task_runner) {
+void ImageTransportFactory::Initialize() {
   DCHECK(!g_factory || g_initialized_for_unit_tests);
   if (g_initialized_for_unit_tests)
     return;
-  SetFactory(new GpuProcessTransportFactory(std::move(resize_task_runner)));
+  SetFactory(new GpuProcessTransportFactory);
 }
 
 void ImageTransportFactory::InitializeForUnitTests(

content/browser/compositor/image_transport_factory.h

@@ -15,10 +15,6 @@
 #include "ui/gfx/native_widget_types.h"
 #include "ui/latency/latency_info.h"
 
-namespace base {
-class SingleThreadTaskRunner;
-}
-
 namespace gfx {
 enum class SwapResult;
 }
@@ -48,8 +44,7 @@ class CONTENT_EXPORT ImageTransportFactory {
   virtual ~ImageTransportFactory() {}
 
   // Initializes the global transport factory.
-  static void Initialize(
-      scoped_refptr<base::SingleThreadTaskRunner> resize_task_runner);
+  static void Initialize();
 
   // Initializes the global transport factory for unit tests using the provided
   // context factory.

content/browser/compositor/surface_utils.cc

@@ -6,7 +6,7 @@
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
-#include "base/sequenced_task_runner.h"
+#include "base/memory/ref_counted.h"
 #include "build/build_config.h"
 #include "cc/output/copy_output_result.h"
 #include "cc/resources/single_release_callback.h"
@@ -214,10 +214,8 @@ void CopyFromCompositingSurfaceHasResult(
 
 namespace surface_utils {
 
-void ConnectWithInProcessFrameSinkManager(
-    viz::HostFrameSinkManager* host,
-    viz::FrameSinkManagerImpl* manager,
-    scoped_refptr<base::SequencedTaskRunner> task_runner) {
+void ConnectWithInProcessFrameSinkManager(viz::HostFrameSinkManager* host,
+                                          viz::FrameSinkManagerImpl* manager) {
   // A mojo pointer to |host| which is the FrameSinkManager's client.
   cc::mojom::FrameSinkManagerClientPtr host_mojo;
   // A mojo pointer to |manager|.
@@ -230,11 +228,11 @@ void ConnectWithInProcessFrameSinkManager(
       mojo::MakeRequest(&manager_mojo);
 
   // Sets |manager_mojo| which is given to the |host|.
-  manager->BindAndSetClient(std::move(manager_mojo_request), task_runner,
-                            std::move(host_mojo));
+  manager->BindPtrAndSetClient(std::move(manager_mojo_request),
+                               std::move(host_mojo));
   // Sets |host_mojo| which was given to the |manager|.
-  host->BindAndSetManager(std::move(host_mojo_request), task_runner,
-                          std::move(manager_mojo));
+  host->BindManagerClientAndSetManagerPtr(std::move(host_mojo_request),
+                                          std::move(manager_mojo));
 }
 
 }  // namespace surface_utils

content/browser/compositor/surface_utils.h

@@ -7,17 +7,12 @@
 
 #include <memory>
 
-#include "base/memory/ref_counted.h"
 #include "components/viz/common/frame_sink_id.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/readback_types.h"
 #include "third_party/skia/include/core/SkImageInfo.h"
 #include "ui/gfx/geometry/size.h"
 
-namespace base {
-class SequencedTaskRunner;
-}
-
 namespace cc {
 class CopyOutputResult;
 class FrameSinkManager;
@@ -46,8 +41,7 @@ namespace surface_utils {
 
 CONTENT_EXPORT void ConnectWithInProcessFrameSinkManager(
     viz::HostFrameSinkManager* host,
-    viz::FrameSinkManagerImpl* manager,
-    scoped_refptr<base::SequencedTaskRunner> task_runner);
+    viz::FrameSinkManagerImpl* manager);
 
 }  // namespace surface_utils
 

content/browser/compositor/test/no_transport_image_transport_factory.cc

@@ -7,7 +7,6 @@
 #include <utility>
 
 #include "base/memory/ptr_util.h"
-#include "base/threading/sequenced_task_runner_handle.h"
 #include "build/build_config.h"
 #include "cc/output/context_provider.h"
 #include "cc/surfaces/surface_manager.h"
@@ -22,9 +21,8 @@ NoTransportImageTransportFactory::NoTransportImageTransportFactory()
     : frame_sink_manager_(false /* use surface references */, nullptr),
       context_factory_(&host_frame_sink_manager_,
                        frame_sink_manager_.frame_sink_manager()) {
-  surface_utils::ConnectWithInProcessFrameSinkManager(
-      &host_frame_sink_manager_, &frame_sink_manager_,
-      base::ThreadTaskRunnerHandle::Get());
+  surface_utils::ConnectWithInProcessFrameSinkManager(&host_frame_sink_manager_,
+                                                      &frame_sink_manager_);
 
   // The context factory created here is for unit tests, thus using a higher
   // refresh rate to spend less time waiting for BeginFrames.

content/browser/renderer_host/compositor_impl_android.cc

@@ -105,8 +105,7 @@ struct CompositorDependencies {
     frame_sink_manager_impl =
         base::MakeUnique<viz::FrameSinkManagerImpl>(false, nullptr);
     surface_utils::ConnectWithInProcessFrameSinkManager(
-        &host_frame_sink_manager, frame_sink_manager_impl.get(),
-        base::ThreadTaskRunnerHandle::Get());
+        &host_frame_sink_manager, frame_sink_manager_impl.get());
   }
 
   SingleThreadTaskGraphRunner task_graph_runner;

content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc

@@ -155,8 +155,7 @@ class OffscreenCanvasProviderImplTest : public testing::Test {
     frame_sink_manager_ =
         base::MakeUnique<viz::FrameSinkManagerImpl>(false, nullptr);
     surface_utils::ConnectWithInProcessFrameSinkManager(
-        host_frame_sink_manager_.get(), frame_sink_manager_.get(),
-        message_loop_.task_runner());
+        host_frame_sink_manager_.get(), frame_sink_manager_.get());
 
     provider_ = base::MakeUnique<OffscreenCanvasProviderImpl>(
         host_frame_sink_manager_.get(), kRendererClientId);

content/browser/renderer_host/render_widget_host_impl.cc

@@ -34,7 +34,6 @@
 #include "components/viz/service/display_embedder/server_shared_bitmap_manager.h"
 #include "content/browser/accessibility/browser_accessibility_state_impl.h"
 #include "content/browser/bad_message.h"
-#include "content/browser/browser_main_loop.h"
 #include "content/browser/browser_plugin/browser_plugin_guest.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/fileapi/browser_file_system_helper.h"
@@ -2559,9 +2558,17 @@ void RenderWidgetHostImpl::RequestCompositorFrameSink(
     cc::mojom::CompositorFrameSinkClientPtr client) {
   if (compositor_frame_sink_binding_.is_bound())
     compositor_frame_sink_binding_.Close();
-  compositor_frame_sink_binding_.Bind(
-      std::move(request),
-      BrowserMainLoop::GetInstance()->GetResizeTaskRunner());
+#if defined(OS_MACOSX)
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner =
+      ui::WindowResizeHelperMac::Get()->task_runner();
+  // In tests, task_runner might not be initialized.
+  if (task_runner)
+    compositor_frame_sink_binding_.Bind(std::move(request), task_runner);
+  else
+    compositor_frame_sink_binding_.Bind(std::move(request));
+#else
+  compositor_frame_sink_binding_.Bind(std::move(request));
+#endif
   if (view_)
     view_->DidCreateNewRendererCompositorFrameSink(client.get());
   renderer_compositor_frame_sink_ = std::move(client);

services/ui/gpu/gpu_main.cc

@@ -9,7 +9,6 @@
 #include "base/message_loop/message_loop.h"
 #include "base/power_monitor/power_monitor_device_source.h"
 #include "base/single_thread_task_runner.h"
-#include "base/threading/sequenced_task_runner_handle.h"
 #include "components/viz/service/display_embedder/gpu_display_provider.h"
 #include "components/viz/service/frame_sinks/frame_sink_manager_impl.h"
 #include "gpu/command_buffer/common/activity_flags.h"
@@ -184,9 +183,8 @@ void GpuMain::CreateFrameSinkManagerOnCompositorThread(
 
   frame_sink_manager_ = base::MakeUnique<viz::FrameSinkManagerImpl>(
       true, display_provider_.get());
-  frame_sink_manager_->BindAndSetClient(std::move(request),
-                                        base::SequencedTaskRunnerHandle::Get(),
-                                        std::move(client));
+  frame_sink_manager_->BindPtrAndSetClient(std::move(request),
+                                           std::move(client));
 }
 
 void GpuMain::TearDownOnCompositorThread() {

ui/compositor/test/context_factories_for_test.cc

@@ -6,7 +6,6 @@
 
 #include "base/command_line.h"
 #include "base/sys_info.h"
-#include "base/threading/sequenced_task_runner_handle.h"
 #include "cc/surfaces/frame_sink_manager.h"
 #include "components/viz/host/host_frame_sink_manager.h"
 #include "components/viz/service/frame_sinks/frame_sink_manager_impl.h"
@@ -33,12 +32,10 @@ void ConnectFrameSinkManager() {
       mojo::MakeRequest(&manager_mojo);
 
   // Make the Mojo connections on both ends.
-  g_frame_sink_manager_impl->BindAndSetClient(
-      std::move(manager_mojo_request), base::SequencedTaskRunnerHandle::Get(),
-      std::move(host_mojo));
-  g_host_frame_sink_manager->BindAndSetManager(
-      std::move(host_mojo_request), base::SequencedTaskRunnerHandle::Get(),
-      std::move(manager_mojo));
+  g_frame_sink_manager_impl->BindPtrAndSetClient(
+      std::move(manager_mojo_request), std::move(host_mojo));
+  g_host_frame_sink_manager->BindManagerClientAndSetManagerPtr(
+      std::move(host_mojo_request), std::move(manager_mojo));
 }
 
 }  // namespace