Revert of [Extensions] Don't count bubble dismissal from focus loss as acknowledgment

chrome/browser/extensions/extension_message_bubble_controller_unittest.cc

 // Copyright (c) 2013 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/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/dev_mode_bubble_delegate.h"
 #include "chrome/browser/extensions/extension_function_test_utils.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/ntp_overridden_bubble_delegate.h"
 #include "chrome/browser/extensions/proxy_overridden_bubble_delegate.h"
@@ skipping 37 matching lines @@
         action_button_callback_count_(0),
         dismiss_button_callback_count_(0),
         link_click_callback_count_(0) {}
   ~TestExtensionMessageBubbleController() override {}
 
   // ExtensionMessageBubbleController:
   void OnBubbleAction() override {
     ++action_button_callback_count_;
     ExtensionMessageBubbleController::OnBubbleAction();
   }
-  void OnBubbleDismiss(bool by_deactivation) override {
+  void OnBubbleDismiss() override {
     ++dismiss_button_callback_count_;
-    ExtensionMessageBubbleController::OnBubbleDismiss(by_deactivation);
+    ExtensionMessageBubbleController::OnBubbleDismiss();
   }
   void OnLinkClicked() override {
     ++link_click_callback_count_;
     ExtensionMessageBubbleController::OnLinkClicked();
   }
 
   size_t action_click_count() { return action_button_callback_count_; }
   size_t dismiss_click_count() { return dismiss_button_callback_count_; }
   size_t link_click_count() { return link_click_callback_count_; }
 
  private:
   // How often each button has been called.
   size_t action_button_callback_count_;
   size_t dismiss_button_callback_count_;
   size_t link_click_callback_count_;
 
   DISALLOW_COPY_AND_ASSIGN(TestExtensionMessageBubbleController);
 };
 
 // A fake bubble used for testing the controller. Takes an action that specifies
 // what should happen when the bubble is "shown" (the bubble is actually not
 // shown, the corresponding action is taken immediately).
 class FakeExtensionMessageBubble {
  public:
   enum ExtensionBubbleAction {
     BUBBLE_ACTION_CLICK_ACTION_BUTTON = 0,
     BUBBLE_ACTION_CLICK_DISMISS_BUTTON,
-    BUBBLE_ACTION_DISMISS_DEACTIVATION,
     BUBBLE_ACTION_CLICK_LINK,
   };
 
-  FakeExtensionMessageBubble()
-      : action_(BUBBLE_ACTION_CLICK_ACTION_BUTTON), controller_(nullptr) {}
+  FakeExtensionMessageBubble() : controller_(nullptr) {}
 
   void set_action_on_show(ExtensionBubbleAction action) {
     action_ = action;
   }
   void set_controller(ExtensionMessageBubbleController* controller) {
     controller_ = controller;
   }
 
   void Show() {
     controller_->OnShown();
-    switch (action_) {
-      case BUBBLE_ACTION_CLICK_ACTION_BUTTON:
-        controller_->OnBubbleAction();
-        break;
-      case BUBBLE_ACTION_CLICK_DISMISS_BUTTON:
-        controller_->OnBubbleDismiss(false);
-        break;
-      case BUBBLE_ACTION_DISMISS_DEACTIVATION:
-        controller_->OnBubbleDismiss(true);
-        break;
-      case BUBBLE_ACTION_CLICK_LINK:
-        controller_->OnLinkClicked();
-        break;
-    }
+    if (action_ == BUBBLE_ACTION_CLICK_ACTION_BUTTON)
+      controller_->OnBubbleAction();
+    else if (action_ == BUBBLE_ACTION_CLICK_DISMISS_BUTTON)
+      controller_->OnBubbleDismiss();
+    else if (action_ == BUBBLE_ACTION_CLICK_LINK)
+      controller_->OnLinkClicked();
   }
 
  private:
   ExtensionBubbleAction action_;
   ExtensionMessageBubbleController* controller_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeExtensionMessageBubble);
 };
 
 class ExtensionMessageBubbleTest : public BrowserWithTestWindowTest {
@@ skipping 176 matching lines @@
   }
 
   ExtensionService* service_;
 
  private:
   scoped_ptr<base::CommandLine> command_line_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionMessageBubbleTest);
 };
 
-TEST_F(ExtensionMessageBubbleTest, BubbleReshowsOnDeactivationDismissal) {
-  Init();
-
-  ASSERT_TRUE(LoadExtensionOverridingNtp("1", kId1, Manifest::INTERNAL));
-  ASSERT_TRUE(LoadExtensionOverridingNtp("2", kId2, Manifest::INTERNAL));
-  scoped_ptr<TestExtensionMessageBubbleController> controller(
-      new TestExtensionMessageBubbleController(
-          new NtpOverriddenBubbleDelegate(browser()->profile()), browser()));
-
-  // The list will contain one enabled unpacked extension (ext 2).
-  EXPECT_TRUE(controller->ShouldShow());
-  std::vector<base::string16> override_extensions =
-      controller->GetExtensionList();
-  ASSERT_EQ(1U, override_extensions.size());
-  EXPECT_EQ(base::ASCIIToUTF16("Extension 2"), override_extensions[0]);
-  EXPECT_EQ(0U, controller->link_click_count());
-  EXPECT_EQ(0U, controller->dismiss_click_count());
-  EXPECT_EQ(0U, controller->action_click_count());
-
-  // Simulate showing the bubble and dismissing it due to deactivation.
-  FakeExtensionMessageBubble bubble;
-  bubble.set_action_on_show(
-      FakeExtensionMessageBubble::BUBBLE_ACTION_DISMISS_DEACTIVATION);
-  bubble.set_controller(controller.get());
-  bubble.Show();
-  EXPECT_EQ(0U, controller->link_click_count());
-  EXPECT_EQ(0U, controller->action_click_count());
-  EXPECT_EQ(1U, controller->dismiss_click_count());
-
-  // No extension should have become disabled.
-  ExtensionRegistry* registry = ExtensionRegistry::Get(profile());
-  EXPECT_TRUE(registry->enabled_extensions().GetByID(kId2));
-  // And since it was dismissed due to deactivation, the extension should not
-  // have been acknowledged.
-  EXPECT_FALSE(controller->delegate()->HasBubbleInfoBeenAcknowledged(kId2));
-
-  bubble.set_action_on_show(
-      FakeExtensionMessageBubble::BUBBLE_ACTION_DISMISS_DEACTIVATION);
-  controller.reset(new TestExtensionMessageBubbleController(
-      new NtpOverriddenBubbleDelegate(browser()->profile()), browser()));
-  // The bubble shouldn't show again for the same profile (we don't want to
-  // be annoying).
-  EXPECT_FALSE(controller->ShouldShow());
-  controller->ClearProfileListForTesting();
-  EXPECT_TRUE(controller->ShouldShow());
-  // Explicitly click the dismiss button. The extension should be acknowledged.
-  bubble.set_controller(controller.get());
-  bubble.set_action_on_show(
-      FakeExtensionMessageBubble::BUBBLE_ACTION_CLICK_DISMISS_BUTTON);
-  bubble.Show();
-  EXPECT_TRUE(controller->delegate()->HasBubbleInfoBeenAcknowledged(kId2));
-
-  // Uninstall the current ntp-controlling extension, allowing the other to
-  // take control.
-  service_->UninstallExtension(kId2, UNINSTALL_REASON_FOR_TESTING,
-                               base::Bind(&base::DoNothing), nullptr);
-
-  // Even though we already showed for the given profile, we should show again,
-  // because it's a different extension.
-  controller.reset(new TestExtensionMessageBubbleController(
-      new NtpOverriddenBubbleDelegate(browser()->profile()), browser()));
-  EXPECT_TRUE(controller->ShouldShow());
-}
-
 // The feature this is meant to test is only enacted on Windows, but it should
 // pass on all platforms.
 TEST_F(ExtensionMessageBubbleTest, WipeoutControllerTest) {
   Init();
   // Add three extensions, and control two of them in this test (extension 1
   // and 2).
   ASSERT_TRUE(LoadExtensionWithAction("1", kId1, Manifest::COMMAND_LINE));
   ASSERT_TRUE(LoadGenericExtension("2", kId2, Manifest::UNPACKED));
   ASSERT_TRUE(LoadGenericExtension("3", kId3, Manifest::EXTERNAL_POLICY));
 
@@ skipping 18 matching lines @@
 
   // Now disable an extension, specifying the wipeout flag.
   service_->DisableExtension(kId1, Extension::DISABLE_NOT_VERIFIED);
 
   EXPECT_FALSE(controller->delegate()->HasBubbleInfoBeenAcknowledged(kId1));
   EXPECT_FALSE(controller->delegate()->HasBubbleInfoBeenAcknowledged(kId2));
   controller.reset(
       new TestExtensionMessageBubbleController(
           new SuspiciousExtensionBubbleDelegate(browser()->profile()),
           browser()));
-  controller->ClearProfileListForTesting();
+  SuspiciousExtensionBubbleDelegate::ClearProfileListForTesting();
   EXPECT_TRUE(controller->ShouldShow());
   suspicious_extensions = controller->GetExtensionList();
   ASSERT_EQ(1U, suspicious_extensions.size());
   EXPECT_TRUE(base::ASCIIToUTF16("Extension 1") == suspicious_extensions[0]);
   bubble.set_controller(controller.get());
   bubble.Show();  // Simulate showing the bubble.
   EXPECT_EQ(0U, controller->link_click_count());
   EXPECT_EQ(1U, controller->dismiss_click_count());
   // Now the acknowledge flag should be set only for the first extension.
   EXPECT_TRUE(controller->delegate()->HasBubbleInfoBeenAcknowledged(kId1));
   EXPECT_FALSE(controller->delegate()->HasBubbleInfoBeenAcknowledged(kId2));
   // Clear the flag.
   controller->delegate()->SetBubbleInfoBeenAcknowledged(kId1, false);
   EXPECT_FALSE(controller->delegate()->HasBubbleInfoBeenAcknowledged(kId1));
 
   // Now disable the other extension and exercise the link click code path.
   service_->DisableExtension(kId2, Extension::DISABLE_NOT_VERIFIED);
 
   bubble.set_action_on_show(
       FakeExtensionMessageBubble::BUBBLE_ACTION_CLICK_LINK);
   controller.reset(
       new TestExtensionMessageBubbleController(
           new SuspiciousExtensionBubbleDelegate(browser()->profile()),
           browser()));
-  controller->ClearProfileListForTesting();
+  SuspiciousExtensionBubbleDelegate::ClearProfileListForTesting();
   EXPECT_TRUE(controller->ShouldShow());
   suspicious_extensions = controller->GetExtensionList();
   ASSERT_EQ(2U, suspicious_extensions.size());
   EXPECT_TRUE(base::ASCIIToUTF16("Extension 1") == suspicious_extensions[1]);
   EXPECT_TRUE(base::ASCIIToUTF16("Extension 2") == suspicious_extensions[0]);
   bubble.set_controller(controller.get());
   bubble.Show();  // Simulate showing the bubble.
   EXPECT_EQ(1U, controller->link_click_count());
   EXPECT_EQ(0U, controller->dismiss_click_count());
   EXPECT_TRUE(controller->delegate()->HasBubbleInfoBeenAcknowledged(kId1));
@@ skipping 41 matching lines @@
   EXPECT_TRUE(registry->enabled_extensions().GetByID(kId1) != NULL);
   EXPECT_TRUE(registry->enabled_extensions().GetByID(kId2) != NULL);
 
   // Do it again, but now press different button (Disable).
   bubble.set_action_on_show(
       FakeExtensionMessageBubble::BUBBLE_ACTION_CLICK_ACTION_BUTTON);
   controller.reset(
       new TestExtensionMessageBubbleController(
           new DevModeBubbleDelegate(browser()->profile()),
           browser()));
-  // Most bubbles would want to show again as long as the extensions weren't
-  // acknowledged and the bubble wasn't dismissed due to deactivation. Since dev
-  // mode extensions can't be (persistently) acknowledged, this isn't the case
-  // for the dev mode bubble, and we should only show once per profile.
-  EXPECT_FALSE(controller->ShouldShow());
-  controller->ClearProfileListForTesting();
+  DevModeBubbleDelegate::ClearProfileListForTesting();
   EXPECT_TRUE(controller->ShouldShow());
   dev_mode_extensions = controller->GetExtensionList();
   EXPECT_EQ(2U, dev_mode_extensions.size());
   bubble.set_controller(controller.get());
   bubble.Show();  // Simulate showing the bubble.
   EXPECT_EQ(0U, controller->link_click_count());
   EXPECT_EQ(1U, controller->action_click_count());
   EXPECT_EQ(0U, controller->dismiss_click_count());
   EXPECT_TRUE(registry->disabled_extensions().GetByID(kId1) != NULL);
   EXPECT_TRUE(registry->disabled_extensions().GetByID(kId2) != NULL);
 
   // Re-enable the extensions (disabled by the action button above).
   service_->EnableExtension(kId1);
   service_->EnableExtension(kId2);
 
   // Show the dialog a third time, but now press the learn more link.
   bubble.set_action_on_show(
       FakeExtensionMessageBubble::BUBBLE_ACTION_CLICK_LINK);
   controller.reset(
       new TestExtensionMessageBubbleController(
           new DevModeBubbleDelegate(browser()->profile()),
           browser()));
-  controller->ClearProfileListForTesting();
+  DevModeBubbleDelegate::ClearProfileListForTesting();
   EXPECT_TRUE(controller->ShouldShow());
   dev_mode_extensions = controller->GetExtensionList();
   EXPECT_EQ(2U, dev_mode_extensions.size());
   bubble.set_controller(controller.get());
   bubble.Show();  // Simulate showing the bubble.
   EXPECT_EQ(1U, controller->link_click_count());
   EXPECT_EQ(0U, controller->action_click_count());
   EXPECT_EQ(0U, controller->dismiss_click_count());
   EXPECT_TRUE(registry->enabled_extensions().GetByID(kId1) != NULL);
   EXPECT_TRUE(registry->enabled_extensions().GetByID(kId2) != NULL);
 
   // Now disable the unpacked extension.
   service_->DisableExtension(kId1, Extension::DISABLE_USER_ACTION);
   service_->DisableExtension(kId2, Extension::DISABLE_USER_ACTION);
 
   controller.reset(
       new TestExtensionMessageBubbleController(
           new DevModeBubbleDelegate(browser()->profile()),
           browser()));
-  controller->ClearProfileListForTesting();
+  DevModeBubbleDelegate::ClearProfileListForTesting();
   EXPECT_FALSE(controller->ShouldShow());
   dev_mode_extensions = controller->GetExtensionList();
   EXPECT_EQ(0U, dev_mode_extensions.size());
 }
 
 // The feature this is meant to test is only implemented on Windows.
 #if defined(OS_WIN)
 #define MAYBE_SettingsApiControllerTest SettingsApiControllerTest
 #else
 #define MAYBE_SettingsApiControllerTest DISABLED_SettingsApiControllerTest
@@ skipping 384 matching lines @@
                                extensions::UNINSTALL_REASON_FOR_TESTING,
                                base::Bind(&base::DoNothing),
                                NULL);
   service_->UninstallExtension(kId3,
                                extensions::UNINSTALL_REASON_FOR_TESTING,
                                base::Bind(&base::DoNothing),
                                NULL);
 }
 
 }  // namespace extensions