android: Pass cc::ContextProvider to VideoContextProvider.

content/browser/android/in_process/synchronous_compositor_factory_impl.cc

Index: content/browser/android/in_process/synchronous_compositor_factory_impl.cc
diff --git a/content/browser/android/in_process/synchronous_compositor_factory_impl.cc b/content/browser/android/in_process/synchronous_compositor_factory_impl.cc
index 470743a547ca066e1f434035ec6e4df9038b3b64..70eea5eb575beb058236bd4f1dcaa8bcbd33762e 100644
--- a/content/browser/android/in_process/synchronous_compositor_factory_impl.cc
+++ b/content/browser/android/in_process/synchronous_compositor_factory_impl.cc
@@ -82,12 +82,10 @@ scoped_ptr<WebGraphicsContext3DInProcessCommandBufferImpl> WrapContext(
 class SynchronousCompositorFactoryImpl::VideoContextProvider
     : public StreamTextureFactorySynchronousImpl::ContextProvider {
  public:
-  VideoContextProvider(
-      scoped_ptr<gpu::GLInProcessContext> gl_in_process_context)
-      : gl_in_process_context_(gl_in_process_context.get()) {
-    context_provider_ = webkit::gpu::ContextProviderInProcess::Create(
-        WrapContext(gl_in_process_context.Pass(), GetDefaultAttribs()),
-        "Video-Offscreen-main-thread");
+  VideoContextProvider(gpu::GLInProcessContext* gl_in_process_context,
+                       scoped_refptr<cc::ContextProvider> context_provider)
+      : gl_in_process_context_(gl_in_process_context),
+        context_provider_(context_provider) {
     context_provider_->BindToCurrentThread();
   }
 
@@ -118,8 +116,8 @@ class SynchronousCompositorFactoryImpl::VideoContextProvider
   friend class base::RefCountedThreadSafe<VideoContextProvider>;
   ~VideoContextProvider() override {}
 
-  scoped_refptr<cc::ContextProvider> context_provider_;
   gpu::GLInProcessContext* gl_in_process_context_;

[boliu, 2015/04/08 20:24:26]

Why the reorder? In theory, this would lead to a moment during destruction where
gl_in_process_context_ is a dangling pointer, since member variables are
destroyed in the reverse order they are declared.

+  scoped_refptr<cc::ContextProvider> context_provider_;
   ObserverList<StreamTextureFactoryContextObserver> observer_list_;
 
   DISALLOW_COPY_AND_ASSIGN(VideoContextProvider);
@@ -262,9 +260,13 @@ SynchronousCompositorFactoryImpl::TryCreateStreamTextureFactory() {
     attributes.shareResources = false;
     // This needs to run in on-screen |service_| context due to SurfaceTexture
     // limitations.
-    video_context_provider_ = new VideoContextProvider(CreateContextHolder(
-        attributes, service_, gpu::GLInProcessContextSharedMemoryLimits(),
-        false));
+    scoped_ptr<gpu::GLInProcessContext> context =
+        CreateContextHolder(attributes, service_,
+                            gpu::GLInProcessContextSharedMemoryLimits(), false);
+    video_context_provider_ = new VideoContextProvider(
+        context.get(), webkit::gpu::ContextProviderInProcess::Create(

[boliu, 2015/04/08 20:24:26]

Just save the raw pointer in a local variable. This is really brittle if someone
reorders the arguments in the future, and don't look closely at what this is
doing.

[tfarina, 2015/04/08 20:26:25]

Unfortunately that didn't work very well with WrapContext, which takes a
scoped_ptr, not a raw pointer. Am I missing something?

[boliu, 2015/04/08 20:29:58]

This should work (ignore poor formatting):

    scoped_ptr<gpu::GLInProcessContext> context =
        CreateContextHolder(attributes, service_,
                            gpu::GLInProcessContextSharedMemoryLimits(), false);
    gpu::GLInProcessContext* context_ptr = context.get();
    video_context_provider_ = new VideoContextProvider(
        webkit::gpu::ContextProviderInProcess::Create(
                           WrapContext(context.Pass(), GetDefaultAttribs()),
                           "Video-Offscreen-main-thread"), context_ptr);

[tfarina, 2015/04/08 20:56:46]

Cool! thanks. Done.

+                           WrapContext(context.Pass(), GetDefaultAttribs()),
+                           "Video-Offscreen-main-thread"));
   }
   return video_context_provider_;
 }