Clean up compositor initialization/destruction.

Moved from https://codereview.chromium.org/21052007/

Currently the ContextFactory is outliving the compositor thread in
tests, which is bad since the contexts in there can be bound to the
compositor thread. Then we end up reusing those contexts and bad
things happen.

This enforces ordering for initializing and destroying the
compositor as follows:

Initialize ContextFactory
..Compositor::Initialize
....Create Compositor instances
....Delete Compositor instances
..Compositor::Terminate

The Compositor::Terminate call also destroys the ContextFactory.

The ContextFactory can be initialized in one of two ways:
1. ImageTransportFactory::Initialize will set up the
ContextFactory. This is used by things that invoke all of
content/browser/.
2. Compositor::InitializeContextFactoryForTests() must be
explicitly called by any unit tests that create a compositor.

Since some tests want a real GL context and some don't, this
does away with the misnomer of initializing the Compositor once
for the entire test suite, and then re-initializing somewhere
in the middle of the test suite. Instead, each test must
Initialize/Terminate the compositor with the ContextFactory
type of its choice.

Incidently, this test enables 20 tests on aura or win_aura that
were previously being skipped.

TBR=piman@chromium.org
BUG=258625, 266565

Review URL: https://codereview.chromium.org/22293003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@216179 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 75 insertions, 55 deletions

diff --git a/content/browser/aura/gpu_process_transport_factory.cc b/content/browser/aura/gpu_process_transport_factory.cc
index dda8bc878..bc9e414 100644
--- a/content/browser/aura/gpu_process_transport_factory.cc
+++ b/content/browser/aura/gpu_process_transport_factory.cc
@@ -319,6 +319,8 @@ void GpuProcessTransportFactory::RemoveCompositor(ui::Compositor* compositor) {
     gl_helper_.reset();
 }
 
+bool GpuProcessTransportFactory::DoesCreateTestContexts() { return false; }
+
 ui::ContextFactory* GpuProcessTransportFactory::AsContextFactory() {
   return this;
 }
diff --git a/content/browser/aura/gpu_process_transport_factory.h b/content/browser/aura/gpu_process_transport_factory.h
index 9e40b5c..72b88d5 100644
--- a/content/browser/aura/gpu_process_transport_factory.h
+++ b/content/browser/aura/gpu_process_transport_factory.h
@@ -35,7 +35,7 @@ class GpuProcessTransportFactory
   scoped_ptr<WebGraphicsContext3DCommandBufferImpl>
   CreateOffscreenCommandBufferContext();
 
-  // ImageTransportFactory implementation.
+  // ContextFactory implementation.
   virtual scoped_ptr<WebKit::WebGraphicsContext3D> CreateOffscreenContext()
       OVERRIDE;
   virtual scoped_ptr<cc::OutputSurface> CreateOutputSurface(
@@ -46,6 +46,9 @@ class GpuProcessTransportFactory
   virtual void RemoveReflector(
       scoped_refptr<ui::Reflector> reflector) OVERRIDE;
   virtual void RemoveCompositor(ui::Compositor* compositor) OVERRIDE;
+  virtual bool DoesCreateTestContexts() OVERRIDE;
+
+  // ImageTransportFactory implementation.
   virtual ui::ContextFactory* AsContextFactory() OVERRIDE;
   virtual gfx::GLSurfaceHandle CreateSharedSurfaceHandle() OVERRIDE;
   virtual void DestroySharedSurfaceHandle(
diff --git a/content/browser/aura/image_transport_factory.cc b/content/browser/aura/image_transport_factory.cc
index 2e92dd0..28b04bf 100644
--- a/content/browser/aura/image_transport_factory.cc
+++ b/content/browser/aura/image_transport_factory.cc
@@ -8,7 +8,12 @@
 #include "content/browser/aura/gpu_process_transport_factory.h"
 #include "content/browser/aura/no_transport_image_transport_factory.h"
 #include "content/public/common/content_switches.h"
-#include "ui/compositor/compositor_setup.h"
+#include "ui/compositor/compositor.h"
+#include "ui/compositor/compositor_switches.h"
+
+#if defined(OS_CHROMEOS)
+#include "base/chromeos/chromeos_version.h"
+#endif
 
 namespace content {
 
@@ -16,15 +21,32 @@ namespace {
 ImageTransportFactory* g_factory;
 }
 
+
+static bool UseTestContextAndTransportFactory() {
+#if defined(OS_CHROMEOS)
+  // If the test is running on the chromeos envrionment (such as
+  // device or vm bots), always use real contexts.
+  if (base::chromeos::IsRunningOnChromeOS())
+    return false;
+#endif
+
+  // Only used if the enable command line flag is used.
+  CommandLine* command_line = CommandLine::ForCurrentProcess();
+  if (!command_line->HasSwitch(switches::kTestCompositor))
+    return false;
+
+  // The disable command line flag preempts the enable flag.
+  if (!command_line->HasSwitch(switches::kDisableTestCompositor))
+    return true;
+
+  return false;
+}
+
 // static
 void ImageTransportFactory::Initialize() {
-  CommandLine* command_line = CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kTestCompositor)) {
-    ui::SetupTestCompositor();
-  }
-  if (ui::IsTestCompositorEnabled()) {
-    g_factory = new NoTransportImageTransportFactory(
-        new ui::TestContextFactory);
+  if (UseTestContextAndTransportFactory()) {
+    g_factory =
+        new NoTransportImageTransportFactory(new ui::TestContextFactory);
   } else {
     g_factory = new GpuProcessTransportFactory;
   }
diff --git a/content/browser/browser_plugin/browser_plugin_host_browsertest.cc b/content/browser/browser_plugin/browser_plugin_host_browsertest.cc
index 5e6d887f..4fc2ae8 100644
--- a/content/browser/browser_plugin/browser_plugin_host_browsertest.cc
+++ b/content/browser/browser_plugin/browser_plugin_host_browsertest.cc
@@ -247,6 +247,11 @@ class BrowserPluginHostTest : public ContentBrowserTest {
     content::BrowserPluginGuestManager::set_factory_for_testing(
         TestBrowserPluginHostFactory::GetInstance());
 
+    // On legacy windows, the AcceptDragEvents test needs this to pass.
+#if defined(OS_WIN) && !defined(USE_AURA)
+    UseRealGLBindings();
+#endif
+
     ContentBrowserTest::SetUp();
   }
   virtual void TearDown() OVERRIDE {
diff --git a/content/browser/gpu/gpu_info_browsertest.cc b/content/browser/gpu/gpu_info_browsertest.cc
index 72487d4..d555d1d 100644
--- a/content/browser/gpu/gpu_info_browsertest.cc
+++ b/content/browser/gpu/gpu_info_browsertest.cc
@@ -11,7 +11,6 @@
 #include "content/public/browser/gpu_data_manager_observer.h"
 #include "content/public/common/content_switches.h"
 #include "content/test/content_browser_test.h"
-#include "ui/compositor/compositor_setup.h"
 
 namespace content {
 
@@ -74,9 +73,11 @@ class GpuInfoBrowserTest : public ContentBrowserTest {
       : message_loop_(base::MessageLoop::TYPE_UI) {
   }
 
-  virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
-    ContentBrowserTest::SetUpInProcessBrowserTestFixture();
-    ui::DisableTestCompositor();
+  virtual void SetUp() {
+    // We expect real pixel output for these tests.
+    UseRealGLContexts();
+
+    ContentBrowserTest::SetUp();
   }
 
   base::MessageLoop* GetMessageLoop() { return &message_loop_; }
diff --git a/content/browser/gpu/gpu_ipc_browsertests.cc b/content/browser/gpu/gpu_ipc_browsertests.cc
index 7b7186c..0ee93db 100644
--- a/content/browser/gpu/gpu_ipc_browsertests.cc
+++ b/content/browser/gpu/gpu_ipc_browsertests.cc
@@ -26,18 +26,6 @@ class ContextTestBase : public content::ContentBrowserTest {
     ContentBrowserTest::SetUpOnMainThread();
   }
 
-  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
-    // TODO(Hubbe): This code is very similar to some code in
-    // test/gpu/gpu_feature_browsertest.cc, and should really be
-    // shared in a common location.
-    if (!command_line->HasSwitch(switches::kUseGpuInTests)) {
-      CHECK(!command_line->HasSwitch(switches::kUseGL))
-          << "kUseGL must not be set by test framework code!";
-      command_line->AppendSwitchASCII(switches::kUseGL,
-                                      gfx::kGLImplementationOSMesaName);
-    }
-  }
-
   virtual void TearDownOnMainThread() OVERRIDE {
     // Must delete the context first.
     context_.reset(NULL);
diff --git a/content/browser/gpu/gpu_pixel_browsertest.cc b/content/browser/gpu/gpu_pixel_browsertest.cc
index dc0b91f..5909321 100644
--- a/content/browser/gpu/gpu_pixel_browsertest.cc
+++ b/content/browser/gpu/gpu_pixel_browsertest.cc
@@ -24,7 +24,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkColor.h"
-#include "ui/compositor/compositor_setup.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/size.h"
 #include "ui/gl/gl_switches.h"
@@ -106,6 +105,13 @@ class GpuPixelBrowserTest : public ContentBrowserTest {
         ref_img_option_(kReferenceImageNone) {
   }
 
+  virtual void SetUp() {
+    // We expect real pixel output for these tests.
+    UseRealGLContexts();
+
+    ContentBrowserTest::SetUp();
+  }
+
   virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
     command_line->AppendSwitchASCII(switches::kTestGLLib,
                                     "libllvmpipe.so");
@@ -150,8 +156,6 @@ class GpuPixelBrowserTest : public ContentBrowserTest {
       ReplaceFirstSubstringAfterOffset(
           &test_name_, 0, test_status_prefixes[i], std::string());
     }
-
-    ui::DisableTestCompositor();
   }
 
   // If the existing ref image was saved from an revision older than the
diff --git a/content/browser/media/media_browsertest.cc b/content/browser/media/media_browsertest.cc
index e5bc620..1d83035 100644
--- a/content/browser/media/media_browsertest.cc
+++ b/content/browser/media/media_browsertest.cc
@@ -26,6 +26,14 @@ const char MediaBrowserTest::kEnded[] = "ENDED";
 const char MediaBrowserTest::kError[] = "ERROR";
 const char MediaBrowserTest::kFailed[] = "FAILED";
 
+void MediaBrowserTest::SetUp() {
+  // TODO(danakj): The GPU Video Decoder needs real GL bindings.
+  // crbug.com/269087
+  UseRealGLBindings();
+
+  ContentBrowserTest::SetUp();
+}
+
 void MediaBrowserTest::RunMediaTestPage(
     const char* html_page, std::vector<StringPair>* query_params,
     const char* expected, bool http) {
diff --git a/content/browser/media/media_browsertest.h b/content/browser/media/media_browsertest.h
index 5da684e..013283b 100644
--- a/content/browser/media/media_browsertest.h
+++ b/content/browser/media/media_browsertest.h
@@ -17,6 +17,8 @@ class MediaBrowserTest : public ContentBrowserTest {
 
   typedef std::pair<const char*, const char*> StringPair;
 
+  virtual void SetUp() OVERRIDE;
+
   // Runs a html page with a list of URL query parameters.
   // If http is true, the test starts a local http test server to load the test
   // page, otherwise a local file URL is loaded inside the content shell.
diff --git a/content/browser/renderer_host/render_widget_host_view_browsertest.cc b/content/browser/renderer_host/render_widget_host_view_browsertest.cc
index 5dc1ed4..3985cb0 100644
--- a/content/browser/renderer_host/render_widget_host_view_browsertest.cc
+++ b/content/browser/renderer_host/render_widget_host_view_browsertest.cc
@@ -28,7 +28,6 @@
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkDevice.h"
 #include "ui/base/ui_base_switches.h"
-#include "ui/compositor/compositor_setup.h"
 #include "ui/gfx/size_conversions.h"
 #include "ui/gl/gl_switches.h"
 
@@ -212,6 +211,18 @@ class RenderWidgetHostViewBrowserTest : public ContentBrowserTest {
 class CompositingRenderWidgetHostViewBrowserTest
     : public RenderWidgetHostViewBrowserTest {
  public:
+  virtual void SetUp() OVERRIDE {
+    // We expect real pixel output for these tests.
+    UseRealGLContexts();
+
+    // On legacy windows, these tests need real GL bindings to pass.
+#if defined(OS_WIN) && !defined(USE_AURA)
+    UseRealGLBindings();
+#endif
+
+    RenderWidgetHostViewBrowserTest::SetUp();
+  }
+
   virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
     // Note: Not appending kForceCompositingMode switch here, since not all bots
     // support compositing.  Some bots will run with compositing on, and others
@@ -414,15 +425,6 @@ IN_PROC_BROWSER_TEST_F(CompositingRenderWidgetHostViewBrowserTest,
                      "supported on this platform.");
     return;
   }
-#if defined(USE_AURA)
-  if (ui::IsTestCompositorEnabled()) {
-    LOG(WARNING) << ("Blindly passing this test: Aura test compositor doesn't "
-                     "support frame subscription.");
-    // TODO(miu): Aura test compositor should support frame subscription for
-    // testing.  http://crbug.com/240572
-    return;
-  }
-#endif
 
   base::RunLoop run_loop;
   scoped_ptr<RenderWidgetHostViewFrameSubscriber> subscriber(
@@ -496,23 +498,6 @@ class CompositingRenderWidgetHostViewBrowserTestTabCapture
         allowable_error_(0),
         test_url_("data:text/html,<!doctype html>") {}
 
-  virtual void SetUp() OVERRIDE {
-    ui::DisableTestCompositor();
-    CompositingRenderWidgetHostViewBrowserTest::SetUp();
-  }
-
-  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
-    // TODO(Hubbe): This code is very similar to some code in
-    // gpu_feature_browsertest.cc, and should really be shared in a common
-    // location.
-    if (!command_line->HasSwitch(switches::kUseGpuInTests)) {
-      CHECK(!command_line->HasSwitch(switches::kUseGL))
-          << "kUseGL must not be set by test framework code!";
-      command_line->AppendSwitchASCII(switches::kUseGL,
-                                      gfx::kGLImplementationOSMesaName);
-    }
-  }
-
   void CopyFromCompositingSurfaceCallback(base::Closure quit_callback,
                                           bool result,
                                           const SkBitmap& bitmap) {