Android media: VideoFrame should not store so many sync points.

content/browser/renderer_host/image_transport_factory_android.cc

 // Copyright (c) 2012 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 "content/browser/renderer_host/image_transport_factory_android.h"
 
 #include "base/lazy_instance.h"
 #include "base/strings/stringprintf.h"
+#include "cc/output/context_provider.h"
 #include "content/browser/gpu/browser_gpu_channel_host_factory.h"
 #include "content/common/gpu/client/gl_helper.h"
 #include "content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.h"
 #include "content/common/gpu/gpu_process_launch_causes.h"
 #include "gpu/command_buffer/client/gles2_implementation.h"
 #include "third_party/WebKit/public/platform/WebGraphicsContext3D.h"
 #include "third_party/khronos/GLES2/gl2.h"
+#include "ui/compositor/compositor.h"
 #include "ui/gfx/android/device_display_info.h"
 
 namespace content {
 
 base::LazyInstance<ObserverList<ImageTransportFactoryAndroidObserver> >::Leaky
     g_factory_observers = LAZY_INSTANCE_INITIALIZER;
 
 class GLContextLostListener
     : public blink::WebGraphicsContext3D::WebGraphicsContextLostCallback {
  public:
   // WebGraphicsContextLostCallback implementation.
   virtual void onContextLost() OVERRIDE;
  private:
   static void DidLoseContext();
 };
 
 namespace {
 
+bool g_initialized_for_unit_tests = false;
 static ImageTransportFactoryAndroid* g_factory = NULL;
 
 class CmdBufferImageTransportFactory : public ImageTransportFactoryAndroid {
  public:
   CmdBufferImageTransportFactory();
   virtual ~CmdBufferImageTransportFactory();
 
   virtual GLHelper* GetGLHelper() OVERRIDE;
   virtual uint32 GetChannelID() OVERRIDE {
     return BrowserGpuChannelHostFactory::instance()->GetGpuChannelId();
@@ skipping 50 matching lines @@
 }
 
 GLHelper* CmdBufferImageTransportFactory::GetGLHelper() {
   if (!gl_helper_)
     gl_helper_.reset(new GLHelper(context_->GetImplementation(),
                                   context_->GetContextSupport()));
 
   return gl_helper_.get();
 }
 
+class NoImageTransportFactory : public ImageTransportFactoryAndroid {

[dshwang, 2014/06/24 14:04:21]

This class is very similar to NoTransportImageTransportFactory except for
inheriting ImageTransportFactoryAndroid.

[danakj, 2014/07/02 21:43:03]

one class per file, can you move this out?

+ public:
+  explicit NoImageTransportFactory(
+      scoped_ptr<ui::ContextFactory> context_factory)
+      : context_factory_(context_factory.Pass()) {}
+  virtual ~NoImageTransportFactory() {}
+
+  virtual GLHelper* GetGLHelper() OVERRIDE {
+    if (!gl_helper_) {
+      context_provider_ = context_factory_->SharedMainThreadContextProvider();
+      gl_helper_.reset(new GLHelper(context_provider_->ContextGL(),
+                                    context_provider_->ContextSupport()));
+    }
+    return gl_helper_.get();
+  }
+  virtual uint32 GetChannelID() OVERRIDE {
+    NOTREACHED();
+    return 0;
+  }
+
+ private:
+  scoped_ptr<ui::ContextFactory> context_factory_;
+  scoped_refptr<cc::ContextProvider> context_provider_;
+  scoped_ptr<GLHelper> gl_helper_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(NoImageTransportFactory);
+};
+
 }  // anonymous namespace
 
 // static
+void ImageTransportFactoryAndroid::InitializeForUnitTests(

[dshwang, 2014/06/24 14:04:21]

Implement it like ImageTransportFactory::InitializeForUnitTests() in order to
use it in unittests.

+    scoped_ptr<ui::ContextFactory> test_factory) {
+  DCHECK(!g_factory);
+  g_initialized_for_unit_tests = true;
+  g_factory = new NoImageTransportFactory(test_factory.Pass());
+}
+
+// static
 ImageTransportFactoryAndroid* ImageTransportFactoryAndroid::GetInstance() {
-  if (!g_factory)
+  if (!g_factory) {
+    DCHECK(!g_initialized_for_unit_tests);

[danakj, 2014/07/02 21:43:03]

i don't think this DCHECK buys much, since there's no Initialize() for this
class unlike the other ITF. what are you trying to guard against? i think just
remove it and the global bool.

what would be better is if this whole NoImageTransportFactory class was only
compiled into test targets. can we do that instead?

     g_factory = new CmdBufferImageTransportFactory();
-
+  }
   return g_factory;
 }
 
 ImageTransportFactoryAndroid::ImageTransportFactoryAndroid()
     : context_lost_listener_(new GLContextLostListener()) {}
 
 ImageTransportFactoryAndroid::~ImageTransportFactoryAndroid() {}
 
 void ImageTransportFactoryAndroid::AddObserver(
     ImageTransportFactoryAndroidObserver* observer) {
@@ skipping 16 matching lines @@
 
 void GLContextLostListener::DidLoseContext() {
   delete g_factory;
   g_factory = NULL;
   FOR_EACH_OBSERVER(ImageTransportFactoryAndroidObserver,
       g_factory_observers.Get(),
       OnLostResources());
 }
 
 } // namespace content