Add TestExtensionsBrowserClient, move ProcessManagerTest to //extensions

Rewrite ProcessManagerTest to eliminate the TestingProfile dependency and switch to TestBrowserContext.

This requires an ExtensionsBrowserClient that knows how to associate an incognito context with a non-incognito context. I added this in hopes it would be useful for converting other tests away from TestingProfile.

BUG=315855
TEST=unit_tests ProcessManager*

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

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

chrome/chrome_tests_unit.gypi

@@ -557,6 +557,7 @@
         '../extensions/browser/info_map_unittest.cc',
         '../extensions/browser/lazy_background_task_queue_unittest.cc',
         '../extensions/browser/management_policy_unittest.cc',
+        '../extensions/browser/process_manager_unittest.cc',
         '../extensions/browser/process_map_unittest.cc',
         '../extensions/browser/quota_service_unittest.cc',
         '../extensions/browser/runtime_data_unittest.cc',
@@ -960,7 +961,6 @@
         'browser/extensions/page_action_controller_unittest.cc',
         'browser/extensions/permissions_updater_unittest.cc',
         'browser/extensions/policy_handlers_unittest.cc',
-        'browser/extensions/process_manager_unittest.cc',
         'browser/extensions/sandboxed_unpacker_unittest.cc',
         'browser/extensions/standard_management_policy_provider_unittest.cc',
         'browser/extensions/tab_helper_unittest.cc',

extensions/browser/process_manager.cc

@@ -95,7 +95,8 @@ void OnRenderViewHostUnregistered(BrowserContext* context,
 class IncognitoProcessManager : public ProcessManager {
  public:
   IncognitoProcessManager(BrowserContext* incognito_context,
-                          BrowserContext* original_context);
+                          BrowserContext* original_context,
+                          ProcessManager* original_manager);
   virtual ~IncognitoProcessManager() {}
   virtual bool CreateBackgroundHost(const Extension* extension,
                                     const GURL& url) OVERRIDE;
@@ -189,12 +190,26 @@ ProcessManager* ProcessManager::Create(BrowserContext* context) {
 
   if (context->IsOffTheRecord()) {
     BrowserContext* original_context = client->GetOriginalContext(context);
-    return new IncognitoProcessManager(context, original_context);
+    ProcessManager* original_manager =
+        ExtensionSystem::Get(original_context)->process_manager();
+    return new IncognitoProcessManager(
+        context, original_context, original_manager);
   }
 
   return new ProcessManager(context, context);
 }
 
+// static
+ProcessManager* ProcessManager::CreateIncognitoForTesting(
+    BrowserContext* incognito_context,
+    BrowserContext* original_context,
+    ProcessManager* original_manager) {
+  DCHECK(incognito_context->IsOffTheRecord());
+  DCHECK(!original_context->IsOffTheRecord());
+  return new IncognitoProcessManager(
+      incognito_context, original_context, original_manager);
+}
+
 ProcessManager::ProcessManager(BrowserContext* context,
                                BrowserContext* original_context)
   : site_instance_(SiteInstance::Create(context)),
@@ -861,10 +876,10 @@ bool ProcessManager::DeferLoadingBackgroundHosts() const {
 
 IncognitoProcessManager::IncognitoProcessManager(
     BrowserContext* incognito_context,
-    BrowserContext* original_context)
+    BrowserContext* original_context,
+    ProcessManager* original_manager)
     : ProcessManager(incognito_context, original_context),
-      original_manager_(
-          ExtensionSystem::Get(original_context)->process_manager()) {
+      original_manager_(original_manager) {
   DCHECK(incognito_context->IsOffTheRecord());
 
   // The original profile will have its own ProcessManager to

extensions/browser/process_manager.h

@@ -131,6 +131,13 @@ class ProcessManager : public content::NotificationObserver {
   void SetKeepaliveImpulseDecrementCallbackForTesting(
       const ImpulseCallbackForTesting& callback);
 
+  // Creates an incognito-context instance for tests. Tests for non-incognito
+  // contexts can just use Create() above.
+  static ProcessManager* CreateIncognitoForTesting(
+      content::BrowserContext* incognito_context,
+      content::BrowserContext* original_context,
+      ProcessManager* original_manager);
+
  protected:
   // If |context| is incognito pass the master context as |original_context|.
   // Otherwise pass the same context for both.

chrome/browser/extensions/process_manager_unittest.cc → extensions/browser/process_manager_unittest.cc

-// Copyright 2013 The Chromium Authors. All rights reserved.
+// Copyright 2014 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 "extensions/browser/process_manager.h"
 
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/extensions/extension_error_reporter.h"
-#include "chrome/test/base/testing_profile.h"
+#include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/notification_service.h"
-#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/site_instance.h"
+#include "content/public/test/test_browser_context.h"
+#include "extensions/browser/test_extensions_browser_client.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "testing/platform_test.h"
 
+using content::BrowserContext;
 using content::SiteInstance;
+using content::TestBrowserContext;
 
 namespace extensions {
 
-// TODO(jamescook): Convert this from TestingProfile to TestBrowserContext and
-// move to extensions/browser. This is dependent on ExtensionPrefs being
-// converted and ExtensionSystem being converted or eliminated.
-// http://crbug.com/315855
+namespace {
+
+// An incognito version of a TestBrowserContext.
+class TestBrowserContextIncognito : public TestBrowserContext {
+ public:
+  TestBrowserContextIncognito() {}
+  virtual ~TestBrowserContextIncognito() {}
+
+  // TestBrowserContext implementation.
+  virtual bool IsOffTheRecord() const OVERRIDE { return true; }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestBrowserContextIncognito);
+};
+
+}  // namespace
 
-// make the test a PlatformTest to setup autorelease pools properly on mac
 class ProcessManagerTest : public testing::Test {
  public:
-  static void SetUpTestCase() {
-    ExtensionErrorReporter::Init(false);  // no noisy errors
+  ProcessManagerTest() : extensions_browser_client_(&original_context_) {
+    extensions_browser_client_.SetIncognitoContext(&incognito_context_);
+    ExtensionsBrowserClient::Set(&extensions_browser_client_);
   }
 
-  virtual void SetUp() {
-    ExtensionErrorReporter::GetInstance()->ClearErrors();
+  virtual ~ProcessManagerTest() {
+    ExtensionsBrowserClient::Set(NULL);
   }
 
+  BrowserContext* original_context() { return &original_context_; }
+  BrowserContext* incognito_context() { return &incognito_context_; }
+
   // Returns true if the notification |type| is registered for |manager| with
-  // source |profile|. Pass NULL for |profile| for all sources.
+  // source |context|. Pass NULL for |context| for all sources.
   static bool IsRegistered(ProcessManager* manager,
                            int type,
-                           TestingProfile* profile) {
+                           BrowserContext* context) {
     return manager->registrar_.IsRegistered(
-        manager, type, content::Source<Profile>(profile));
+        manager, type, content::Source<BrowserContext>(context));
   }
+
+ private:
+  TestBrowserContext original_context_;
+  TestBrowserContextIncognito incognito_context_;
+  TestExtensionsBrowserClient extensions_browser_client_;
+
+  DISALLOW_COPY_AND_ASSIGN(ProcessManagerTest);
 };
 
 // Test that notification registration works properly.
 TEST_F(ProcessManagerTest, ExtensionNotificationRegistration) {
-  // Test for a normal profile.
-  scoped_ptr<TestingProfile> original_profile(new TestingProfile);
+  // Test for a normal context ProcessManager.
   scoped_ptr<ProcessManager> manager1(
-      ProcessManager::Create(original_profile.get()));
+      ProcessManager::Create(original_context()));
 
-  EXPECT_EQ(original_profile.get(), manager1->GetBrowserContext());
+  EXPECT_EQ(original_context(), manager1->GetBrowserContext());
   EXPECT_EQ(0u, manager1->background_hosts().size());
 
-  // It observes other notifications from this profile.
+  // It observes other notifications from this context.
   EXPECT_TRUE(IsRegistered(manager1.get(),
                            chrome::NOTIFICATION_EXTENSIONS_READY,
-                           original_profile.get()));
+                           original_context()));
   EXPECT_TRUE(IsRegistered(manager1.get(),
                            chrome::NOTIFICATION_EXTENSION_LOADED,
-                           original_profile.get()));
+                           original_context()));
   EXPECT_TRUE(IsRegistered(manager1.get(),
                            chrome::NOTIFICATION_EXTENSION_UNLOADED,
-                           original_profile.get()));
+                           original_context()));
   EXPECT_TRUE(IsRegistered(manager1.get(),
                            chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED,
-                           original_profile.get()));
+                           original_context()));
 
-  // Now add an incognito profile associated with the master above.
-  TestingProfile::Builder builder;
-  builder.SetIncognito();
-  scoped_ptr<TestingProfile> incognito_profile = builder.Build();
-  incognito_profile->SetOriginalProfile(original_profile.get());
-  scoped_ptr<ProcessManager> manager2(
-      ProcessManager::Create(incognito_profile.get()));
+  // Test for an incognito context ProcessManager.
+  scoped_ptr<ProcessManager> manager2(ProcessManager::CreateIncognitoForTesting(
+      incognito_context(), original_context(), manager1.get()));
 
-  EXPECT_EQ(incognito_profile.get(), manager2->GetBrowserContext());
+  EXPECT_EQ(incognito_context(), manager2->GetBrowserContext());
   EXPECT_EQ(0u, manager2->background_hosts().size());
 
-  // Some notifications are observed for the original profile.
+  // Some notifications are observed for the original context.
   EXPECT_TRUE(IsRegistered(manager2.get(),
                            chrome::NOTIFICATION_EXTENSION_LOADED,
-                           original_profile.get()));
+                           original_context()));
 
-  // Some notifications are observed for the incognito profile.
+  // Some notifications are observed for the incognito context.
   EXPECT_TRUE(IsRegistered(manager2.get(),
                            chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED,
-                           incognito_profile.get()));
+                           incognito_context()));
 
   // Some notifications are observed for both incognito and original.
   EXPECT_TRUE(IsRegistered(manager2.get(),
                            chrome::NOTIFICATION_PROFILE_DESTROYED,
-                           original_profile.get()));
+                           original_context()));
   EXPECT_TRUE(IsRegistered(manager2.get(),
                            chrome::NOTIFICATION_PROFILE_DESTROYED,
-                           incognito_profile.get()));
+                           incognito_context()));
 
   // Some are not observed at all.
   EXPECT_FALSE(IsRegistered(manager2.get(),
                             chrome::NOTIFICATION_EXTENSIONS_READY,
-                            original_profile.get()));
+                            original_context()));
 
-  // This notification is observed for incognito profiles only.
+  // This notification is observed for incognito contexts only.
   EXPECT_TRUE(IsRegistered(manager2.get(),
                            chrome::NOTIFICATION_PROFILE_DESTROYED,
-                           incognito_profile.get()));
+                           incognito_context()));
 }
 
 // Test that extensions get grouped in the right SiteInstance (and therefore
 // process) based on their URLs.
 TEST_F(ProcessManagerTest, ProcessGrouping) {
-  // Extensions in different profiles should always be different SiteInstances.
-  // Note: we don't initialize these, since we're not testing that
-  // functionality.  This means we can get away with a NULL UserScriptMaster.
-  TestingProfile profile1;
-  scoped_ptr<ProcessManager> manager1(ProcessManager::Create(&profile1));
-
-  TestingProfile profile2;
-  scoped_ptr<ProcessManager> manager2(ProcessManager::Create(&profile2));
+  content::ContentBrowserClient content_browser_client;
+  content::SetBrowserClientForTesting(&content_browser_client);
+
+  // Extensions in different browser contexts should always be different
+  // SiteInstances.
+  scoped_ptr<ProcessManager> manager1(
+      ProcessManager::Create(original_context()));
+  // NOTE: This context is not associated with the TestExtensionsBrowserClient.
+  // That's OK because we're not testing regular vs. incognito behavior.
+  TestBrowserContext another_context;
+  scoped_ptr<ProcessManager> manager2(ProcessManager::Create(&another_context));
 
   // Extensions with common origins ("scheme://id/") should be grouped in the
   // same SiteInstance.

extensions/browser/test_extensions_browser_client.cc

+// Copyright 2014 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 "extensions/browser/test_extensions_browser_client.h"
+
+#include "content/public/browser/browser_context.h"
+#include "extensions/browser/app_sorting.h"
+
+using content::BrowserContext;
+
+namespace extensions {
+
+TestExtensionsBrowserClient::TestExtensionsBrowserClient(
+    BrowserContext* main_context)
+    : main_context_(main_context), incognito_context_(NULL) {
+  DCHECK(main_context_);
+  DCHECK(!main_context_->IsOffTheRecord());
+}
+
+TestExtensionsBrowserClient::~TestExtensionsBrowserClient() {}
+
+void TestExtensionsBrowserClient::SetIncognitoContext(BrowserContext* context) {
+  // If a context is provided it must be off-the-record.
+  DCHECK(!context || context->IsOffTheRecord());
+  incognito_context_ = context;
+}
+
+bool TestExtensionsBrowserClient::IsShuttingDown() { return false; }
+
+bool TestExtensionsBrowserClient::AreExtensionsDisabled(
+    const CommandLine& command_line,
+    BrowserContext* context) {
+  return false;
+}
+
+bool TestExtensionsBrowserClient::IsValidContext(BrowserContext* context) {
+  return context == main_context_ ||
+         (incognito_context_ && context == incognito_context_);
+}
+
+bool TestExtensionsBrowserClient::IsSameContext(BrowserContext* first,
+                                                BrowserContext* second) {
+  DCHECK(first);
+  DCHECK(second);
+  return first == second ||
+         (first == main_context_ && second == incognito_context_) ||
+         (first == incognito_context_ && second == main_context_);
+}
+
+bool TestExtensionsBrowserClient::HasOffTheRecordContext(
+    BrowserContext* context) {
+  return context == main_context_ && incognito_context_ != NULL;
+}
+
+BrowserContext* TestExtensionsBrowserClient::GetOffTheRecordContext(
+    BrowserContext* context) {
+  if (context == main_context_)
+    return incognito_context_;
+  return NULL;
+}
+
+BrowserContext* TestExtensionsBrowserClient::GetOriginalContext(
+    BrowserContext* context) {
+  return main_context_;
+}
+
+bool TestExtensionsBrowserClient::IsGuestSession(
+    BrowserContext* context) const {
+  return false;
+}
+
+bool TestExtensionsBrowserClient::IsExtensionIncognitoEnabled(
+    const std::string& extension_id,
+    content::BrowserContext* context) const {
+  return false;
+}
+
+bool TestExtensionsBrowserClient::CanExtensionCrossIncognito(
+    const extensions::Extension* extension,
+    content::BrowserContext* context) const {
+  return false;
+}
+
+PrefService* TestExtensionsBrowserClient::GetPrefServiceForContext(
+    BrowserContext* context) {
+  return NULL;
+}
+
+bool TestExtensionsBrowserClient::DeferLoadingBackgroundHosts(
+    BrowserContext* context) const {
+  return false;
+}
+
+bool TestExtensionsBrowserClient::IsBackgroundPageAllowed(
+    BrowserContext* context) const {
+  return true;
+}
+
+void TestExtensionsBrowserClient::OnExtensionHostCreated(
+    content::WebContents* web_contents) {}
+
+void TestExtensionsBrowserClient::OnRenderViewCreatedForBackgroundPage(
+    ExtensionHost* host) {}
+
+bool TestExtensionsBrowserClient::DidVersionUpdate(BrowserContext* context) {
+  return false;
+}
+
+scoped_ptr<AppSorting> TestExtensionsBrowserClient::CreateAppSorting() {
+  return scoped_ptr<AppSorting>();
+}
+
+bool TestExtensionsBrowserClient::IsRunningInForcedAppMode() { return false; }
+
+content::JavaScriptDialogManager*
+TestExtensionsBrowserClient::GetJavaScriptDialogManager() {
+  return NULL;
+}
+
+ApiActivityMonitor* TestExtensionsBrowserClient::GetApiActivityMonitor(
+    BrowserContext* context) {
+  return NULL;
+}
+
+ExtensionSystemProvider*
+TestExtensionsBrowserClient::GetExtensionSystemFactory() {
+  // Tests requiring an extension system should override this function.
+  NOTREACHED();
+  return NULL;
+}
+
+void TestExtensionsBrowserClient::RegisterExtensionFunctions(
+    ExtensionFunctionRegistry* registry) const {}
+
+}  // namespace extensions

extensions/browser/test_extensions_browser_client.h

+// Copyright 2014 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 EXTENSIONS_BROWSER_TEST_EXTENSIONS_BROWSER_CLIENT_H_
+#define EXTENSIONS_BROWSER_TEST_EXTENSIONS_BROWSER_CLIENT_H_
+
+#include "base/compiler_specific.h"
+#include "extensions/browser/extensions_browser_client.h"
+
+namespace extensions {
+
+// A simplified ExtensionsBrowserClient for a single normal browser context and
+// an optional incognito browser context associated with it. A test that uses
+// this class should call ExtensionsBrowserClient::Set() with its instance.
+class TestExtensionsBrowserClient : public ExtensionsBrowserClient {
+ public:
+  // |context| is required and must not be an incognito context.
+  TestExtensionsBrowserClient(content::BrowserContext* main_context);
+  virtual ~TestExtensionsBrowserClient();
+
+  // Associates an incognito context with |main_context_|.
+  void SetIncognitoContext(content::BrowserContext* incognito_context);
+
+  // ExtensionsBrowserClient overrides:
+  virtual bool IsShuttingDown() OVERRIDE;
+  virtual bool AreExtensionsDisabled(const CommandLine& command_line,
+                                     content::BrowserContext* context) OVERRIDE;
+  virtual bool IsValidContext(content::BrowserContext* context) OVERRIDE;
+  virtual bool IsSameContext(content::BrowserContext* first,
+                             content::BrowserContext* second) OVERRIDE;
+  virtual bool HasOffTheRecordContext(content::BrowserContext* context)
+      OVERRIDE;
+  virtual content::BrowserContext* GetOffTheRecordContext(
+      content::BrowserContext* context) OVERRIDE;
+  virtual content::BrowserContext* GetOriginalContext(
+      content::BrowserContext* context) OVERRIDE;
+  virtual bool IsGuestSession(content::BrowserContext* context) const OVERRIDE;
+  virtual bool IsExtensionIncognitoEnabled(
+      const std::string& extension_id,
+      content::BrowserContext* context) const OVERRIDE;
+  virtual bool CanExtensionCrossIncognito(
+      const extensions::Extension* extension,
+      content::BrowserContext* context) const OVERRIDE;
+  virtual PrefService* GetPrefServiceForContext(
+      content::BrowserContext* context) OVERRIDE;
+  virtual bool DeferLoadingBackgroundHosts(
+      content::BrowserContext* context) const OVERRIDE;
+  virtual bool IsBackgroundPageAllowed(content::BrowserContext* context) const
+      OVERRIDE;
+  virtual void OnExtensionHostCreated(content::WebContents* web_contents)
+      OVERRIDE;
+  virtual void OnRenderViewCreatedForBackgroundPage(ExtensionHost* host)
+      OVERRIDE;
+  virtual bool DidVersionUpdate(content::BrowserContext* context) OVERRIDE;
+  virtual scoped_ptr<AppSorting> CreateAppSorting() OVERRIDE;
+  virtual bool IsRunningInForcedAppMode() OVERRIDE;
+  virtual content::JavaScriptDialogManager* GetJavaScriptDialogManager()
+      OVERRIDE;
+  virtual ApiActivityMonitor* GetApiActivityMonitor(
+      content::BrowserContext* context) OVERRIDE;
+  virtual ExtensionSystemProvider* GetExtensionSystemFactory() OVERRIDE;
+  virtual void RegisterExtensionFunctions(
+      ExtensionFunctionRegistry* registry) const OVERRIDE;
+
+ private:
+  content::BrowserContext* main_context_;       // Not owned.
+  content::BrowserContext* incognito_context_;  // Not owned, defaults to NULL.
+
+  DISALLOW_COPY_AND_ASSIGN(TestExtensionsBrowserClient);
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_BROWSER_TEST_EXTENSIONS_BROWSER_CLIENT_H_

extensions/extensions.gyp

@@ -297,6 +297,8 @@
         '..',
       ],
       'sources': [
+        'browser/test_extensions_browser_client.cc',
+        'browser/test_extensions_browser_client.h',
         'browser/test_management_policy.cc',
         'browser/test_management_policy.h',
         'common/extension_builder.cc',