summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorrdevlin.cronin <rdevlin.cronin@chromium.org>2015-11-20 09:20:35 -0800
committerCommit bot <commit-bot@chromium.org>2015-11-20 17:21:08 +0000
commit486b20e71bb481cf6ae6491a998aa252081a8833 (patch)
tree35d1d0ff76938b895d5129159db9fcc47d1f10e4
parent4dee2a9aaab948a2603c88be06e3de528bdb35bd (diff)
downloadchromium_src-486b20e71bb481cf6ae6491a998aa252081a8833.zip
chromium_src-486b20e71bb481cf6ae6491a998aa252081a8833.tar.gz
chromium_src-486b20e71bb481cf6ae6491a998aa252081a8833.tar.bz2
[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}
-rw-r--r--chrome/browser/extensions/dev_mode_bubble_delegate.cc16
-rw-r--r--chrome/browser/extensions/dev_mode_bubble_delegate.h5
-rw-r--r--chrome/browser/extensions/extension_message_bubble_controller.cc56
-rw-r--r--chrome/browser/extensions/extension_message_bubble_controller.h33
-rw-r--r--chrome/browser/extensions/extension_message_bubble_controller_unittest.cc108
-rw-r--r--chrome/browser/extensions/ntp_overridden_bubble_delegate.cc4
-rw-r--r--chrome/browser/extensions/ntp_overridden_bubble_delegate.h1
-rw-r--r--chrome/browser/extensions/proxy_overridden_bubble_delegate.cc4
-rw-r--r--chrome/browser/extensions/proxy_overridden_bubble_delegate.h1
-rw-r--r--chrome/browser/extensions/settings_api_bubble_delegate.cc13
-rw-r--r--chrome/browser/extensions/settings_api_bubble_delegate.h1
-rw-r--r--chrome/browser/extensions/suspicious_extension_bubble_delegate.cc12
-rw-r--r--chrome/browser/extensions/suspicious_extension_bubble_delegate.h6
-rw-r--r--chrome/browser/ui/cocoa/extensions/extension_message_bubble_bridge.mm7
-rw-r--r--chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.mm4
-rw-r--r--chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac_unittest.mm9
-rw-r--r--chrome/browser/ui/toolbar/toolbar_actions_bar_bubble_delegate.h3
-rw-r--r--chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc2
-rw-r--r--chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc3
-rw-r--r--chrome/browser/ui/views/extensions/extension_message_bubble_view.cc6
-rw-r--r--chrome/browser/ui/views/extensions/extension_toolbar_icon_surfacing_bubble_views.cc6
-rw-r--r--chrome/browser/ui/views/extensions/extension_toolbar_icon_surfacing_bubble_views_unittest.cc7
-rw-r--r--ui/views/bubble/bubble_delegate.cc41
-rw-r--r--ui/views/bubble/bubble_delegate.h12
-rw-r--r--ui/views/bubble/bubble_delegate_unittest.cc55
-rw-r--r--ui/views/bubble/bubble_frame_view.cc7
-rw-r--r--ui/views/bubble/bubble_frame_view.h6
27 files changed, 319 insertions, 109 deletions
diff --git a/chrome/browser/extensions/dev_mode_bubble_delegate.cc b/chrome/browser/extensions/dev_mode_bubble_delegate.cc
index 5f723de..1ac48c9 100644
--- a/chrome/browser/extensions/dev_mode_bubble_delegate.cc
+++ b/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
diff --git a/chrome/browser/extensions/dev_mode_bubble_delegate.h b/chrome/browser/extensions/dev_mode_bubble_delegate.h
index b8bdaab..7412aa7 100644
--- a/chrome/browser/extensions/dev_mode_bubble_delegate.h
+++ b/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);
diff --git a/chrome/browser/extensions/extension_message_bubble_controller.cc b/chrome/browser/extensions/extension_message_bubble_controller.cc
index 8dd0160..7c9d2ce 100644
--- a/chrome/browser/extensions/extension_message_bubble_controller.cc
+++ b/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
diff --git a/chrome/browser/extensions/extension_message_bubble_controller.h b/chrome/browser/extensions/extension_message_bubble_controller.h
index 40e5b81..c3059c5 100644
--- a/chrome/browser/extensions/extension_message_bubble_controller.h
+++ b/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_;
diff --git a/chrome/browser/extensions/extension_message_bubble_controller_unittest.cc b/chrome/browser/extensions/extension_message_bubble_controller_unittest.cc
index 129990f..4af1301 100644
--- a/chrome/browser/extensions/extension_message_bubble_controller_unittest.cc
+++ b/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());
diff --git a/chrome/browser/extensions/ntp_overridden_bubble_delegate.cc b/chrome/browser/extensions/ntp_overridden_bubble_delegate.cc
index f76ebdc..c2baee0 100644
--- a/chrome/browser/extensions/ntp_overridden_bubble_delegate.cc
+++ b/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
diff --git a/chrome/browser/extensions/ntp_overridden_bubble_delegate.h b/chrome/browser/extensions/ntp_overridden_bubble_delegate.h
index 80026ab..357d09c 100644
--- a/chrome/browser/extensions/ntp_overridden_bubble_delegate.h
+++ b/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.
diff --git a/chrome/browser/extensions/proxy_overridden_bubble_delegate.cc b/chrome/browser/extensions/proxy_overridden_bubble_delegate.cc
index 59c7dd4..b2b3db5 100644
--- a/chrome/browser/extensions/proxy_overridden_bubble_delegate.cc
+++ b/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
diff --git a/chrome/browser/extensions/proxy_overridden_bubble_delegate.h b/chrome/browser/extensions/proxy_overridden_bubble_delegate.h
index cbd851d..dedc843 100644
--- a/chrome/browser/extensions/proxy_overridden_bubble_delegate.h
+++ b/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.
diff --git a/chrome/browser/extensions/settings_api_bubble_delegate.cc b/chrome/browser/extensions/settings_api_bubble_delegate.cc
index 2641883..26ac3da 100644
--- a/chrome/browser/extensions/settings_api_bubble_delegate.cc
+++ b/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
diff --git a/chrome/browser/extensions/settings_api_bubble_delegate.h b/chrome/browser/extensions/settings_api_bubble_delegate.h
index dde081d..aea8350 100644
--- a/chrome/browser/extensions/settings_api_bubble_delegate.h
+++ b/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
diff --git a/chrome/browser/extensions/suspicious_extension_bubble_delegate.cc b/chrome/browser/extensions/suspicious_extension_bubble_delegate.cc
index c4b95ed..33381d7 100644
--- a/chrome/browser/extensions/suspicious_extension_bubble_delegate.cc
+++ b/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
diff --git a/chrome/browser/extensions/suspicious_extension_bubble_delegate.h b/chrome/browser/extensions/suspicious_extension_bubble_delegate.h
index 9ecad26..0d1406e 100644
--- a/chrome/browser/extensions/suspicious_extension_bubble_delegate.h
+++ b/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);
diff --git a/chrome/browser/ui/cocoa/extensions/extension_message_bubble_bridge.mm b/chrome/browser/ui/cocoa/extensions/extension_message_bubble_bridge.mm
index 86f1361..28b77e5e 100644
--- a/chrome/browser/ui/cocoa/extensions/extension_message_bubble_bridge.mm
+++ b/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;
diff --git a/chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.mm b/chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.mm
index 5e01460..ccba1d8 100644
--- a/chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.mm
+++ b/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;
diff --git a/chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac_unittest.mm b/chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac_unittest.mm
index 6c266f4..4f9a443 100644
--- a/chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac_unittest.mm
+++ b/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]);
}
diff --git a/chrome/browser/ui/toolbar/toolbar_actions_bar_bubble_delegate.h b/chrome/browser/ui/toolbar/toolbar_actions_bar_bubble_delegate.h
index 2fb9bc8..508371e 100644
--- a/chrome/browser/ui/toolbar/toolbar_actions_bar_bubble_delegate.h
+++ b/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() {}
diff --git a/chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc b/chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc
index 7e79337..b0ce77b 100644
--- a/chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc
+++ b/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()));
diff --git a/chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc b/chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc
index 787e5da..c0e7d26 100644
--- a/chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc
+++ b/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,
diff --git a/chrome/browser/ui/views/extensions/extension_message_bubble_view.cc b/chrome/browser/ui/views/extensions/extension_message_bubble_view.cc
index c30ba3b..fc528bc 100644
--- a/chrome/browser/ui/views/extensions/extension_message_bubble_view.cc
+++ b/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(
diff --git a/chrome/browser/ui/views/extensions/extension_toolbar_icon_surfacing_bubble_views.cc b/chrome/browser/ui/views/extensions/extension_toolbar_icon_surfacing_bubble_views.cc
index c0ddf62..24fefe36 100644
--- a/chrome/browser/ui/views/extensions/extension_toolbar_icon_surfacing_bubble_views.cc
+++ b/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;
}
}
diff --git a/chrome/browser/ui/views/extensions/extension_toolbar_icon_surfacing_bubble_views_unittest.cc b/chrome/browser/ui/views/extensions/extension_toolbar_icon_surfacing_bubble_views_unittest.cc
index ab7cbd2..ac8a6da 100644
--- a/chrome/browser/ui/views/extensions/extension_toolbar_icon_surfacing_bubble_views_unittest.cc
+++ b/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());
}
diff --git a/ui/views/bubble/bubble_delegate.cc b/ui/views/bubble/bubble_delegate.cc
index 184a8ac..11dc4ff 100644
--- a/ui/views/bubble/bubble_delegate.cc
+++ b/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;
}
diff --git a/ui/views/bubble/bubble_delegate.h b/ui/views/bubble/bubble_delegate.h
index 3bc73f8..1bd9634 100644
--- a/ui/views/bubble/bubble_delegate.h
+++ b/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);
};
diff --git a/ui/views/bubble/bubble_delegate_unittest.cc b/ui/views/bubble/bubble_delegate_unittest.cc
index 16f14c5..1ec4e39 100644
--- a/ui/views/bubble/bubble_delegate_unittest.cc
+++ b/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
diff --git a/ui/views/bubble/bubble_frame_view.cc b/ui/views/bubble/bubble_frame_view.cc
index 88025b0..c9fcd40 100644
--- a/ui/views/bubble/bubble_frame_view.cc
+++ b/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) {
diff --git a/ui/views/bubble/bubble_frame_view.h b/ui/views/bubble/bubble_frame_view.h
index b21215c..28750c3 100644
--- a/ui/views/bubble/bubble_frame_view.h
+++ b/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);
};