[Extensions] Expand testing overrides for extension warning bubbles

Expand the testing override for the extension warning bubbles from just forcing
enabled to also forcing disabled. As we roll this out on finch, it will begin
becoming the default on trybots, but we don't want to have these bubbles
interfering with our tests when we load unpacked extensions (which is in almost
all extension browser tests).

TBR=avi@chromium.org (micro changes to permission bubble test)
TBR=mmenke@chromium.org (micro changes to prerender test)

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

Cr-Commit-Position: refs/heads/master@{#342402}

chrome/browser/extensions/extension_browsertest.cc

@@ -31,6 +31,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
+#include "chrome/browser/ui/extensions/extension_message_bubble_factory.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
@@ -122,6 +123,10 @@ void ExtensionBrowserTest::SetUpCommandLine(base::CommandLine* command_line) {
   test_data_dir_ = test_data_dir_.AppendASCII("extensions");
   observer_.reset(new ExtensionTestNotificationObserver(browser()));
 
+  // We don't want any warning bubbles for, e.g., unpacked extensions.
+  ExtensionMessageBubbleFactory::set_override_for_tests(
+      ExtensionMessageBubbleFactory::OVERRIDE_DISABLED);
+
 #if defined(OS_CHROMEOS)
   if (set_chromeos_user_) {
     // This makes sure that we create the Default profile first, with no
@@ -143,6 +148,12 @@ void ExtensionBrowserTest::SetUpOnMainThread() {
   }
 }
 
+void ExtensionBrowserTest::TearDownOnMainThread() {
+  ExtensionMessageBubbleFactory::set_override_for_tests(
+      ExtensionMessageBubbleFactory::NO_OVERRIDE);
+  InProcessBrowserTest::TearDownOnMainThread();
+}
+
 const Extension* ExtensionBrowserTest::LoadExtension(
     const base::FilePath& path) {
   return LoadExtensionWithFlags(path, kFlagEnableFileAccess);

chrome/browser/extensions/extension_browsertest.h

@@ -79,6 +79,7 @@ class ExtensionBrowserTest : virtual public InProcessBrowserTest {
   void SetUp() override;
   void SetUpCommandLine(base::CommandLine* command_line) override;
   void SetUpOnMainThread() override;
+  void TearDownOnMainThread() override;
 
   const extensions::Extension* LoadExtension(const base::FilePath& path);
 

chrome/browser/prerender/prerender_browsertest.cc

@@ -3446,6 +3446,11 @@ class PrerenderBrowserTestWithExtensions : public PrerenderBrowserTest,
     ExtensionApiTest::TearDownInProcessBrowserTestFixture();
   }
 
+  void TearDownOnMainThread() override {
+    PrerenderBrowserTest::TearDownOnMainThread();
+    ExtensionApiTest::TearDownOnMainThread();
+  }
+
   void SetUpOnMainThread() override {
     PrerenderBrowserTest::SetUpOnMainThread();
   }

chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa.h

@@ -47,7 +47,7 @@ class PermissionBubbleCocoa : public PermissionBubbleView {
                            HasLocationBarByDefault);
   FRIEND_TEST_ALL_PREFIXES(PermissionBubbleBrowserTest,
                            FullscreenHasLocationBar);
-  FRIEND_TEST_ALL_PREFIXES(PermissionBubbleAppBrowserTest, AppHasNoLocationBar);
+  FRIEND_TEST_ALL_PREFIXES(PermissionBubbleBrowserTest, AppHasNoLocationBar);
   FRIEND_TEST_ALL_PREFIXES(PermissionBubbleKioskBrowserTest,
                            KioskHasNoLocationBar);
 

chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa_browser_test.mm

@@ -50,8 +50,9 @@ IN_PROC_BROWSER_TEST_F(PermissionBubbleBrowserTest, FullscreenHasLocationBar) {
   bubble.Hide();
 }
 
-IN_PROC_BROWSER_TEST_F(PermissionBubbleAppBrowserTest, AppHasNoLocationBar) {
-  PermissionBubbleCocoa bubble(app_browser());
+IN_PROC_BROWSER_TEST_F(PermissionBubbleBrowserTest, AppHasNoLocationBar) {
+  Browser* app_browser = OpenExtensionAppWindow();
+  PermissionBubbleCocoa bubble(app_browser);
   bubble.SetDelegate(test_delegate());
   bubble.Show(requests(), accept_states());
   EXPECT_FALSE(bubble.HasLocationBar());

chrome/browser/ui/extensions/extension_message_bubble_browsertest.cc

@@ -25,7 +25,8 @@ void ExtensionMessageBubbleBrowserTest::SetUpCommandLine(
       new extensions::FeatureSwitch::ScopedOverride(
           extensions::FeatureSwitch::force_dev_mode_highlighting(),
           true));
-  ExtensionMessageBubbleFactory::set_enabled_for_tests(true);
+  ExtensionMessageBubbleFactory::set_override_for_tests(
+      ExtensionMessageBubbleFactory::OVERRIDE_ENABLED);
 }
 
 void ExtensionMessageBubbleBrowserTest::TestBubbleAnchoredToExtensionAction() {

chrome/browser/ui/extensions/extension_message_bubble_factory.cc

@@ -29,8 +29,10 @@ namespace {
 base::LazyInstance<std::set<Profile*> > g_profiles_evaluated =
     LAZY_INSTANCE_INITIALIZER;
 
-// This is used to turn on all bubbles for testing.
-bool g_enabled_for_tests = false;
+// This is used to turn on override whether bubbles are enabled or disabled for
+// testing.
+ExtensionMessageBubbleFactory::OverrideForTesting g_override_for_testing =
+    ExtensionMessageBubbleFactory::NO_OVERRIDE;
 
 const char kEnableDevModeWarningExperimentName[] =
     "ExtensionDeveloperModeWarning";
@@ -52,14 +54,17 @@ bool IsExperimentEnabled(const char* experiment_name) {
 }
 
 bool EnableSuspiciousExtensionsBubble() {
-  return g_enabled_for_tests || extensions::InstallVerifier::ShouldEnforce();
+  return g_override_for_testing ==
+             ExtensionMessageBubbleFactory::OVERRIDE_ENABLED ||
+         extensions::InstallVerifier::ShouldEnforce();
 }
 
 bool EnableSettingsApiBubble() {
 #if defined(OS_WIN)
   return true;
 #else
-  return g_enabled_for_tests;
+  return g_override_for_testing ==
+         ExtensionMessageBubbleFactory::OVERRIDE_ENABLED;
 #endif
 }
 
@@ -67,7 +72,8 @@ bool EnableProxyOverrideBubble() {
 #if defined(OS_WIN)
   return true;
 #else
-  return g_enabled_for_tests ||
+  return g_override_for_testing ==
+             ExtensionMessageBubbleFactory::OVERRIDE_ENABLED ||
          IsExperimentEnabled(kEnableProxyWarningExperimentName);
 #endif
 }
@@ -81,7 +87,8 @@ bool EnableDevModeBubble() {
     return true;
 #endif
 
-  return g_enabled_for_tests ||
+  return g_override_for_testing ==
+             ExtensionMessageBubbleFactory::OVERRIDE_ENABLED ||
          IsExperimentEnabled(kEnableDevModeWarningExperimentName);
 }
 
@@ -101,6 +108,9 @@ ExtensionMessageBubbleFactory::GetController() {
   bool is_initial_check = profiles_evaluated.count(original_profile) == 0;
   profiles_evaluated.insert(original_profile);
 
+  if (g_override_for_testing == OVERRIDE_DISABLED)
+    return scoped_ptr<extensions::ExtensionMessageBubbleController>();
+
   // The list of suspicious extensions takes priority over the dev mode bubble
   // and the settings API bubble, since that needs to be shown as soon as we
   // disable something. The settings API bubble is shown on first startup after
@@ -151,6 +161,7 @@ ExtensionMessageBubbleFactory::GetController() {
 }
 
 // static
-void ExtensionMessageBubbleFactory::set_enabled_for_tests(bool enabled) {
-  g_enabled_for_tests = enabled;
+void ExtensionMessageBubbleFactory::set_override_for_tests(
+    OverrideForTesting override) {
+  g_override_for_testing = override;
 }

chrome/browser/ui/extensions/extension_message_bubble_factory.h

@@ -19,14 +19,21 @@ class ExtensionMessageBubbleController;
 // running in developer mode that we want to warn the user about.
 class ExtensionMessageBubbleFactory {
  public:
+  // An enum to allow us to override the default behavior for testing.
+  enum OverrideForTesting {
+    NO_OVERRIDE,
+    OVERRIDE_ENABLED,
+    OVERRIDE_DISABLED,
+  };
+
   explicit ExtensionMessageBubbleFactory(Browser* browser);
   ~ExtensionMessageBubbleFactory();
 
   // Returns the controller for the bubble that should be shown, if any.
   scoped_ptr<extensions::ExtensionMessageBubbleController> GetController();
 
-  // Enables the bubbles across all platforms for testing.
-  static void set_enabled_for_tests(bool enabled);
+  // Overrides the default behavior for testing.
+  static void set_override_for_tests(OverrideForTesting override);
 
  private:
   Browser* browser_;

chrome/browser/ui/website_settings/permission_bubble_browser_test_util.cc

@@ -21,15 +21,14 @@ TestPermissionBubbleViewDelegate::TestPermissionBubbleViewDelegate()
     : PermissionBubbleView::Delegate() {
 }
 
-PermissionBubbleBrowserTest::PermissionBubbleBrowserTest()
-    : InProcessBrowserTest() {
+PermissionBubbleBrowserTest::PermissionBubbleBrowserTest() {
 }
 
 PermissionBubbleBrowserTest::~PermissionBubbleBrowserTest() {
 }
 
 void PermissionBubbleBrowserTest::SetUpOnMainThread() {
-  InProcessBrowserTest::SetUpOnMainThread();
+  ExtensionBrowserTest::SetUpOnMainThread();
 
   // Add a single permission request.
   MockPermissionBubbleRequest* request = new MockPermissionBubbleRequest(
@@ -38,57 +37,26 @@ void PermissionBubbleBrowserTest::SetUpOnMainThread() {
   requests_.push_back(request);
 }
 
-PermissionBubbleAppBrowserTest::PermissionBubbleAppBrowserTest()
-    : InProcessBrowserTest(),
-      PermissionBubbleBrowserTest(),
-      ExtensionBrowserTest(),
-      app_browser_(nullptr) {
-}
-
-PermissionBubbleAppBrowserTest::~PermissionBubbleAppBrowserTest() {
-}
-
-void PermissionBubbleAppBrowserTest::SetUpOnMainThread() {
-  PermissionBubbleBrowserTest::SetUpOnMainThread();
-  ExtensionBrowserTest::SetUpOnMainThread();
-
+Browser* PermissionBubbleBrowserTest::OpenExtensionAppWindow() {
   auto extension =
       LoadExtension(test_data_dir_.AppendASCII("app_with_panel_container/"));
-  ASSERT_TRUE(extension);
-
-  app_browser_ = OpenExtensionAppWindow(extension);
-  ASSERT_TRUE(app_browser());
-  ASSERT_TRUE(app_browser()->is_app());
-}
-
-void PermissionBubbleAppBrowserTest::SetUp() {
-  ExtensionBrowserTest::SetUp();
-}
-
-void PermissionBubbleAppBrowserTest::SetUpCommandLine(
-    base::CommandLine* command_line) {
-  ExtensionBrowserTest::SetUpCommandLine(command_line);
-}
-
-Browser* PermissionBubbleAppBrowserTest::OpenExtensionAppWindow(
-    const extensions::Extension* extension) {
-  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
-  command_line.AppendSwitchASCII(switches::kAppId, extension->id());
+  CHECK(extension);
 
   AppLaunchParams params(browser()->profile(), extension,
                          extensions::LAUNCH_CONTAINER_PANEL, NEW_WINDOW,
-                         extensions::SOURCE_COMMAND_LINE);
-  params.command_line = command_line;
-  params.current_directory = base::FilePath();
+                         extensions::SOURCE_TEST);
 
   content::WebContents* app_window = OpenApplication(params);
-  assert(app_window);
+  CHECK(app_window);
+
+  Browser* app_browser = chrome::FindBrowserWithWebContents(app_window);
+  CHECK(app_browser);
+  CHECK(app_browser->is_app());
 
-  return chrome::FindBrowserWithWebContents(app_window);
+  return app_browser;
 }
 
-PermissionBubbleKioskBrowserTest::PermissionBubbleKioskBrowserTest()
-    : PermissionBubbleBrowserTest() {
+PermissionBubbleKioskBrowserTest::PermissionBubbleKioskBrowserTest() {
 }
 
 PermissionBubbleKioskBrowserTest::~PermissionBubbleKioskBrowserTest() {

chrome/browser/ui/website_settings/permission_bubble_browser_test_util.h

@@ -27,14 +27,19 @@ class TestPermissionBubbleViewDelegate : public PermissionBubbleView::Delegate {
   DISALLOW_COPY_AND_ASSIGN(TestPermissionBubbleViewDelegate);
 };
 
-// Use this class to test on a default window.
-class PermissionBubbleBrowserTest : public virtual InProcessBrowserTest {
+// Use this class to test on a default window or an app window. Inheriting from
+// ExtensionBrowserTest allows us to easily load and launch apps, and doesn't
+// really add any extra work.
+class PermissionBubbleBrowserTest : public ExtensionBrowserTest {
  public:
   PermissionBubbleBrowserTest();
   ~PermissionBubbleBrowserTest() override;
 
   void SetUpOnMainThread() override;
 
+  // Opens an app window, and returns the associated browser.
+  Browser* OpenExtensionAppWindow();
+
   std::vector<PermissionBubbleRequest*> requests() { return requests_.get(); }
   std::vector<bool> accept_states() { return accept_states_; }
   PermissionBubbleView::Delegate* test_delegate() { return &test_delegate_; }
@@ -43,27 +48,8 @@ class PermissionBubbleBrowserTest : public virtual InProcessBrowserTest {
   TestPermissionBubbleViewDelegate test_delegate_;
   ScopedVector<PermissionBubbleRequest> requests_;
   std::vector<bool> accept_states_;
-};
-
-// Use this class to test on an app window.
-class PermissionBubbleAppBrowserTest : public PermissionBubbleBrowserTest,
-                                       public ExtensionBrowserTest {
- public:
-  PermissionBubbleAppBrowserTest();
-  ~PermissionBubbleAppBrowserTest() override;
-
-  void SetUpOnMainThread() override;
 
-  // Override from ExtensionBrowserTest to avoid "inheritance via dominance".
-  void SetUp() override;
-  void SetUpCommandLine(base::CommandLine* command_line) override;
-
-  Browser* app_browser() { return app_browser_; }
-
- private:
-  Browser* app_browser_;
-
-  Browser* OpenExtensionAppWindow(const extensions::Extension* extension);
+  DISALLOW_COPY_AND_ASSIGN(PermissionBubbleBrowserTest);
 };
 
 // Use this class to test on a kiosk window.
@@ -73,6 +59,8 @@ class PermissionBubbleKioskBrowserTest : public PermissionBubbleBrowserTest {
   ~PermissionBubbleKioskBrowserTest() override;
 
   void SetUpCommandLine(base::CommandLine* command_line) override;
+
+  DISALLOW_COPY_AND_ASSIGN(PermissionBubbleKioskBrowserTest);
 };
 
 #endif  // CHROME_BROWSER_UI_WEBSITE_SETTINGS_PERMISSION_BUBBLE_BROWSER_TEST_UTIL_H_