Moves cc::Surfaces for Videos onto its own thread.

Previously we were running the cc::Surface for Videos project on the media thread.
This was found to cause some smoothness regressions. This CL fixes that by pushing the
work onto its own thread.

TBR=gab@chromium.org

Bug: 866508
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ide12cb6cebc9dea83a7686c09e6688537bf3389a
Reviewed-on: https://chromium-review.googlesource.com/1176879
Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587811}

base/threading/thread_restrictions.h

@@ -23,6 +23,9 @@ class ScopedAllowInitGLBindings;
 namespace audio {
 class OutputDevice;
 }
+namespace blink {
+class VideoFrameResourceProvider;
+}
 namespace cc {
 class CompletionEvent;
 class SingleThreadTaskGraphRunner;
@@ -330,6 +333,7 @@ class BASE_EXPORT ScopedAllowBaseSyncPrimitives {
       LaunchXdgUtilityScopedAllowBaseSyncPrimitives;
   friend class webrtc::DesktopConfigurationMonitor;
   friend class content::ServiceWorkerSubresourceLoader;
+  friend class blink::VideoFrameResourceProvider;
 
   ScopedAllowBaseSyncPrimitives() EMPTY_BODY_IF_DCHECK_IS_OFF;
   ~ScopedAllowBaseSyncPrimitives() EMPTY_BODY_IF_DCHECK_IS_OFF;

content/renderer/media/media_factory.cc

@@ -24,6 +24,7 @@
 #include "content/renderer/render_frame_impl.h"
 #include "content/renderer/render_thread_impl.h"
 #include "content/renderer/render_view_impl.h"
+#include "media/base/bind_to_current_loop.h"
 #include "media/base/cdm_factory.h"
 #include "media/base/decoder_factory.h"
 #include "media/base/media_switches.h"
@@ -100,38 +101,26 @@ class FrameFetchContext : public media::ResourceFetchContext {
   DISALLOW_COPY_AND_ASSIGN(FrameFetchContext);
 };
 
-void ObtainAndSetContextProvider(
-    base::OnceCallback<void(bool,
-                            scoped_refptr<ws::ContextProviderCommandBuffer>)>
-        set_context_provider_callback,
-    std::pair<media::GpuVideoAcceleratorFactories*, bool> gpu_info) {
-  if (gpu_info.first) {
-    scoped_refptr<ws::ContextProviderCommandBuffer> context_provider =
-        gpu_info.first->GetMediaContextProvider();
-    std::move(set_context_provider_callback)
-        .Run(gpu_info.second, std::move(context_provider));
-  } else {
-    std::move(set_context_provider_callback).Run(false, nullptr);
-  }
-}
-
 // Obtains the media ContextProvider and calls the given callback on the same
 // thread this is called on. Obtaining the media ContextProvider requires
-// getting GPuVideoAcceleratorFactories, which must be done on the main
-// thread.
-void PostMediaContextProviderToCallback(
+// establishing a GPUChannelHost, which must be done on the main thread.
+void PostContextProviderToCallback(
     scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
-    base::OnceCallback<void(bool,
-                            scoped_refptr<ws::ContextProviderCommandBuffer>)>
-        set_context_provider_callback) {
-  base::PostTaskAndReplyWithResult(
-      main_task_runner.get(), FROM_HERE, base::BindOnce([]() {
-        return std::pair<media::GpuVideoAcceleratorFactories*, bool>(
-            content::RenderThreadImpl::current()->GetGpuFactories(),
-            !content::RenderThreadImpl::current()->IsGpuCompositingDisabled());
-      }),
-      base::BindOnce(&ObtainAndSetContextProvider,
-                     std::move(set_context_provider_callback)));
+    scoped_refptr<viz::ContextProvider> unwanted_context_provider,
+    blink::WebSubmitterConfigurationCallback set_context_provider_callback) {
+  main_task_runner->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          [](scoped_refptr<viz::ContextProvider> unwanted_context_provider,
+             blink::WebSubmitterConfigurationCallback cb) {
+            auto* rti = content::RenderThreadImpl::current();
+            auto context_provider = rti->GetVideoFrameCompositorContextProvider(
+                unwanted_context_provider);
+            std::move(cb).Run(!rti->IsGpuCompositingDisabled(),
+                              std::move(context_provider));
+          },
+          std::move(unwanted_context_provider),
+          media::BindToCurrentLoop(std::move(set_context_provider_callback))));
 }
 
 }  // namespace
@@ -307,12 +296,11 @@ blink::WebMediaPlayer* MediaFactory::CreateMediaPlayer(
   std::unique_ptr<blink::WebVideoFrameSubmitter> submitter;
   bool use_surface_layer_for_video = VideoSurfaceLayerEnabled();
   if (use_surface_layer_for_video) {
-    // TODO(lethalantidote): Use a separate task_runner. https://crbug/753605.
     video_frame_compositor_task_runner =
-        render_thread->GetMediaThreadTaskRunner();
+        render_thread->CreateVideoFrameCompositorTaskRunner();
     submitter = blink::WebVideoFrameSubmitter::Create(
         base::BindRepeating(
-            &PostMediaContextProviderToCallback,
+            &PostContextProviderToCallback,
             RenderThreadImpl::current()->GetCompositorMainThreadTaskRunner()),
         settings);
   } else {

content/renderer/render_thread_impl.cc

@@ -31,6 +31,7 @@
 #include "base/strings/string_split.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/task/post_task.h"
 #include "base/threading/simple_thread.h"
 #include "base/threading/thread_local.h"
 #include "base/threading/thread_restrictions.h"
@@ -1191,6 +1192,21 @@ void RenderThreadImpl::InitializeCompositorThread() {
 #endif
 }
 
+scoped_refptr<base::SingleThreadTaskRunner>
+RenderThreadImpl::CreateVideoFrameCompositorTaskRunner() {
+  if (!video_frame_compositor_task_runner_) {
+    // All of Chromium's GPU code must know which thread it's running on, and
+    // be the same thread on which the rendering context was initialized. This
+    // is why this must be a SingleThreadTaskRunner instead of a
+    // SequencedTaskRunner.
+    video_frame_compositor_task_runner_ =
+        base::CreateSingleThreadTaskRunnerWithTraits(
+            {base::MayBlock(), base::TaskPriority::USER_VISIBLE});
+  }
+
+  return video_frame_compositor_task_runner_;
+}
+
 void RenderThreadImpl::InitializeWebKit(
     service_manager::BinderRegistry* registry) {
   DCHECK(!blink_platform_impl_);
@@ -1434,6 +1450,39 @@ media::GpuVideoAcceleratorFactories* RenderThreadImpl::GetGpuFactories() {
   return gpu_factories_.back().get();
 }
 
+scoped_refptr<viz::ContextProvider>
+RenderThreadImpl::GetVideoFrameCompositorContextProvider(
+    scoped_refptr<viz::ContextProvider> unwanted_context_provider) {
+  if (video_frame_compositor_context_provider_ &&
+      video_frame_compositor_context_provider_ != unwanted_context_provider) {
+    return video_frame_compositor_context_provider_;
+  }
+
+  video_frame_compositor_context_provider_ = nullptr;
+
+  scoped_refptr<gpu::GpuChannelHost> gpu_channel_host =
+      EstablishGpuChannelSync();
+  if (!gpu_channel_host)
+    return nullptr;
+
+  // This context is only used to create textures and mailbox them, so
+  // use lower limits than the default.
+  gpu::SharedMemoryLimits limits = gpu::SharedMemoryLimits::ForMailboxContext();
+
+  bool support_locking = false;
+  bool support_gles2_interface = true;
+  bool support_raster_interface = false;
+  bool support_oop_rasterization = false;
+  bool support_grcontext = false;
+  video_frame_compositor_context_provider_ = CreateOffscreenContext(
+      gpu_channel_host, GetGpuMemoryBufferManager(), limits, support_locking,
+      support_gles2_interface, support_raster_interface,
+      support_oop_rasterization, support_grcontext,
+      ws::command_buffer_metrics::ContextType::RENDER_COMPOSITOR,
+      kGpuStreamIdMedia, kGpuStreamPriorityMedia);
+  return video_frame_compositor_context_provider_;
+}
+
 scoped_refptr<ws::ContextProviderCommandBuffer>
 RenderThreadImpl::SharedMainThreadContextProvider() {
   DCHECK(IsMainThread());

content/renderer/render_thread_impl.h

@@ -392,6 +392,13 @@ class CONTENT_EXPORT RenderThreadImpl
   // A TaskRunner instance that runs tasks on the raster worker pool.
   base::TaskRunner* GetWorkerTaskRunner();
 
+  // Creates a ContextProvider if yet created, and returns it to be used for
+  // video frame compositing. The ContextProvider given as an argument is
+  // one that has been lost, and is a hint to the RenderThreadImpl to clear
+  // it's |video_frame_compositor_context_provider_| if it matches.
+  scoped_refptr<viz::ContextProvider> GetVideoFrameCompositorContextProvider(
+      scoped_refptr<viz::ContextProvider>);
+
   // Returns a worker context provider that will be bound on the compositor
   // thread.
   scoped_refptr<viz::RasterContextProvider>
@@ -508,6 +515,9 @@ class CONTENT_EXPORT RenderThreadImpl
   // Sets the current pipeline rendering color space.
   void SetRenderingColorSpace(const gfx::ColorSpace& color_space);
 
+  scoped_refptr<base::SingleThreadTaskRunner>
+  CreateVideoFrameCompositorTaskRunner();
+
  private:
   void OnProcessFinalRelease() override;
   // IPC::Listener
@@ -676,6 +686,10 @@ class CONTENT_EXPORT RenderThreadImpl
   // regardless of whether |compositor_thread_| is overriden.
   scoped_refptr<base::SingleThreadTaskRunner> compositor_task_runner_;
 
+  // Task to run the VideoFrameCompositor on.
+  scoped_refptr<base::SingleThreadTaskRunner>
+      video_frame_compositor_task_runner_;
+
   // Pool of workers used for raster operations (e.g., tile rasterization).
   scoped_refptr<CategorizedWorkerPool> categorized_worker_pool_;
 
@@ -687,6 +701,8 @@ class CONTENT_EXPORT RenderThreadImpl
 
   base::ObserverList<RenderThreadObserver>::Unchecked observers_;
 
+  scoped_refptr<viz::ContextProvider> video_frame_compositor_context_provider_;
+
   scoped_refptr<viz::RasterContextProvider> shared_worker_context_provider_;
 
   std::unique_ptr<AudioRendererMixerManager> audio_renderer_mixer_manager_;

third_party/blink/public/platform/web_video_frame_submitter.h

@@ -14,17 +14,20 @@ namespace cc {
 class LayerTreeSettings;
 }
 
-namespace ws {
-class ContextProviderCommandBuffer;
-}  // namespace ui
+namespace viz {
+class ContextProvider;
+}  // namespace viz
 
 namespace blink {
 
+// Sets the proper context_provider and compositing mode onto the Submitter.
+using WebSubmitterConfigurationCallback =
+    base::OnceCallback<void(bool, scoped_refptr<viz::ContextProvider>)>;
 // Callback to obtain the media ContextProvider and a bool indicating whether
 // we are in software compositing mode.
-using WebContextProviderCallback = base::RepeatingCallback<void(
-    base::OnceCallback<void(bool,
-                            scoped_refptr<ws::ContextProviderCommandBuffer>)>)>;
+using WebContextProviderCallback =
+    base::RepeatingCallback<void(scoped_refptr<viz::ContextProvider>,
+                                 WebSubmitterConfigurationCallback)>;
 using WebFrameSinkDestroyedCallback = base::RepeatingCallback<void()>;
 
 // Exposes the VideoFrameSubmitter, which submits CompositorFrames containing

third_party/blink/renderer/platform/graphics/DEPS

@@ -7,6 +7,7 @@ include_rules = [
 
     # Dependencies.
     "+base/threading/sequenced_task_runner_handle.h",
+    "+base/threading/thread_restrictions.h",
     "+cc",
     "+components/viz/client",
     "+components/viz/common",

third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc

@@ -6,6 +6,7 @@
 
 #include <memory>
 #include "base/bind.h"
+#include "base/threading/thread_restrictions.h"
 #include "base/trace_event/trace_event.h"
 #include "components/viz/client/client_resource_provider.h"
 #include "components/viz/common/gpu/context_provider.h"
@@ -92,7 +93,11 @@ void VideoFrameResourceProvider::AppendQuads(
       break;
   }
 
+  // When obtaining frame resources, we end up having to wait. See
+  // https://crbug/878070.
+  base::ScopedAllowBaseSyncPrimitives allow_base_sync_primitives;
   resource_updater_->ObtainFrameResources(frame);
+
   // TODO(lethalantidote) : update with true value;
   gfx::Rect visible_layer_rect = gfx::Rect(rotated_size);
   gfx::Rect clip_rect = gfx::Rect(frame->coded_size());

third_party/blink/renderer/platform/graphics/video_frame_resource_provider.h

@@ -56,7 +56,6 @@ class PLATFORM_EXPORT VideoFrameResourceProvider {
  private:
   const cc::LayerTreeSettings settings_;
 
-  WebContextProviderCallback context_provider_callback_;
   viz::ContextProvider* context_provider_;
   std::unique_ptr<viz::ClientResourceProvider> resource_provider_;
   std::unique_ptr<media::VideoResourceUpdater> resource_updater_;

third_party/blink/renderer/platform/graphics/video_frame_submitter.cc

@@ -25,7 +25,8 @@ namespace blink {
 namespace {
 
 // Delay to retry getting the context_provider.
-constexpr int kGetContextProviderRetryMS = 150;
+constexpr base::TimeDelta kGetContextProviderRetryTimeout =
+    base::TimeDelta::FromMilliseconds(150);
 
 }  // namespace
 
@@ -162,33 +163,45 @@ void VideoFrameSubmitter::Initialize(cc::VideoFrameProvider* provider) {
     DCHECK(!provider_);
     provider_ = provider;
     context_provider_callback_.Run(
-        base::BindOnce(&VideoFrameSubmitter::OnReceivedContextProvider,
-                       weak_ptr_factory_.GetWeakPtr()));
+        nullptr, base::BindOnce(&VideoFrameSubmitter::OnReceivedContextProvider,
+                                weak_ptr_factory_.GetWeakPtr()));
   }
 }
 
 void VideoFrameSubmitter::OnReceivedContextProvider(
     bool use_gpu_compositing,
-    scoped_refptr<ws::ContextProviderCommandBuffer> context_provider) {
-  // We could get a null |context_provider| back if the context is still lost.
-  if (!context_provider && use_gpu_compositing) {
-    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-        FROM_HERE,
-        base::BindOnce(
-            context_provider_callback_,
-            base::BindOnce(&VideoFrameSubmitter::OnReceivedContextProvider,
-                           weak_ptr_factory_.GetWeakPtr())),
-        base::TimeDelta::FromMilliseconds(kGetContextProviderRetryMS));
+    scoped_refptr<viz::ContextProvider> context_provider) {
+  if (!use_gpu_compositing) {
+    resource_provider_->Initialize(nullptr, this);
     return;
   }
 
-  context_provider_ = std::move(context_provider);
-  if (use_gpu_compositing) {
-    context_provider_->AddObserver(this);
-    resource_provider_->Initialize(context_provider_.get(), nullptr);
-  } else {
-    resource_provider_->Initialize(nullptr, this);
+  bool has_good_context = false;
+  while (!has_good_context) {
+    if (!context_provider) {
+      base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+          FROM_HERE,
+          base::BindOnce(
+              context_provider_callback_, context_provider_,
+              base::BindOnce(&VideoFrameSubmitter::OnReceivedContextProvider,
+                             weak_ptr_factory_.GetWeakPtr())),
+          kGetContextProviderRetryTimeout);
+      return;
+    }
+
+    // Note that |context_provider| is now null after the move, such that if we
+    // end up having !|has_good_context|, we will retry to obtain the
+    // context_provider.
+    context_provider_ = std::move(context_provider);
+    auto result = context_provider_->BindToCurrentThread();
+
+    has_good_context =
+        result == gpu::ContextResult::kSuccess ||
+        context_provider_->ContextGL()->GetGraphicsResetStatusKHR() ==
+            GL_NO_ERROR;
   }
+  context_provider_->AddObserver(this);
+  resource_provider_->Initialize(context_provider_.get(), nullptr);
 
   if (frame_sink_id_.is_valid())
     StartSubmitting();
@@ -339,7 +352,6 @@ void VideoFrameSubmitter::OnContextLost() {
 
   if (context_provider_) {
     context_provider_->RemoveObserver(this);
-    context_provider_ = nullptr;
   }
   waiting_for_compositor_ack_ = false;
 
@@ -349,6 +361,7 @@ void VideoFrameSubmitter::OnContextLost() {
   compositor_frame_sink_.reset();
 
   context_provider_callback_.Run(
+      context_provider_,
       base::BindOnce(&VideoFrameSubmitter::OnReceivedContextProvider,
                      weak_ptr_factory_.GetWeakPtr()));
 

third_party/blink/renderer/platform/graphics/video_frame_submitter.h

@@ -47,9 +47,7 @@ class PLATFORM_EXPORT VideoFrameSubmitter
     return &binding_;
   }
 
-  void OnReceivedContextProvider(
-      bool,
-      scoped_refptr<ws::ContextProviderCommandBuffer>);
+  void OnReceivedContextProvider(bool, scoped_refptr<viz::ContextProvider>);
 
   // cc::VideoFrameProvider::Client implementation.
   void StopUsingProvider() override;
@@ -118,7 +116,7 @@ class PLATFORM_EXPORT VideoFrameSubmitter
   bool ShouldSubmit() const;
 
   cc::VideoFrameProvider* provider_ = nullptr;
-  scoped_refptr<ws::ContextProviderCommandBuffer> context_provider_;
+  scoped_refptr<viz::ContextProvider> context_provider_;
   viz::mojom::blink::CompositorFrameSinkPtr compositor_frame_sink_;
   mojo::Binding<viz::mojom::blink::CompositorFrameSinkClient> binding_;
   WebContextProviderCallback context_provider_callback_;

third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc

@@ -148,8 +148,9 @@ class VideoFrameSubmitterTest : public testing::Test {
         context_provider_.get(), nullptr);
     submitter_ = std::make_unique<VideoFrameSubmitter>(
         base::BindRepeating(
-            [](base::OnceCallback<void(
-                   bool, scoped_refptr<ws::ContextProviderCommandBuffer>)>) {}),
+            [](scoped_refptr<viz::ContextProvider>,
+               base::OnceCallback<void(
+                   bool, scoped_refptr<viz::ContextProvider>)>) {}),
         base::WrapUnique<MockVideoFrameResourceProvider>(resource_provider_));
 
     submitter_->Initialize(provider_.get());