[Reland][Extensions] Don't count bubble focus loss as acknowledgment

Currently, if an extension message bubble is shown, and then it is dismissed
because it loses focus, we treat it the same as the user clicking the dismiss
button - which serves as acknowledging the extension.  We could ignore focus
loss, but this makes for very noisy, awkward bubbles.  Instead, allow the bubble
to close, but don't treat this as user acknowledgment, and show the bubble again
on next startup.

This also involves tracking the close reason for a BubbleDelegateView.

BUG=548269
TBR=finnur@chromium.org (no changes relevant from original CL)
TBR=avi@chromium.org (no changes relevant from original CL)

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

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

chrome/browser/extensions/dev_mode_bubble_delegate.cc

@@ -19,13 +19,6 @@
 
 namespace extensions {
 
-namespace {
-
-base::LazyInstance<std::set<Profile*> > g_shown_for_profiles =
-    LAZY_INSTANCE_INITIALIZER;
-
-}  // namespace
-
 DevModeBubbleDelegate::DevModeBubbleDelegate(Profile* profile)
     : ExtensionMessageBubbleController::Delegate(profile) {
 }
@@ -105,13 +98,12 @@ void DevModeBubbleDelegate::LogAction(
       action, ExtensionMessageBubbleController::ACTION_BOUNDARY);
 }
 
-std::set<Profile*>* DevModeBubbleDelegate::GetProfileSet() {
-  return g_shown_for_profiles.Pointer();
+const char* DevModeBubbleDelegate::GetKey() {
+  return "DevModeBubbleDelegate";
 }
 
-// static
-void DevModeBubbleDelegate::ClearProfileListForTesting() {
-  g_shown_for_profiles.Get().clear();
+bool DevModeBubbleDelegate::ClearProfileSetAfterAction() {
+  return false;
 }
 
 }  // namespace extensions

chrome/browser/extensions/dev_mode_bubble_delegate.h

@@ -40,9 +40,8 @@ class DevModeBubbleDelegate
   bool ShouldLimitToEnabledExtensions() const override;
   void LogExtensionCount(size_t count) override;
   void LogAction(ExtensionMessageBubbleController::BubbleAction) override;
-  std::set<Profile*>* GetProfileSet() override;
-
-  static void ClearProfileListForTesting();
+  const char* GetKey() override;
+  bool ClearProfileSetAfterAction() override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DevModeBubbleDelegate);

chrome/browser/extensions/extension_message_bubble_controller.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/extensions/extension_message_bubble_controller.h"
 
 #include "base/bind.h"
+#include "base/lazy_instance.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ -24,12 +25,18 @@
 namespace extensions {
 
 namespace {
+
 // How many extensions to show in the bubble (max).
 const int kMaxExtensionsToShow = 7;
 
 // Whether or not to ignore the learn more link navigation for testing.
 bool g_should_ignore_learn_more_for_testing = false;
-}
+
+using ProfileSetMap = std::map<std::string, std::set<Profile*>>;
+base::LazyInstance<ProfileSetMap> g_shown_for_profiles =
+    LAZY_INSTANCE_INITIALIZER;
+
+}  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 // ExtensionMessageBubbleController::Delegate
@@ -48,6 +55,10 @@ base::string16 ExtensionMessageBubbleController::Delegate::GetLearnMoreLabel()
   return l10n_util::GetStringUTF16(IDS_LEARN_MORE);
 }
 
+bool ExtensionMessageBubbleController::Delegate::ClearProfileSetAfterAction() {
+  return true;
+}
+
 bool ExtensionMessageBubbleController::Delegate::HasBubbleInfoBeenAcknowledged(
     const std::string& extension_id) {
   std::string pref_name = get_acknowledged_flag_pref_name();
@@ -71,11 +82,6 @@ void ExtensionMessageBubbleController::Delegate::SetBubbleInfoBeenAcknowledged(
                              value ? new base::FundamentalValue(value) : NULL);
 }
 
-std::set<Profile*>*
-ExtensionMessageBubbleController::Delegate::GetProfileSet() {
-  return nullptr;
-}
-
 std::string
 ExtensionMessageBubbleController::Delegate::get_acknowledged_flag_pref_name()
     const {
@@ -108,8 +114,8 @@ Profile* ExtensionMessageBubbleController::profile() {
 }
 
 bool ExtensionMessageBubbleController::ShouldShow() {
-  std::set<Profile*>* profiles = delegate_->GetProfileSet();
-  return (!profiles || !profiles->count(profile()->GetOriginalProfile())) &&
+  std::set<Profile*>* profiles = GetProfileSet();
+  return !profiles->count(profile()->GetOriginalProfile()) &&
          !GetExtensionList().empty();
 }
 
@@ -170,9 +176,7 @@ void ExtensionMessageBubbleController::HighlightExtensionsIfNecessary() {
 }
 
 void ExtensionMessageBubbleController::OnShown() {
-  std::set<Profile*>* profiles = delegate_->GetProfileSet();
-  if (profiles)
-    profiles->insert(profile()->GetOriginalProfile());
+  GetProfileSet()->insert(profile()->GetOriginalProfile());
 }
 
 void ExtensionMessageBubbleController::OnBubbleAction() {
@@ -185,18 +189,21 @@ void ExtensionMessageBubbleController::OnBubbleAction() {
   OnClose();
 }
 
-void ExtensionMessageBubbleController::OnBubbleDismiss() {
+void ExtensionMessageBubbleController::OnBubbleDismiss(
+    bool closed_by_deactivation) {
   // OnBubbleDismiss() can be called twice when we receive multiple
   // "OnWidgetDestroying" notifications (this can at least happen when we close
   // a window with a notification open). Handle this gracefully.
   if (user_action_ != ACTION_BOUNDARY) {
-    DCHECK(user_action_ == ACTION_DISMISS);
+    DCHECK(user_action_ == ACTION_DISMISS_USER_ACTION ||
+           user_action_ == ACTION_DISMISS_DEACTIVATION);
     return;
   }
 
-  user_action_ = ACTION_DISMISS;
+  user_action_ = closed_by_deactivation ? ACTION_DISMISS_DEACTIVATION
+                                        : ACTION_DISMISS_USER_ACTION;
 
-  delegate_->LogAction(ACTION_DISMISS);
+  delegate_->LogAction(user_action_);
 
   OnClose();
 }
@@ -217,6 +224,10 @@ void ExtensionMessageBubbleController::OnLinkClicked() {
   OnClose();
 }
 
+void ExtensionMessageBubbleController::ClearProfileListForTesting() {
+  GetProfileSet()->clear();
+}
+
 // static
 void ExtensionMessageBubbleController::set_should_ignore_learn_more_for_testing(
     bool should_ignore) {
@@ -252,9 +263,22 @@ ExtensionIdList* ExtensionMessageBubbleController::GetOrCreateExtensionList() {
 }
 
 void ExtensionMessageBubbleController::OnClose() {
-  AcknowledgeExtensions();
+  DCHECK_NE(ACTION_BOUNDARY, user_action_);
+  // If the bubble was closed due to deactivation, don't treat it as
+  // acknowledgment so that the user will see the bubble again (until they
+  // explicitly take an action).
+  if (user_action_ != ACTION_DISMISS_DEACTIVATION) {
+    AcknowledgeExtensions();
+    if (delegate_->ClearProfileSetAfterAction())
+      GetProfileSet()->clear();
+  }
+
   if (did_highlight_)
     ToolbarActionsModel::Get(profile())->StopHighlighting();
 }
 
+std::set<Profile*>* ExtensionMessageBubbleController::GetProfileSet() {
+  return &g_shown_for_profiles.Get()[delegate_->GetKey()];
+}
+
 }  // namespace extensions

chrome/browser/extensions/extension_message_bubble_controller.h

@@ -24,8 +24,9 @@ class ExtensionMessageBubbleController {
   enum BubbleAction {
     ACTION_LEARN_MORE = 0,
     ACTION_EXECUTE,
-    ACTION_DISMISS,
-    ACTION_BOUNDARY, // Must be the last value.
+    ACTION_DISMISS_USER_ACTION,
+    ACTION_DISMISS_DEACTIVATION,
+    ACTION_BOUNDARY,  // Must be the last value.
   };
 
   class Delegate {
@@ -74,14 +75,22 @@ class ExtensionMessageBubbleController {
     virtual void LogExtensionCount(size_t count) = 0;
     virtual void LogAction(BubbleAction action) = 0;
 
-    // Has the user acknowledged info about the extension the bubble reports.
-    virtual bool HasBubbleInfoBeenAcknowledged(const std::string& extension_id);
-    virtual void SetBubbleInfoBeenAcknowledged(const std::string& extension_id,
-                                               bool value);
+    // Returns a key unique to the type of bubble that can be used to retrieve
+    // state specific to the type (e.g., shown for profiles).
+    virtual const char* GetKey() = 0;
+
+    // Whether the "shown for profiles" set should be cleared if an action is
+    // taken on the bubble. This defaults to true, since once an action is
+    // taken, the extension will usually either be acknowledged or removed, and
+    // the bubble won't show for that extension.
+    // This should be false in cases where there is no acknowledgment option
+    // (as in the developer-mode extension warning).
+    virtual bool ClearProfileSetAfterAction();
 
-    // Returns the set of profiles for which this bubble has been shown.
-    // If profiles are not tracked, returns null (default).
-    virtual std::set<Profile*>* GetProfileSet();
+    // Has the user acknowledged info about the extension the bubble reports.
+    bool HasBubbleInfoBeenAcknowledged(const std::string& extension_id);
+    void SetBubbleInfoBeenAcknowledged(const std::string& extension_id,
+                                       bool value);
 
    protected:
     Profile* profile() { return profile_; }
@@ -141,9 +150,11 @@ class ExtensionMessageBubbleController {
 
   // Callbacks from bubble. Declared virtual for testing purposes.
   virtual void OnBubbleAction();
-  virtual void OnBubbleDismiss();
+  virtual void OnBubbleDismiss(bool dismissed_by_deactivation);
   virtual void OnLinkClicked();
 
+  void ClearProfileListForTesting();
+
   static void set_should_ignore_learn_more_for_testing(
       bool should_ignore_learn_more);
 
@@ -157,6 +168,8 @@ class ExtensionMessageBubbleController {
   // Performs cleanup after the bubble closes.
   void OnClose();
 
+  std::set<Profile*>* GetProfileSet();
+
   // A weak pointer to the Browser we are associated with. Not owned by us.
   Browser* browser_;
 

chrome/browser/extensions/extension_message_bubble_controller_unittest.cc

@@ -2,6 +2,7 @@
 // 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"
@@ -59,9 +60,9 @@ class TestExtensionMessageBubbleController :
     ++action_button_callback_count_;
     ExtensionMessageBubbleController::OnBubbleAction();
   }
-  void OnBubbleDismiss() override {
+  void OnBubbleDismiss(bool by_deactivation) override {
     ++dismiss_button_callback_count_;
-    ExtensionMessageBubbleController::OnBubbleDismiss();
+    ExtensionMessageBubbleController::OnBubbleDismiss(by_deactivation);
   }
   void OnLinkClicked() override {
     ++link_click_callback_count_;
@@ -89,10 +90,12 @@ class FakeExtensionMessageBubble {
   enum ExtensionBubbleAction {
     BUBBLE_ACTION_CLICK_ACTION_BUTTON = 0,
     BUBBLE_ACTION_CLICK_DISMISS_BUTTON,
+    BUBBLE_ACTION_DISMISS_DEACTIVATION,
     BUBBLE_ACTION_CLICK_LINK,
   };
 
-  FakeExtensionMessageBubble() : controller_(nullptr) {}
+  FakeExtensionMessageBubble()
+      : action_(BUBBLE_ACTION_CLICK_ACTION_BUTTON), controller_(nullptr) {}
 
   void set_action_on_show(ExtensionBubbleAction action) {
     action_ = action;
@@ -103,12 +106,20 @@ class FakeExtensionMessageBubble {
 
   void Show() {
     controller_->OnShown();
-    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();
+    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;
+    }
   }
 
  private:
@@ -305,6 +316,70 @@ class ExtensionMessageBubbleTest : public BrowserWithTestWindowTest {
   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) {
@@ -343,7 +418,7 @@ TEST_F(ExtensionMessageBubbleTest, WipeoutControllerTest) {
       new TestExtensionMessageBubbleController(
           new SuspiciousExtensionBubbleDelegate(browser()->profile()),
           browser()));
-  SuspiciousExtensionBubbleDelegate::ClearProfileListForTesting();
+  controller->ClearProfileListForTesting();
   EXPECT_TRUE(controller->ShouldShow());
   suspicious_extensions = controller->GetExtensionList();
   ASSERT_EQ(1U, suspicious_extensions.size());
@@ -368,7 +443,7 @@ TEST_F(ExtensionMessageBubbleTest, WipeoutControllerTest) {
       new TestExtensionMessageBubbleController(
           new SuspiciousExtensionBubbleDelegate(browser()->profile()),
           browser()));
-  SuspiciousExtensionBubbleDelegate::ClearProfileListForTesting();
+  controller->ClearProfileListForTesting();
   EXPECT_TRUE(controller->ShouldShow());
   suspicious_extensions = controller->GetExtensionList();
   ASSERT_EQ(2U, suspicious_extensions.size());
@@ -430,7 +505,12 @@ TEST_F(ExtensionMessageBubbleTest, DevModeControllerTest) {
       new TestExtensionMessageBubbleController(
           new DevModeBubbleDelegate(browser()->profile()),
           browser()));
-  DevModeBubbleDelegate::ClearProfileListForTesting();
+  // 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();
   EXPECT_TRUE(controller->ShouldShow());
   dev_mode_extensions = controller->GetExtensionList();
   EXPECT_EQ(2U, dev_mode_extensions.size());
@@ -453,7 +533,7 @@ TEST_F(ExtensionMessageBubbleTest, DevModeControllerTest) {
       new TestExtensionMessageBubbleController(
           new DevModeBubbleDelegate(browser()->profile()),
           browser()));
-  DevModeBubbleDelegate::ClearProfileListForTesting();
+  controller->ClearProfileListForTesting();
   EXPECT_TRUE(controller->ShouldShow());
   dev_mode_extensions = controller->GetExtensionList();
   EXPECT_EQ(2U, dev_mode_extensions.size());
@@ -473,7 +553,7 @@ TEST_F(ExtensionMessageBubbleTest, DevModeControllerTest) {
       new TestExtensionMessageBubbleController(
           new DevModeBubbleDelegate(browser()->profile()),
           browser()));
-  DevModeBubbleDelegate::ClearProfileListForTesting();
+  controller->ClearProfileListForTesting();
   EXPECT_FALSE(controller->ShouldShow());
   dev_mode_extensions = controller->GetExtensionList();
   EXPECT_EQ(0U, dev_mode_extensions.size());

chrome/browser/extensions/ntp_overridden_bubble_delegate.cc

@@ -129,4 +129,8 @@ void NtpOverriddenBubbleDelegate::LogAction(
       ExtensionMessageBubbleController::ACTION_BOUNDARY);
 }
 
+const char* NtpOverriddenBubbleDelegate::GetKey() {
+  return "NtpOverriddenBubbleDelegate";
+}
+
 }  // namespace extensions

chrome/browser/extensions/ntp_overridden_bubble_delegate.h

@@ -39,6 +39,7 @@ class NtpOverriddenBubbleDelegate
   bool ShouldLimitToEnabledExtensions() const override;
   void LogExtensionCount(size_t count) override;
   void LogAction(ExtensionMessageBubbleController::BubbleAction) override;
+  const char* GetKey() override;
 
  private:
   // The ID of the extension we are showing the bubble for.

chrome/browser/extensions/proxy_overridden_bubble_delegate.cc

@@ -143,4 +143,8 @@ void ProxyOverriddenBubbleDelegate::LogAction(
                             ExtensionMessageBubbleController::ACTION_BOUNDARY);
 }
 
+const char* ProxyOverriddenBubbleDelegate::GetKey() {
+  return "ProxyOverriddenBubbleDelegate";
+}
+
 }  // namespace extensions

chrome/browser/extensions/proxy_overridden_bubble_delegate.h

@@ -40,6 +40,7 @@ class ProxyOverriddenBubbleDelegate
   bool ShouldLimitToEnabledExtensions() const override;
   void LogExtensionCount(size_t count) override;
   void LogAction(ExtensionMessageBubbleController::BubbleAction) override;
+  const char* GetKey() override;
 
  private:
   // The ID of the extension we are showing the bubble for.

chrome/browser/extensions/settings_api_bubble_delegate.cc

@@ -238,4 +238,17 @@ void SettingsApiBubbleDelegate::LogAction(
   }
 }
 
+const char* SettingsApiBubbleDelegate::GetKey() {
+  switch (type_) {
+    case BUBBLE_TYPE_HOME_PAGE:
+      return "SettingsApiBubbleDelegate.HomePage";
+    case BUBBLE_TYPE_STARTUP_PAGES:
+      return "SettingsApiBubbleDelegate.StartupPages";
+    case BUBBLE_TYPE_SEARCH_ENGINE:
+      return "SettingsApiBubbleDelegate.SearchEngine";
+  }
+  NOTREACHED();
+  return "";
+}
+
 }  // namespace extensions

chrome/browser/extensions/settings_api_bubble_delegate.h

@@ -39,6 +39,7 @@ class SettingsApiBubbleDelegate
   bool ShouldLimitToEnabledExtensions() const override;
   void LogExtensionCount(size_t count) override;
   void LogAction(ExtensionMessageBubbleController::BubbleAction) override;
+  const char* GetKey() override;
 
  private:
   // The type of settings override this bubble will report on. This can be, for

chrome/browser/extensions/suspicious_extension_bubble_delegate.cc

@@ -26,9 +26,6 @@ namespace {
 // Whether the user has been notified about extension being wiped out.
 const char kWipeoutAcknowledged[] = "ack_wiped";
 
-base::LazyInstance<std::set<Profile*> > g_shown_for_profiles =
-  LAZY_INSTANCE_INITIALIZER;
-
 }  // namespace
 
 namespace extensions {
@@ -132,13 +129,8 @@ void SuspiciousExtensionBubbleDelegate::LogAction(
       action, ExtensionMessageBubbleController::ACTION_BOUNDARY);
 }
 
-std::set<Profile*>* SuspiciousExtensionBubbleDelegate::GetProfileSet() {
-  return g_shown_for_profiles.Pointer();
-}
-
-// static
-void SuspiciousExtensionBubbleDelegate::ClearProfileListForTesting() {
-  g_shown_for_profiles.Get().clear();
+const char* SuspiciousExtensionBubbleDelegate::GetKey() {
+  return "SuspiciousExtensionBubbleDelegate";
 }
 
 }  // namespace extensions

chrome/browser/extensions/suspicious_extension_bubble_delegate.h

@@ -38,11 +38,7 @@ class SuspiciousExtensionBubbleDelegate
   bool ShouldLimitToEnabledExtensions() const override;
   void LogExtensionCount(size_t count) override;
   void LogAction(ExtensionMessageBubbleController::BubbleAction) override;
-  std::set<Profile*>* GetProfileSet() override;
-
-  // Clears the list of profiles the bubble has been shown for. Should only be
-  // used during testing.
-  static void ClearProfileListForTesting();
+  const char* GetKey() override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SuspiciousExtensionBubbleDelegate);

chrome/browser/ui/cocoa/extensions/extension_message_bubble_bridge.mm

@@ -48,9 +48,12 @@ void ExtensionMessageBubbleBridge::OnBubbleShown() {
 
 void ExtensionMessageBubbleBridge::OnBubbleClosed(CloseAction action) {
   switch(action) {
-    case CLOSE_DISMISS:
-      controller_->OnBubbleDismiss();
+    case CLOSE_DISMISS_USER_ACTION:
+    case CLOSE_DISMISS_DEACTIVATION: {
+      bool close_by_deactivate = action == CLOSE_DISMISS_DEACTIVATION;
+      controller_->OnBubbleDismiss(close_by_deactivate);
       break;
+    }
     case CLOSE_EXECUTE:
       controller_->OnBubbleAction();
       break;

chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.mm

@@ -106,7 +106,7 @@ CGFloat kMinWidth = 320.0;
 - (void)windowWillClose:(NSNotification*)notification {
   if (!acknowledged_) {
     delegate_->OnBubbleClosed(
-        ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS);
+        ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_DEACTIVATION);
     acknowledged_ = YES;
   }
   [super windowWillClose:notification];
@@ -327,7 +327,7 @@ CGFloat kMinWidth = 320.0;
   if (learnMoreButton_ && sender == learnMoreButton_) {
     action = ToolbarActionsBarBubbleDelegate::CLOSE_LEARN_MORE;
   } else if (dismissButton_ && sender == dismissButton_) {
-    action = ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS;
+    action = ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_USER_ACTION;
   } else {
     DCHECK_EQ(sender, actionButton_);
     action = ToolbarActionsBarBubbleDelegate::CLOSE_EXECUTE;

chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac_unittest.mm

@@ -117,12 +117,15 @@ void ToolbarActionsBarBubbleMacTest::TestBubbleButton(
     case ToolbarActionsBarBubbleDelegate::CLOSE_EXECUTE:
       button = [bubble actionButton];
       break;
-    case ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS:
+    case ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_USER_ACTION:
       button = [bubble dismissButton];
       break;
     case ToolbarActionsBarBubbleDelegate::CLOSE_LEARN_MORE:
       button = [bubble learnMoreButton];
       break;
+    case ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_DEACTIVATION:
+      NOTREACHED();  // Deactivation is tested below.
+      break;
   }
   ASSERT_TRUE(button);
 
@@ -144,7 +147,7 @@ void ToolbarActionsBarBubbleMacTest::TestBubbleButton(
 TEST_F(ToolbarActionsBarBubbleMacTest, CloseActionAndDismiss) {
   // Test all the possible actions.
   TestBubbleButton(ToolbarActionsBarBubbleDelegate::CLOSE_EXECUTE);
-  TestBubbleButton(ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS);
+  TestBubbleButton(ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_USER_ACTION);
   TestBubbleButton(ToolbarActionsBarBubbleDelegate::CLOSE_LEARN_MORE);
 
   {
@@ -160,7 +163,7 @@ TEST_F(ToolbarActionsBarBubbleMacTest, CloseActionAndDismiss) {
     [bubble close];
     chrome::testing::NSRunLoopRunAllPending();
     ASSERT_TRUE(delegate.close_action());
-    EXPECT_EQ(ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS,
+    EXPECT_EQ(ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_DEACTIVATION,
               *delegate.close_action());
     EXPECT_TRUE([windowObserver windowIsClosing]);
   }

chrome/browser/ui/toolbar/toolbar_actions_bar_bubble_delegate.h

@@ -13,7 +13,8 @@ class ToolbarActionsBarBubbleDelegate {
   enum CloseAction {
     CLOSE_LEARN_MORE,
     CLOSE_EXECUTE,
-    CLOSE_DISMISS
+    CLOSE_DISMISS_USER_ACTION,
+    CLOSE_DISMISS_DEACTIVATION,
   };
 
   virtual ~ToolbarActionsBarBubbleDelegate() {}

chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc

@@ -424,7 +424,7 @@ TEST_F(ToolbarActionsBarRedesignUnitTest, IconSurfacingBubbleAppearance) {
       new ExtensionToolbarIconSurfacingBubbleDelegate(profile()));
   bubble_delegate->OnBubbleShown();
   bubble_delegate->OnBubbleClosed(
-      ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS);
+      ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_USER_ACTION);
   EXPECT_FALSE(
     ExtensionToolbarIconSurfacingBubbleDelegate::ShouldShowForProfile(
         profile()));

chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc

@@ -1229,7 +1229,8 @@ TEST_F(ToolbarActionsModelUnitTest, ToolbarModelHighlightsForToolbarRedesign) {
 
   scoped_ptr<ToolbarActionsBarBubbleDelegate> bubble(
       new ExtensionToolbarIconSurfacingBubbleDelegate(profile()));
-  bubble->OnBubbleClosed(ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS);
+  bubble->OnBubbleClosed(
+      ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_USER_ACTION);
 
   EXPECT_FALSE(toolbar_model->is_highlighting());
   EXPECT_EQ(ToolbarActionsModel::HIGHLIGHT_NONE,

chrome/browser/ui/views/extensions/extension_message_bubble_view.cc

@@ -77,8 +77,10 @@ void ExtensionMessageBubbleView::Show() {
 void ExtensionMessageBubbleView::OnWidgetDestroying(views::Widget* widget) {
   // To catch Esc, we monitor destroy message. Unless the link has been clicked,
   // we assume Dismiss was the action taken.
-  if (!link_clicked_ && !action_taken_)
-    controller_->OnBubbleDismiss();
+  if (!link_clicked_ && !action_taken_) {
+    bool closed_on_deactivation = close_reason() == CloseReason::DEACTIVATION;
+    controller_->OnBubbleDismiss(closed_on_deactivation);
+  }
 }
 
 void ExtensionMessageBubbleView::set_bubble_appearance_wait_time_for_testing(

chrome/browser/ui/views/extensions/extension_toolbar_icon_surfacing_bubble_views.cc

@@ -78,7 +78,11 @@ void ExtensionToolbarIconSurfacingBubble::OnWidgetDestroying(
     views::Widget* widget) {
   BubbleDelegateView::OnWidgetDestroying(widget);
   if (!acknowledged_) {
-    delegate_->OnBubbleClosed(ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS);
+    ToolbarActionsBarBubbleDelegate::CloseAction close_action =
+        close_reason() == CloseReason::DEACTIVATION
+            ? ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_DEACTIVATION
+            : ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_USER_ACTION;
+    delegate_->OnBubbleClosed(close_action);
     acknowledged_ = true;
   }
 }

chrome/browser/ui/views/extensions/extension_toolbar_icon_surfacing_bubble_views_unittest.cc

@@ -81,11 +81,12 @@ TEST_F(ExtensionToolbarIconSurfacingBubbleTest,
     views::test::TestWidgetObserver bubble_observer(bubble_widget);
     EXPECT_FALSE(delegate.close_action());
 
-    // Close the bubble. The delegate should be told it was dismissed.
-    bubble_widget->Close();
+    // Close the bubble by activating another widget. The delegate should be
+    // told it was dismissed.
+    anchor_widget->Activate();
     base::RunLoop().RunUntilIdle();
     ASSERT_TRUE(delegate.close_action());
-    EXPECT_EQ(ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS,
+    EXPECT_EQ(ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_DEACTIVATION,
               *delegate.close_action());
     EXPECT_TRUE(bubble_observer.widget_closed());
   }

ui/views/bubble/bubble_delegate.cc

@@ -51,25 +51,10 @@ Widget* CreateBubbleWidget(BubbleDelegateView* bubble) {
 const char BubbleDelegateView::kViewClassName[] = "BubbleDelegateView";
 
 BubbleDelegateView::BubbleDelegateView()
-    : close_on_esc_(true),
-      close_on_deactivate_(true),
-      anchor_view_storage_id_(ViewStorage::GetInstance()->CreateStorageID()),
-      anchor_widget_(NULL),
-      arrow_(BubbleBorder::TOP_LEFT),
-      shadow_(BubbleBorder::SMALL_SHADOW),
-      color_explicitly_set_(false),
-      margins_(kDefaultMargin, kDefaultMargin, kDefaultMargin, kDefaultMargin),
-      accept_events_(true),
-      border_accepts_events_(true),
-      adjust_if_offscreen_(true),
-      parent_window_(NULL) {
-  AddAccelerator(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
-  UpdateColorsFromTheme(GetNativeTheme());
-}
+    : BubbleDelegateView(nullptr, BubbleBorder::TOP_LEFT) {}
 
-BubbleDelegateView::BubbleDelegateView(
-    View* anchor_view,
-    BubbleBorder::Arrow arrow)
+BubbleDelegateView::BubbleDelegateView(View* anchor_view,
+                                       BubbleBorder::Arrow arrow)
     : close_on_esc_(true),
       close_on_deactivate_(true),
       anchor_view_storage_id_(ViewStorage::GetInstance()->CreateStorageID()),
@@ -81,8 +66,10 @@ BubbleDelegateView::BubbleDelegateView(
       accept_events_(true),
       border_accepts_events_(true),
       adjust_if_offscreen_(true),
-      parent_window_(NULL) {
-  SetAnchorView(anchor_view);
+      parent_window_(NULL),
+      close_reason_(CloseReason::UNKNOWN) {
+  if (anchor_view)
+    SetAnchorView(anchor_view);
   AddAccelerator(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
   UpdateColorsFromTheme(GetNativeTheme());
 }
@@ -151,6 +138,14 @@ const char* BubbleDelegateView::GetClassName() const {
   return kViewClassName;
 }
 
+void BubbleDelegateView::OnWidgetClosing(Widget* widget) {
+  DCHECK(GetBubbleFrameView());
+  if (widget == GetWidget() && close_reason_ == CloseReason::UNKNOWN &&
+      GetBubbleFrameView()->close_button_clicked()) {
+    close_reason_ = CloseReason::CLOSE_BUTTON;
+  }
+}
+
 void BubbleDelegateView::OnWidgetDestroying(Widget* widget) {
   if (anchor_widget() == widget)
     SetAnchorView(NULL);
@@ -175,8 +170,11 @@ void BubbleDelegateView::OnWidgetVisibilityChanged(Widget* widget,
 
 void BubbleDelegateView::OnWidgetActivationChanged(Widget* widget,
                                                    bool active) {
-  if (close_on_deactivate() && widget == GetWidget() && !active)
+  if (close_on_deactivate() && widget == GetWidget() && !active) {
+    if (close_reason_ == CloseReason::UNKNOWN)
+      close_reason_ = CloseReason::DEACTIVATION;
     GetWidget()->Close();
+  }
 }
 
 void BubbleDelegateView::OnWidgetBoundsChanged(Widget* widget,
@@ -221,6 +219,7 @@ bool BubbleDelegateView::AcceleratorPressed(
     const ui::Accelerator& accelerator) {
   if (!close_on_esc() || accelerator.key_code() != ui::VKEY_ESCAPE)
     return false;
+  close_reason_ = CloseReason::ESCAPE;
   GetWidget()->Close();
   return true;
 }

ui/views/bubble/bubble_delegate.h

@@ -28,6 +28,13 @@ class VIEWS_EXPORT BubbleDelegateView : public WidgetDelegateView,
   // Internal class name.
   static const char kViewClassName[];
 
+  enum class CloseReason {
+    DEACTIVATION,
+    ESCAPE,
+    CLOSE_BUTTON,
+    UNKNOWN,
+  };
+
   BubbleDelegateView();
   BubbleDelegateView(View* anchor_view, BubbleBorder::Arrow arrow);
   ~BubbleDelegateView() override;
@@ -44,6 +51,7 @@ class VIEWS_EXPORT BubbleDelegateView : public WidgetDelegateView,
   const char* GetClassName() const override;
 
   // WidgetObserver overrides:
+  void OnWidgetClosing(Widget* widget) override;
   void OnWidgetDestroying(Widget* widget) override;
   void OnWidgetVisibilityChanging(Widget* widget, bool visible) override;
   void OnWidgetVisibilityChanged(Widget* widget, bool visible) override;
@@ -93,6 +101,8 @@ class VIEWS_EXPORT BubbleDelegateView : public WidgetDelegateView,
   bool adjust_if_offscreen() const { return adjust_if_offscreen_; }
   void set_adjust_if_offscreen(bool adjust) { adjust_if_offscreen_ = adjust; }
 
+  CloseReason close_reason() const { return close_reason_; }
+
   // Get the arrow's anchor rect in screen space.
   virtual gfx::Rect GetAnchorRect() const;
 
@@ -192,6 +202,8 @@ class VIEWS_EXPORT BubbleDelegateView : public WidgetDelegateView,
   // Parent native window of the bubble.
   gfx::NativeView parent_window_;
 
+  CloseReason close_reason_;
+
   DISALLOW_COPY_AND_ASSIGN(BubbleDelegateView);
 };
 

ui/views/bubble/bubble_delegate_unittest.cc

@@ -3,8 +3,10 @@
 // found in the LICENSE file.
 
 #include "ui/base/hit_test.h"
+#include "ui/events/event_utils.h"
 #include "ui/views/bubble/bubble_delegate.h"
 #include "ui/views/bubble/bubble_frame_view.h"
+#include "ui/views/controls/button/label_button.h"
 #include "ui/views/test/test_widget_observer.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget.h"
@@ -36,6 +38,10 @@ class TestBubbleDelegateView : public BubbleDelegateView {
   View* GetInitiallyFocusedView() override { return view_; }
   gfx::Size GetPreferredSize() const override { return gfx::Size(200, 200); }
 
+  BubbleFrameView* GetBubbleFrameViewForTest() const {
+    return GetBubbleFrameView();
+  }
+
  private:
   View* view_;
 
@@ -261,4 +267,53 @@ TEST_F(BubbleDelegateTest, NotActivatable) {
   EXPECT_FALSE(bubble_widget->CanActivate());
 }
 
+TEST_F(BubbleDelegateTest, CloseReasons) {
+  {
+    scoped_ptr<Widget> anchor_widget(CreateTestWidget());
+    BubbleDelegateView* bubble_delegate = new BubbleDelegateView(
+        anchor_widget->GetContentsView(), BubbleBorder::NONE);
+    bubble_delegate->set_close_on_deactivate(true);
+    Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+    bubble_widget->Show();
+    anchor_widget->Activate();
+    EXPECT_TRUE(bubble_widget->IsClosed());
+    EXPECT_EQ(BubbleDelegateView::CloseReason::DEACTIVATION,
+              bubble_delegate->close_reason());
+  }
+
+  {
+    scoped_ptr<Widget> anchor_widget(CreateTestWidget());
+    BubbleDelegateView* bubble_delegate = new BubbleDelegateView(
+        anchor_widget->GetContentsView(), BubbleBorder::NONE);
+    bubble_delegate->set_close_on_esc(true);
+    Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+    bubble_widget->Show();
+    // Cast as a test hack to access AcceleratorPressed() (which is protected
+    // in BubbleDelegate).
+    static_cast<View*>(bubble_delegate)
+        ->AcceleratorPressed(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
+    EXPECT_TRUE(bubble_widget->IsClosed());
+    EXPECT_EQ(BubbleDelegateView::CloseReason::ESCAPE,
+              bubble_delegate->close_reason());
+  }
+
+  {
+    scoped_ptr<Widget> anchor_widget(CreateTestWidget());
+    TestBubbleDelegateView* bubble_delegate =
+        new TestBubbleDelegateView(anchor_widget->GetContentsView());
+    Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+    bubble_widget->Show();
+    BubbleFrameView* frame_view = bubble_delegate->GetBubbleFrameViewForTest();
+    LabelButton* close_button = frame_view->close_;
+    ASSERT_TRUE(close_button);
+    frame_view->ButtonPressed(
+        close_button,
+        ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(0, 0), gfx::Point(0, 0),
+                       ui::EventTimeForNow(), ui::EF_NONE, ui::EF_NONE));
+    EXPECT_TRUE(bubble_widget->IsClosed());
+    EXPECT_EQ(BubbleDelegateView::CloseReason::CLOSE_BUTTON,
+              bubble_delegate->close_reason());
+  }
+}
+
 }  // namespace views

ui/views/bubble/bubble_frame_view.cc

@@ -70,7 +70,8 @@ BubbleFrameView::BubbleFrameView(const gfx::Insets& content_margins)
       title_icon_(new views::ImageView()),
       title_(nullptr),
       close_(nullptr),
-      titlebar_extra_view_(nullptr) {
+      titlebar_extra_view_(nullptr),
+      close_button_clicked_(false) {
   AddChildView(title_icon_);
 
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
@@ -321,8 +322,10 @@ void BubbleFrameView::OnNativeThemeChanged(const ui::NativeTheme* theme) {
 }
 
 void BubbleFrameView::ButtonPressed(Button* sender, const ui::Event& event) {
-  if (sender == close_)
+  if (sender == close_) {
+    close_button_clicked_ = true;
     GetWidget()->Close();
+  }
 }
 
 void BubbleFrameView::SetBubbleBorder(scoped_ptr<BubbleBorder> border) {

ui/views/bubble/bubble_frame_view.h

@@ -84,6 +84,8 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView,
                                    gfx::Size client_size,
                                    bool adjust_if_offscreen);
 
+  bool close_button_clicked() const { return close_button_clicked_; }
+
  protected:
   // Returns the available screen bounds if the frame were to show in |rect|.
   virtual gfx::Rect GetAvailableScreenBounds(const gfx::Rect& rect) const;
@@ -93,6 +95,7 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView,
 
  private:
   FRIEND_TEST_ALL_PREFIXES(BubbleFrameViewTest, GetBoundsForClientView);
+  FRIEND_TEST_ALL_PREFIXES(BubbleDelegateTest, CloseReasons);
 
   // Mirrors the bubble's arrow location on the |vertical| or horizontal axis,
   // if the generated window bounds don't fit in the monitor bounds.
@@ -123,6 +126,9 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView,
   // (x) close button.
   View* titlebar_extra_view_;
 
+  // Whether the close button was clicked.
+  bool close_button_clicked_;
+
   DISALLOW_COPY_AND_ASSIGN(BubbleFrameView);
 };