Add an ExtensionBrowserTest base class that allows in-process browser tests of extensions using ExtensionsService directly, rather than TestExtensionLoaded. Use it to re-enable some old browser tests that had been disabled.

Review URL: http://codereview.chromium.org/150213

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

10 files changed, 284 insertions, 328 deletions

diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index 74a9fb5..3dcef8b 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -1,173 +1,116 @@
-// Copyright (c) 2009 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 "chrome/browser/extensions/extension_browsertest.h"
 
-#include "base/ref_counted.h"
+#include "base/command_line.h"
+#include "base/file_path.h"
+#include "base/path_service.h"
 #include "chrome/browser/browser.h"
-#include "chrome/browser/browser_list.h"
-#include "chrome/browser/renderer_host/render_view_host.h"
 #include "chrome/browser/extensions/extension_host.h"
-#include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extensions_service.h"
-#include "chrome/browser/extensions/test_extension_loader.h"
 #include "chrome/browser/profile.h"
-#include "chrome/browser/renderer_host/site_instance.h"
-#include "chrome/browser/views/extensions/extension_shelf.h"
-#include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/common/extensions/extension_error_reporter.h"
-#include "chrome/common/url_constants.h"
-#include "chrome/test/in_process_browser_test.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/notification_registrar.h"
+#include "chrome/common/notification_service.h"
+#include "chrome/common/notification_type.h"
 #include "chrome/test/ui_test_utils.h"
 
 namespace {
+// Amount of time to wait to load an extension. This is purposely obscenely
+// long because it will only get used in the case of failure and we want to
+// minimize false positives.
+static const int kTimeoutMs = 60 * 1000;  // 1 minute
+};
 
-// How long to wait for the extension to put up a javascript alert before giving
-// up.
-const int kAlertTimeoutMs = 20000;
-
-// The extensions we're using as our test case.
-const char* kGoodExtension1Id = "behllobkkfkfnphdnhnkndlbkcpglgmj";
-const char* kGoodCrxId        = "ldnnhddmnhbkjipkidpdiheffobcpfmf";
-
-};  // namespace
-
-// This class starts up an extension process and waits until it tries to put
-// up a javascript alert.
-class MockExtensionHost : public ExtensionHost {
- public:
-  MockExtensionHost(Extension* extension, const GURL& url,
-                    SiteInstance* instance)
-      : ExtensionHost(extension, instance, url),
-        got_message_(false) {
-    CreateRenderView(NULL);
-    MessageLoop::current()->PostDelayedTask(FROM_HERE,
-        new MessageLoop::QuitTask, kAlertTimeoutMs);
-    ui_test_utils::RunMessageLoop();
-  }
+// Base class for extension browser tests. Provides utilities for loading,
+// unloading, and installing extensions.
+void ExtensionBrowserTest::SetUpCommandLine(CommandLine* command_line) {
+  // This enables DOM automation for tab contentses.
+  EnableDOMAutomation();
 
-  virtual ExtensionFunctionDispatcher* CreateExtensionFunctionDispatcher(
-      RenderViewHost* render_view_host) {
-    NOTREACHED();
-    return NULL;
-  }
+  // This enables it for extension hosts.
+  ExtensionHost::EnableDOMAutomation();
 
-  bool got_message() { return got_message_; }
- private:
-  virtual void RunJavaScriptMessage(
-      const std::wstring& message,
-      const std::wstring& default_prompt,
-      const GURL& frame_url,
-      const int flags,
-      IPC::Message* reply_msg,
-      bool* did_suppress_message) {
-    got_message_ = true;
-    MessageLoopForUI::current()->Quit();
-
-    // Call super, otherwise we'll leak reply_msg.
-    ExtensionHost::RunJavaScriptMessage(
-        message, default_prompt, frame_url, flags,
-        reply_msg, did_suppress_message);
-  }
+  command_line->AppendSwitch(switches::kEnableExtensions);
 
-  bool got_message_;
-};
+  PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir_);
+  test_data_dir_ = test_data_dir_.AppendASCII("extensions");
+}
 
-class ExtensionViewTest : public InProcessBrowserTest {
- public:
-  virtual void SetUp() {
-    // Initialize the error reporter here, otherwise BrowserMain will create it
-    // with the wrong MessageLoop.
-    ExtensionErrorReporter::Init(false);
+bool ExtensionBrowserTest::LoadExtension(const FilePath& path) {
+  ExtensionsService* service = browser()->profile()->GetExtensionsService();
+  size_t num_before = service->extensions()->size();
+  registrar_.Add(this, NotificationType::EXTENSIONS_LOADED,
+                 NotificationService::AllSources());
+  service->LoadExtension(path);
+  MessageLoop::current()->PostDelayedTask(FROM_HERE, new MessageLoop::QuitTask,
+      kTimeoutMs);
+  ui_test_utils::RunMessageLoop();
+  registrar_.Remove(this, NotificationType::EXTENSIONS_LOADED,
+                    NotificationService::AllSources());
+  size_t num_after = service->extensions()->size();
+  if (num_after != (num_before + 1))
+    return false;
+
+  return WaitForExtensionHostsToLoad();
+}
 
-    InProcessBrowserTest::SetUp();
-  }
-  virtual void SetUpCommandLine(CommandLine* command_line) {
-    command_line->AppendSwitch(switches::kEnableExtensions);
-  }
-};
+bool ExtensionBrowserTest::InstallExtension(const FilePath& path) {
+  ExtensionsService* service = browser()->profile()->GetExtensionsService();
+  service->set_show_extensions_prompts(false);
+  size_t num_before = service->extensions()->size();
+
+  registrar_.Add(this, NotificationType::EXTENSION_INSTALLED,
+                 NotificationService::AllSources());
+  service->InstallExtension(path);
+  MessageLoop::current()->PostDelayedTask(FROM_HERE, new MessageLoop::QuitTask,
+      kTimeoutMs);
+  ui_test_utils::RunMessageLoop();
+  registrar_.Remove(this, NotificationType::EXTENSION_INSTALLED,
+                    NotificationService::AllSources());
+  size_t num_after = service->extensions()->size();
+  if (num_after != (num_before + 1))
+    return false;
+
+  return WaitForExtensionHostsToLoad();
+}
 
-// Tests that ExtensionView starts an extension process and runs the script
-// contained in the extension's toolstrip.
-// TODO(mpcomplete): http://crbug.com/15081 Disabled because it fails.
-IN_PROC_BROWSER_TEST_F(ExtensionViewTest, DISABLED_Toolstrip) {
-  // Get the path to our extension.
-  FilePath path;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path));
-  path = path.AppendASCII("extensions")
-      .AppendASCII("good")
-      .AppendASCII("Extensions")
-      .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")
-      .AppendASCII("1.0.0.0");
-  ASSERT_TRUE(file_util::DirectoryExists(path));  // sanity check
-
-  // Wait for the extension to load and grab a pointer to it.
-  TestExtensionLoader loader(browser()->profile());
-  Extension* extension = loader.Load(kGoodExtension1Id, path);
-  ASSERT_TRUE(extension);
-  GURL url = Extension::GetResourceURL(extension->url(), "toolstrip1.html");
-
-  // Start the extension process and wait for it to show a javascript alert.
-  MockExtensionHost host(extension, url,
-      browser()->profile()->GetExtensionProcessManager()->
-          GetSiteInstanceForURL(url));
-  EXPECT_TRUE(host.got_message());
+void ExtensionBrowserTest::UninstallExtension(const std::string& extension_id) {
+  ExtensionsService* service = browser()->profile()->GetExtensionsService();
+  service->UninstallExtension(extension_id, false);
 }
 
-// Tests that the ExtensionShelf initializes properly, notices that
-// an extension loaded and has a view available, and then sets that up
-// properly.
-// TODO(mpcomplete): http://crbug.com/15081 Disabled because it fails.
-IN_PROC_BROWSER_TEST_F(ExtensionViewTest, DISABLED_Shelf) {
-  // When initialized, there are no extension views and the preferred height
-  // should be zero.
-  scoped_ptr<ExtensionShelf> shelf(new ExtensionShelf(browser()));
-  EXPECT_EQ(shelf->GetChildViewCount(), 0);
-  EXPECT_EQ(shelf->GetPreferredSize().height(), 0);
-
-  // Get the path to our extension.
-  FilePath path;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path));
-  path = path.AppendASCII("extensions")
-      .AppendASCII("good")
-      .AppendASCII("Extensions")
-      .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")
-      .AppendASCII("1.0.0.0");
-  ASSERT_TRUE(file_util::DirectoryExists(path));  // sanity check
-
-  // Wait for the extension to load and grab a pointer to it.
-  TestExtensionLoader loader(browser()->profile());
-  Extension* extension = loader.Load(kGoodExtension1Id, path);
-  ASSERT_TRUE(extension);
-
-  // There should now be two extension views and preferred height of the view
-  // should be non-zero.
-  EXPECT_EQ(shelf->GetChildViewCount(), 2);
-  EXPECT_NE(shelf->GetPreferredSize().height(), 0);
+bool ExtensionBrowserTest::WaitForExtensionHostsToLoad() {
+  // Wait for all the extension hosts that exist to finish loading.
+  // NOTE: This assumes that the extension host list is not changing while
+  // this method is running.
+  ExtensionProcessManager* manager =
+        browser()->profile()->GetExtensionProcessManager();
+  base::Time start_time = base::Time::Now();
+  for (ExtensionProcessManager::const_iterator iter = manager->begin();
+     iter != manager->end(); ++iter) {
+    while (!(*iter)->did_stop_loading()) {
+      if ((base::Time::Now() - start_time).InMilliseconds() > kTimeoutMs)
+        return false;
+
+      MessageLoop::current()->PostDelayedTask(FROM_HERE,
+          new MessageLoop::QuitTask, 200);
+      ui_test_utils::RunMessageLoop();
+    }
+  }
+
+  return true;
 }
 
-// Tests that installing and uninstalling extensions don't crash with an
-// incognito window open.
-// TODO(mpcomplete): http://crbug.com/14947 disabled because it crashes.
-IN_PROC_BROWSER_TEST_F(ExtensionViewTest, DISABLED_Incognito) {
-  // Open an incognito window to the extensions management page.  We just
-  // want to make sure that we don't crash while playing with extensions when
-  // this guy is around.
-  Browser::OpenURLOffTheRecord(browser()->profile(),
-                               GURL(chrome::kChromeUIExtensionsURL));
-
-  // Get the path to our extension.
-  FilePath path;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path));
-  path = path.AppendASCII("extensions").AppendASCII("good.crx");
-  ASSERT_TRUE(file_util::PathExists(path));  // sanity check
-
-  // Wait for the extension to load and grab a pointer to it.
-  TestExtensionLoader loader(browser()->profile());
-  Extension* extension = loader.Install(kGoodCrxId, path);
-  ASSERT_TRUE(extension);
-
-  // TODO(mpcomplete): wait for uninstall to complete?
-  browser()->profile()->GetExtensionsService()->
-             UninstallExtension(kGoodCrxId, false);
+void ExtensionBrowserTest::Observe(NotificationType type,
+                     const NotificationSource& source,
+                     const NotificationDetails& details) {
+  switch (type.value) {
+    case NotificationType::EXTENSION_INSTALLED:
+    case NotificationType::EXTENSIONS_LOADED:
+      MessageLoopForUI::current()->Quit();
+      break;
+    default:
+      NOTREACHED();
+      break;
+  }
 }
diff --git a/chrome/browser/extensions/extension_browsertest.h b/chrome/browser/extensions/extension_browsertest.h
new file mode 100644
index 0000000..385fd9e
--- /dev/null
+++ b/chrome/browser/extensions/extension_browsertest.h
@@ -0,0 +1,41 @@
+// Copyright (c) 2009 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.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_BROWSER_TEST_H_
+#define CHROME_BROWSER_EXTENSIONS_EXTENSION_BROWSER_TEST_H_
+
+#include <string>
+
+#include "base/command_line.h"
+#include "base/file_path.h"
+#include "chrome/common/notification_details.h"
+#include "chrome/common/notification_observer.h"
+#include "chrome/common/notification_registrar.h"
+#include "chrome/common/notification_type.h"
+#include "chrome/test/in_process_browser_test.h"
+
+// Base class for extension browser tests. Provides utilities for loading,
+// unloading, and installing extensions.
+class ExtensionBrowserTest
+    : public InProcessBrowserTest, public NotificationObserver {
+ protected:
+  virtual void SetUpCommandLine(CommandLine* command_line);
+  bool LoadExtension(const FilePath& path);
+  bool InstallExtension(const FilePath& path);
+  void UninstallExtension(const std::string& extension_id);
+
+  bool loaded_;
+  bool installed_;
+  FilePath test_data_dir_;
+
+ private:
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+  bool WaitForExtensionHostsToLoad();
+
+  NotificationRegistrar registrar_;
+};
+
+#endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_BROWSER_TEST_H_
diff --git a/chrome/browser/extensions/extension_browsertests_misc.cc b/chrome/browser/extensions/extension_browsertests_misc.cc
new file mode 100644
index 0000000..1f149d4
--- /dev/null
+++ b/chrome/browser/extensions/extension_browsertests_misc.cc
@@ -0,0 +1,106 @@
+// Copyright (c) 2009 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 "base/ref_counted.h"
+#include "chrome/browser/browser.h"
+#include "chrome/browser/browser_list.h"
+#include "chrome/browser/renderer_host/render_view_host.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/extensions/extension_host.h"
+#include "chrome/browser/extensions/extension_process_manager.h"
+#include "chrome/browser/extensions/extensions_service.h"
+#include "chrome/browser/profile.h"
+#include "chrome/browser/renderer_host/site_instance.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/browser/views/extensions/extension_shelf.h"
+#include "chrome/browser/views/frame/browser_view.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/extensions/extension_error_reporter.h"
+#include "chrome/common/url_constants.h"
+#include "chrome/test/ui_test_utils.h"
+
+// Tests that toolstrips initializes properly and can run basic extension js.
+IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, Toolstrip) {
+  ASSERT_TRUE(LoadExtension(
+      test_data_dir_.AppendASCII("good").AppendASCII("Extensions")
+                    .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")
+                    .AppendASCII("1.0.0.0")));
+
+  // At this point, there should be two ExtensionHosts loaded because this
+  // extension has two toolstrips. Find the one that is hosting toolstrip1.html.
+  ExtensionProcessManager* manager =
+      browser()->profile()->GetExtensionProcessManager();
+  ExtensionHost* host = NULL;
+  int num_hosts = 0;
+  for (ExtensionProcessManager::const_iterator iter = manager->begin();
+       iter != manager->end(); ++iter) {
+    if ((*iter)->GetURL().path() == "/toolstrip1.html") {
+      ASSERT_FALSE(host);
+      host = *iter;
+    }
+    num_hosts++;
+  }
+  EXPECT_EQ(2, num_hosts);
+
+  // Tell it to run some JavaScript that tests that basic extension code works.
+  bool result = false;
+  ui_test_utils::ExecuteJavaScriptAndExtractBool(
+      host->render_view_host(), L"", L"testTabsAPI()", &result);
+  EXPECT_TRUE(result);
+}
+
+// Tests that the ExtensionShelf initializes properly, notices that
+// an extension loaded and has a view available, and then sets that up
+// properly.
+IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, Shelf) {
+  // When initialized, there are no extension views and the preferred height
+  // should be zero.
+  BrowserView* browser_view = static_cast<BrowserView*>(browser()->window());
+  ExtensionShelf* shelf = browser_view->extension_shelf();
+  ASSERT_TRUE(shelf);
+  EXPECT_EQ(shelf->GetChildViewCount(), 0);
+  EXPECT_EQ(shelf->GetPreferredSize().height(), 0);
+
+  ASSERT_TRUE(LoadExtension(
+      test_data_dir_.AppendASCII("good").AppendASCII("Extensions")
+                    .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")
+                    .AppendASCII("1.0.0.0")));
+
+  // There should now be two extension views and preferred height of the view
+  // should be non-zero.
+  EXPECT_EQ(shelf->GetChildViewCount(), 2);
+  EXPECT_NE(shelf->GetPreferredSize().height(), 0);
+}
+
+// Tests that installing and uninstalling extensions don't crash with an
+// incognito window open.
+IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, Incognito) {
+  // Open an incognito window to the extensions management page.  We just
+  // want to make sure that we don't crash while playing with extensions when
+  // this guy is around.
+  Browser::OpenURLOffTheRecord(browser()->profile(),
+                               GURL(chrome::kChromeUIExtensionsURL));
+
+  ASSERT_TRUE(InstallExtension(test_data_dir_.AppendASCII("good.crx")));
+  UninstallExtension("ldnnhddmnhbkjipkidpdiheffobcpfmf");
+}
+
+// Tests that we can load extension pages into the tab area and they can call
+// extension APIs.
+IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, TabContents) {
+  ASSERT_TRUE(LoadExtension(
+      test_data_dir_.AppendASCII("good").AppendASCII("Extensions")
+                    .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")
+                    .AppendASCII("1.0.0.0")));
+
+  ui_test_utils::NavigateToURL(
+      browser(),
+      GURL("chrome-extension://behllobkkfkfnphdnhnkndlbkcpglgmj/page.html"));
+
+  bool result = false;
+  ui_test_utils::ExecuteJavaScriptAndExtractBool(
+      browser()->GetSelectedTabContents()->render_view_host(), L"",
+      L"testTabsAPI()", &result);
+  EXPECT_TRUE(result);
+}
diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc
index 48853d8..15319b3 100644
--- a/chrome/browser/extensions/extension_host.cc
+++ b/chrome/browser/extensions/extension_host.cc
@@ -77,6 +77,10 @@ class CrashedExtensionInfobarDelegate : public ConfirmInfoBarDelegate {
 
 }  // namespace
 
+
+// static
+bool ExtensionHost::enable_dom_automation_ = false;
+
 ExtensionHost::ExtensionHost(Extension* extension, SiteInstance* site_instance,
                              const GURL& url)
     : extension_(extension),
@@ -86,6 +90,8 @@ ExtensionHost::ExtensionHost(Extension* extension, SiteInstance* site_instance,
   render_view_host_ = new RenderViewHost(
       site_instance, this, MSG_ROUTING_NONE, NULL);
   render_view_host_->AllowBindings(BindingsPolicy::EXTENSION);
+  if (enable_dom_automation_)
+    render_view_host_->AllowBindings(BindingsPolicy::DOM_AUTOMATION);
 }
 
 ExtensionHost::~ExtensionHost() {
diff --git a/chrome/browser/extensions/extension_host.h b/chrome/browser/extensions/extension_host.h
index e54fc39..a3b04f2 100644
--- a/chrome/browser/extensions/extension_host.h
+++ b/chrome/browser/extensions/extension_host.h
@@ -32,6 +32,9 @@ class ExtensionHost : public RenderViewHostDelegate,
                       public RenderViewHostDelegate::View,
                       public ExtensionFunctionDispatcher::Delegate {
  public:
+  // Enable DOM automation in created render view hosts.
+  static void EnableDOMAutomation() { enable_dom_automation_ = true; }
+
   ExtensionHost(Extension* extension, SiteInstance* site_instance,
                 const GURL& url);
   ~ExtensionHost();
@@ -104,6 +107,10 @@ class ExtensionHost : public RenderViewHostDelegate,
   virtual void UpdatePreferredWidth(int pref_width);
 
  private:
+  // Whether to allow DOM automation for created RenderViewHosts. This is used
+  // for testing.
+  static bool enable_dom_automation_;
+
   // ExtensionFunctionDispatcher::Delegate
   // If this ExtensionHost has a view, this returns the Browser that view is a
   // part of.  If this is a global background page, we use the active Browser
diff --git a/chrome/browser/extensions/extension_shelf_model_unittest.cc b/chrome/browser/extensions/extension_shelf_model_unittest.cc
index b41754c..d1e2217 100644
--- a/chrome/browser/extensions/extension_shelf_model_unittest.cc
+++ b/chrome/browser/extensions/extension_shelf_model_unittest.cc
@@ -3,11 +3,13 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/browser.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/extensions/extension_shelf_model.h"
 #include "chrome/browser/extensions/extensions_service.h"
-#include "chrome/browser/extensions/test_extension_loader.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/views/extensions/extension_shelf.h"
+#include "chrome/browser/views/frame/browser_view.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_error_reporter.h"
@@ -26,31 +28,28 @@ const char* kExtensionId = "behllobkkfkfnphdnhnkndlbkcpglgmj";
 // It would be nice to refactor things so that ExtensionShelfModel,
 // ExtensionHost and ExtensionsService could run without so much of the browser
 // in place.
-class ExtensionShelfModelTest : public InProcessBrowserTest,
+class ExtensionShelfModelTest : public ExtensionBrowserTest,
                                 public ExtensionShelfModelObserver {
  public:
   virtual void SetUp() {
-    // Initialize the error reporter here, or BrowserMain will create it with
-    // the wrong MessageLoop.
-    ExtensionErrorReporter::Init(false);
     inserted_count_ = 0;
     removed_count_ = 0;
     moved_count_ = 0;
-
     InProcessBrowserTest::SetUp();
   }
 
-  virtual void SetUpCommandLine(CommandLine* command_line) {
-    command_line->AppendSwitch(switches::kEnableExtensions);
-  }
-
   virtual Browser* CreateBrowser(Profile* profile) {
     Browser* b = InProcessBrowserTest::CreateBrowser(profile);
-    model_ = new ExtensionShelfModel(b);
+    BrowserView* browser_view = static_cast<BrowserView*>(b->window());
+    model_ = browser_view->extension_shelf()->model();
     model_->AddObserver(this);
     return b;
   }
 
+  virtual void CleanUpOnMainThread() {
+    model_->RemoveObserver(this);
+  }
+
   virtual void ToolstripInsertedAt(ExtensionHost* toolstrip, int index) {
     inserted_count_++;
   }
@@ -73,21 +72,11 @@ class ExtensionShelfModelTest : public InProcessBrowserTest,
   int moved_count_;
 };
 
-// TODO(erikkay): http://crbug.com/15291 disabled because fails on build-bot.
-IN_PROC_BROWSER_TEST_F(ExtensionShelfModelTest, DISABLED_Basic) {
-  // Get the path to our extension.
-  FilePath path;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path));
-  path = path.AppendASCII("extensions")
-      .AppendASCII("good")
-      .AppendASCII("Extensions")
-      .AppendASCII(kExtensionId).AppendASCII("1.0.0.0");
-  ASSERT_TRUE(file_util::DirectoryExists(path));  // sanity check
-
-  // Wait for the extension to load and grab a pointer to it.
-  TestExtensionLoader loader(browser()->profile());
-  Extension* extension = loader.Load(kExtensionId, path);
-  ASSERT_TRUE(extension);
+IN_PROC_BROWSER_TEST_F(ExtensionShelfModelTest, Basic) {
+  ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("good")
+                                          .AppendASCII("Extensions")
+                                          .AppendASCII(kExtensionId)
+                                          .AppendASCII("1.0.0.0")));
 
   // extension1 has two toolstrips
   EXPECT_EQ(inserted_count_, 2);
@@ -105,12 +94,4 @@ IN_PROC_BROWSER_TEST_F(ExtensionShelfModelTest, DISABLED_Basic) {
   EXPECT_EQ(one, model_->ToolstripAt(0));
   EXPECT_EQ(1, model_->count());
   EXPECT_EQ(removed_count_, 1);
-
-  // Tear down |model_| manually here rather than in the destructor or with
-  // a scoped_ptr.  InProcessBrowserTest doesn't give us a chance to clean
-  // up before the browser and all of its services have been shut down,
-  // and |model_| depends on these existing.
-  model_->RemoveObserver(this);
-  delete model_;
-  model_ = NULL;
 }
diff --git a/chrome/browser/extensions/extension_startup_unittest.cc b/chrome/browser/extensions/extension_startup_unittest.cc
index a9c494a..540671e 100644
--- a/chrome/browser/extensions/extension_startup_unittest.cc
+++ b/chrome/browser/extensions/extension_startup_unittest.cc
@@ -7,6 +7,7 @@
 #include "chrome/browser/browser.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/notification_details.h"
@@ -18,22 +19,22 @@
 #include "chrome/test/ui_test_utils.h"
 #include "net/base/net_util.h"
 
-// This file contains high-level regression tests for the extensions system. The
-// goal here is not to test everything in depth, but to run the system as close
-// as possible end-to-end to find any gaps in test coverage in the lower-level
-// unit tests.
+// This file contains high-level startup tests for the extensions system. We've
+// had many silly bugs where command line flags did not get propagated correctly
+// into the services, so we didn't start correctly.
 
 class ExtensionStartupTestBase
   : public InProcessBrowserTest, public NotificationObserver {
  public:
   ExtensionStartupTestBase()
       : enable_extensions_(false), enable_user_scripts_(false) {
-    EnableDOMAutomation();
   }
 
  protected:
   // InProcessBrowserTest
   virtual void SetUpCommandLine(CommandLine* command_line) {
+    EnableDOMAutomation();
+
     FilePath profile_dir;
     PathService::Get(chrome::DIR_USER_DATA, &profile_dir);
     profile_dir = profile_dir.AppendASCII("Default");
@@ -145,7 +146,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionsStartupTest, Test) {
   // Test that the content script ran.
   bool result = false;
   ui_test_utils::ExecuteJavaScriptAndExtractBool(
-      browser()->GetSelectedTabContents(), L"",
+      browser()->GetSelectedTabContents()->render_view_host(), L"",
       L"window.domAutomationController.send("
       L"document.defaultView.getComputedStyle(document.body, null)."
       L"getPropertyValue('background-color') == 'rgb(245, 245, 220)')",
@@ -154,21 +155,10 @@ IN_PROC_BROWSER_TEST_F(ExtensionsStartupTest, Test) {
 
   result = false;
   ui_test_utils::ExecuteJavaScriptAndExtractBool(
-      browser()->GetSelectedTabContents(), L"",
+      browser()->GetSelectedTabContents()->render_view_host(), L"",
       L"window.domAutomationController.send(document.title == 'Modified')",
       &result);
   EXPECT_TRUE(result);
-
-  // Load an extension page into the tab area to make sure it works.
-  result = false;
-  ui_test_utils::NavigateToURL(
-      browser(),
-      GURL("chrome-extension://behllobkkfkfnphdnhnkndlbkcpglgmj/page.html"));
-  ui_test_utils::ExecuteJavaScriptAndExtractBool(
-      browser()->GetSelectedTabContents(), L"", L"testTabsAPI()", &result);
-  EXPECT_TRUE(result);
-
-  // TODO(aa): Move the stuff in ExtensionBrowserTest here?
 }
 
 
@@ -207,7 +197,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionsStartupUserScriptTest, Test) {
   // Test that the user script ran.
   bool result = false;
   ui_test_utils::ExecuteJavaScriptAndExtractBool(
-      browser()->GetSelectedTabContents(), L"",
+      browser()->GetSelectedTabContents()->render_view_host(), L"",
       L"window.domAutomationController.send(document.title == 'Modified')",
       &result);
   EXPECT_TRUE(result);
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index 4b28169..5afa547 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -129,7 +129,11 @@ class ExtensionsService
                                ExtensionInstallCallback* callback);
 
   // Uninstalls the specified extension. Callers should only call this method
-  // with extensions that exist.
+  // with extensions that exist. |external_uninstall| is a magical parameter
+  // that is only used to send information to ExtensionPrefs, which external
+  // callers should never set to true.
+  // TODO(aa): Remove |external_uninstall| -- this information should be passed
+  // to ExtensionPrefs some other way.
   void UninstallExtension(const std::string& extension_id,
                           bool external_uninstall);
 
diff --git a/chrome/browser/extensions/test_extension_loader.cc b/chrome/browser/extensions/test_extension_loader.cc
deleted file mode 100644
index d17f557..0000000
--- a/chrome/browser/extensions/test_extension_loader.cc
+++ /dev/null
@@ -1,82 +0,0 @@
-// Copyright (c) 2009 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 "chrome/browser/extensions/test_extension_loader.h"
-
-#include "base/file_path.h"
-#include "base/message_loop.h"
-#include "chrome/browser/profile.h"
-#include "chrome/browser/extensions/extensions_service.h"
-#include "chrome/common/notification_service.h"
-#include "chrome/test/ui_test_utils.h"
-
-namespace {
-
-// How long to wait for the extension to load before giving up.
-const int kLoadTimeoutMs = 5000;
-const int kInstallTimeoutMs = 10000;
-
-}  // namespace
-
-TestExtensionLoader::TestExtensionLoader(Profile* profile)
-    : profile_(profile),
-      extension_(NULL) {
-  registrar_.Add(this, NotificationType::EXTENSIONS_LOADED,
-      NotificationService::AllSources());
-
-  profile_->GetExtensionsService()->Init();
-  profile_->GetExtensionsService()->set_show_extensions_prompts(false);
-  DCHECK(profile_->GetExtensionsService()->extensions()->empty());
-}
-
-Extension* TestExtensionLoader::Load(const char* extension_id,
-                                     const FilePath& path) {
-  loading_extension_id_ = extension_id;
-
-  // Load the extension.
-  profile_->GetExtensionsService()->LoadExtension(path);
-
-  // Wait for the load to complete.  Stick a QuitTask into the message loop
-  // with the timeout so it will exit if the extension never loads.
-  extension_ = NULL;
-  MessageLoop::current()->PostDelayedTask(FROM_HERE,
-      new MessageLoop::QuitTask, kLoadTimeoutMs);
-  ui_test_utils::RunMessageLoop();
-
-  return extension_;
-}
-
-Extension* TestExtensionLoader::Install(const char* extension_id,
-                                        const FilePath& path) {
-  loading_extension_id_ = extension_id;
-
-  // Install the extension.  When installed, the extension will automatically
-  // be loaded.
-  profile_->GetExtensionsService()->InstallExtension(path);
-
-  // Wait for the load to complete.
-  extension_ = NULL;
-  MessageLoop::current()->PostDelayedTask(FROM_HERE,
-      new MessageLoop::QuitTask, kInstallTimeoutMs);
-  ui_test_utils::RunMessageLoop();
-
-  return extension_;
-}
-
-void TestExtensionLoader::Observe(NotificationType type,
-                                  const NotificationSource& source,
-                                  const NotificationDetails& details) {
-  if (type == NotificationType::EXTENSIONS_LOADED) {
-    ExtensionList* extensions = Details<ExtensionList>(details).ptr();
-    for (size_t i = 0; i < (*extensions).size(); ++i) {
-      if ((*extensions)[i]->id() == loading_extension_id_) {
-        extension_ = (*extensions)[i];
-        MessageLoopForUI::current()->Quit();
-        break;
-      }
-    }
-  } else {
-    NOTREACHED();
-  }
-}
diff --git a/chrome/browser/extensions/test_extension_loader.h b/chrome/browser/extensions/test_extension_loader.h
deleted file mode 100644
index ddf5585..0000000
--- a/chrome/browser/extensions/test_extension_loader.h
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright (c) 2009 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.
-
-#ifndef CHROME_BROWSER_EXTENSIONS_TEST_EXTENSION_LOADER_H_
-#define CHROME_BROWSER_EXTENSIONS_TEST_EXTENSION_LOADER_H_
-
-#include "chrome/common/notification_observer.h"
-#include "chrome/common/notification_registrar.h"
-
-class Extension;
-class FilePath;
-class Profile;
-
-class TestExtensionLoader : public NotificationObserver {
- public:
-  explicit TestExtensionLoader(Profile* profile);
-
-  // Tells the extension service to load the extension at the given path.  It
-  // waits for the extension with the expected ID to load, then returns a
-  // handle to it.
-  Extension* Load(const char* extension_id, const FilePath& path);
-
-  // Same as above, but installs from a CRX first.
-  Extension* Install(const char* extension_id, const FilePath& path);
-
-  virtual void Observe(NotificationType type,
-                       const NotificationSource& source,
-                       const NotificationDetails& details);
-
- private:
-  Profile* profile_;
-  Extension* extension_;
-  NotificationRegistrar registrar_;
-  std::string loading_extension_id_;
-
-  DISALLOW_COPY_AND_ASSIGN(TestExtensionLoader);
-};
-
-#endif  // CHROME_BROWSER_EXTENSIONS_TEST_EXTENSION_LOADER_H_