summaryrefslogtreecommitdiffstats
path: root/chrome/browser/protector
diff options
context:
space:
mode:
authorivankr@chromium.org <ivankr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-03-13 12:45:00 +0000
committerivankr@chromium.org <ivankr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-03-13 12:45:00 +0000
commit0cf77af7a54e7a817171c249a808b404b36ba4a4 (patch)
tree9eb43a09bc7d3dabdf46cecda716fc56f830b62f /chrome/browser/protector
parentdaa3b8227052fe93d204f5c46cf9dcd66b62b4a8 (diff)
downloadchromium_src-0cf77af7a54e7a817171c249a808b404b36ba4a4.zip
chromium_src-0cf77af7a54e7a817171c249a808b404b36ba4a4.tar.gz
chromium_src-0cf77af7a54e7a817171c249a808b404b36ba4a4.tar.bz2
[protector] Added changes do not hide the currently shown change, if any.
BUG=114288 TEST=ProtectorServiceTest.* Review URL: http://codereview.chromium.org/9664063 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@126381 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/protector')
-rw-r--r--chrome/browser/protector/protector_service.cc20
-rw-r--r--chrome/browser/protector/protector_service.h4
-rw-r--r--chrome/browser/protector/protector_service_browsertest.cc62
-rw-r--r--chrome/browser/protector/settings_change_global_error.cc131
-rw-r--r--chrome/browser/protector/settings_change_global_error.h22
5 files changed, 151 insertions, 88 deletions
diff --git a/chrome/browser/protector/protector_service.cc b/chrome/browser/protector/protector_service.cc
index 488fc6a..93e353d 100644
--- a/chrome/browser/protector/protector_service.cc
+++ b/chrome/browser/protector/protector_service.cc
@@ -20,7 +20,8 @@
namespace protector {
ProtectorService::ProtectorService(Profile* profile)
- : profile_(profile) {
+ : profile_(profile),
+ has_active_change_(false) {
}
ProtectorService::~ProtectorService() {
@@ -40,7 +41,9 @@ void ProtectorService::ShowChange(BaseSettingChange* change) {
new SettingsChangeGlobalError(change, this);
new_item.error.reset(error);
items_.push_back(new_item);
- error->ShowForProfile(profile_);
+ // Do not show the bubble immediately if another one is active.
+ error->AddToProfile(profile_, !has_active_change_);
+ has_active_change_ = true;
}
bool ProtectorService::IsShowingChange() const {
@@ -80,12 +83,14 @@ void ProtectorService::OnApplyChange(SettingsChangeGlobalError* error,
Browser* browser) {
DVLOG(1) << "Apply change";
error->change()->Apply(browser);
+ has_active_change_ = false;
}
void ProtectorService::OnDiscardChange(SettingsChangeGlobalError* error,
Browser* browser) {
DVLOG(1) << "Discard change";
error->change()->Discard(browser);
+ has_active_change_ = false;
}
void ProtectorService::OnDecisionTimeout(SettingsChangeGlobalError* error) {
@@ -98,6 +103,17 @@ void ProtectorService::OnRemovedFromProfile(SettingsChangeGlobalError* error) {
std::find_if(items_.begin(), items_.end(), MatchItemByError(error));
DCHECK(item != items_.end());
items_.erase(item);
+
+ if (!has_active_change_) {
+ // If there are changes that haven't been shown yet, show the first one.
+ for (item = items_.begin(); item != items_.end(); ++item) {
+ if (!item->error->HasShownBubbleView()) {
+ item->error->ShowBubble();
+ has_active_change_ = true;
+ return;
+ }
+ }
+ }
}
BaseSettingChange* ProtectorService::GetLastChange() {
diff --git a/chrome/browser/protector/protector_service.h b/chrome/browser/protector/protector_service.h
index efb7843..942e567 100644
--- a/chrome/browser/protector/protector_service.h
+++ b/chrome/browser/protector/protector_service.h
@@ -113,6 +113,10 @@ class ProtectorService : public ProfileKeyedService,
// Profile which settings we are protecting.
Profile* profile_;
+ // True if there is a change that has been shown and not yet accepted or
+ // discarded by user.
+ bool has_active_change_;
+
DISALLOW_COPY_AND_ASSIGN(ProtectorService);
};
diff --git a/chrome/browser/protector/protector_service_browsertest.cc b/chrome/browser/protector/protector_service_browsertest.cc
index 5e41dbf..17912ff 100644
--- a/chrome/browser/protector/protector_service_browsertest.cc
+++ b/chrome/browser/protector/protector_service_browsertest.cc
@@ -154,7 +154,6 @@ IN_PROC_BROWSER_TEST_F(ProtectorServiceTest, BubbleClosedInsideApply) {
ASSERT_TRUE(bubble_view);
EXPECT_CALL(*mock_change_, Apply(browser())).WillOnce(InvokeWithoutArgs(
bubble_view, &GlobalErrorBubbleViewBase::CloseBubbleView));
-
// Pressing Cancel applies the change.
error->BubbleViewCancelButtonPressed(browser());
ui_test_utils::RunAllPendingInMessageLoop();
@@ -236,6 +235,11 @@ IN_PROC_BROWSER_TEST_F(ProtectorServiceTest,
ui_test_utils::RunAllPendingInMessageLoop();
EXPECT_TRUE(IsGlobalErrorActive(mock_change_));
+ // The first bubble view has been displayed.
+ GlobalError* error = GetGlobalError(mock_change_);
+ ASSERT_TRUE(error);
+ ASSERT_TRUE(error->HasShownBubbleView());
+
// ProtectService will own this change instance as well.
MockSettingChange* mock_change2 = new NiceMock<MockSettingChange>();
// Show the second change.
@@ -246,27 +250,73 @@ IN_PROC_BROWSER_TEST_F(ProtectorServiceTest,
EXPECT_TRUE(IsGlobalErrorActive(mock_change_));
EXPECT_TRUE(IsGlobalErrorActive(mock_change2));
+ // The second bubble view hasn't been displayed because the first is still
+ // shown.
+ GlobalError* error2 = GetGlobalError(mock_change2);
+ ASSERT_TRUE(error2);
+ EXPECT_FALSE(error2->HasShownBubbleView());
+
// Apply the first change, mimicking a button click; the second should still
// be active.
EXPECT_CALL(*mock_change_, Apply(browser()));
+ error->BubbleViewCancelButtonPressed(browser());
+ error->GetBubbleView()->CloseBubbleView();
+ ui_test_utils::RunAllPendingInMessageLoop();
+ EXPECT_FALSE(IsGlobalErrorActive(mock_change_));
+ EXPECT_TRUE(IsGlobalErrorActive(mock_change2));
+
+ // Now the second bubble view should be shown.
+ ASSERT_TRUE(error2->HasShownBubbleView());
+
+ // Finally apply the second change.
+ EXPECT_CALL(*mock_change2, Apply(browser()));
+ error2->BubbleViewCancelButtonPressed(browser());
+ error2->GetBubbleView()->CloseBubbleView();
+ ui_test_utils::RunAllPendingInMessageLoop();
+ EXPECT_FALSE(IsGlobalErrorActive(mock_change_));
+ EXPECT_FALSE(IsGlobalErrorActive(mock_change2));
+}
+
+IN_PROC_BROWSER_TEST_F(ProtectorServiceTest,
+ ShowMultipleChangesAndApplyManuallyBeforeOther) {
+ // Show the first change.
+ EXPECT_CALL(*mock_change_, MockInit(browser()->profile())).
+ WillOnce(Return(true));
+ protector_service_->ShowChange(mock_change_);
+ ui_test_utils::RunAllPendingInMessageLoop();
+ EXPECT_TRUE(IsGlobalErrorActive(mock_change_));
+
+ // The first bubble view has been displayed.
GlobalError* error = GetGlobalError(mock_change_);
ASSERT_TRUE(error);
- error->ShowBubbleView(browser());
+ ASSERT_TRUE(error->HasShownBubbleView());
+
+ // Apply the first change, mimicking a button click.
+ EXPECT_CALL(*mock_change_, Apply(browser()));
error->BubbleViewCancelButtonPressed(browser());
error->GetBubbleView()->CloseBubbleView();
ui_test_utils::RunAllPendingInMessageLoop();
EXPECT_FALSE(IsGlobalErrorActive(mock_change_));
+
+ // ProtectService will own this change instance as well.
+ MockSettingChange* mock_change2 = new NiceMock<MockSettingChange>();
+ // Show the second change.
+ EXPECT_CALL(*mock_change2, MockInit(browser()->profile())).
+ WillOnce(Return(true));
+ protector_service_->ShowChange(mock_change2);
+ ui_test_utils::RunAllPendingInMessageLoop();
EXPECT_TRUE(IsGlobalErrorActive(mock_change2));
+ // The second bubble view has been displayed.
+ GlobalError* error2 = GetGlobalError(mock_change2);
+ ASSERT_TRUE(error2);
+ ASSERT_TRUE(error2->HasShownBubbleView());
+
// Finally apply the second change.
EXPECT_CALL(*mock_change2, Apply(browser()));
- GlobalError* error2 = GetGlobalError(mock_change2);
- ASSERT_TRUE(error);
- error2->ShowBubbleView(browser());
error2->BubbleViewCancelButtonPressed(browser());
error2->GetBubbleView()->CloseBubbleView();
ui_test_utils::RunAllPendingInMessageLoop();
- EXPECT_FALSE(IsGlobalErrorActive(mock_change_));
EXPECT_FALSE(IsGlobalErrorActive(mock_change2));
}
diff --git a/chrome/browser/protector/settings_change_global_error.cc b/chrome/browser/protector/settings_change_global_error.cc
index cfb1d69..a51103b 100644
--- a/chrome/browser/protector/settings_change_global_error.cc
+++ b/chrome/browser/protector/settings_change_global_error.cc
@@ -67,6 +67,46 @@ SettingsChangeGlobalError::~SettingsChangeGlobalError() {
menu_ids.Get().reset(menu_id_ - IDC_SHOW_SETTINGS_CHANGE_FIRST);
}
+void SettingsChangeGlobalError::AddToProfile(Profile* profile,
+ bool show_bubble) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ profile_ = profile;
+ GlobalErrorServiceFactory::GetForProfile(profile_)->AddGlobalError(this);
+ BrowserList::AddObserver(this);
+ if (show_bubble) {
+ ShowBubble();
+ } else {
+ // Start inactivity timer.
+ BrowserThread::PostDelayedTask(
+ BrowserThread::UI, FROM_HERE,
+ base::Bind(&SettingsChangeGlobalError::OnInactiveTimeout,
+ weak_factory_.GetWeakPtr()),
+ base::TimeDelta::FromMilliseconds(kMenuItemDisplayPeriodMs));
+ }
+}
+
+void SettingsChangeGlobalError::RemoveFromProfile() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ if (profile_) {
+ GlobalErrorServiceFactory::GetForProfile(profile_)->RemoveGlobalError(this);
+ profile_ = NULL;
+ }
+ BrowserList::RemoveObserver(this);
+ // This will delete |this|.
+ delegate_->OnRemovedFromProfile(this);
+}
+
+void SettingsChangeGlobalError::ShowBubble() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DCHECK(profile_);
+ Browser* browser = BrowserList::FindTabbedBrowser(
+ profile_,
+ // match incognito
+ true);
+ if (browser)
+ ShowBubbleInBrowser(browser);
+}
+
bool SettingsChangeGlobalError::HasBadge() {
return true;
}
@@ -92,7 +132,7 @@ int SettingsChangeGlobalError::MenuItemIconResourceID() {
}
void SettingsChangeGlobalError::ExecuteMenuItem(Browser* browser) {
- ShowInBrowser(browser);
+ ShowBubbleInBrowser(browser);
}
bool SettingsChangeGlobalError::HasBubbleView() {
@@ -124,45 +164,6 @@ string16 SettingsChangeGlobalError::GetBubbleViewCancelButtonLabel() {
return change_->GetApplyButtonText();
}
-void SettingsChangeGlobalError::BubbleViewAcceptButtonPressed(
- Browser* browser) {
- closed_by_button_ = true;
- delegate_->OnDiscardChange(this, browser);
-}
-
-void SettingsChangeGlobalError::BubbleViewCancelButtonPressed(
- Browser* browser) {
- closed_by_button_ = true;
- delegate_->OnApplyChange(this, browser);
-}
-
-void SettingsChangeGlobalError::OnBrowserSetLastActive(
- const Browser* browser) {
- if (show_on_browser_activation_ && browser && browser->is_type_tabbed()) {
- // A tabbed browser window got activated, show the error bubble again.
- // Calling Show() immediately from here does not always work because the
- // old browser window may still have focus.
- // Multiple posted Show() calls are fine since the first successful one
- // will invalidate all the weak pointers.
- // Note that Show() will display the bubble in the last active browser
- // (which may not be |browser| at the moment Show() is executed).
- BrowserThread::PostTask(
- BrowserThread::UI, FROM_HERE,
- base::Bind(&SettingsChangeGlobalError::Show,
- weak_factory_.GetWeakPtr()));
- }
-}
-
-void SettingsChangeGlobalError::RemoveFromProfile() {
- if (profile_) {
- GlobalErrorServiceFactory::GetForProfile(profile_)->RemoveGlobalError(this);
- profile_ = NULL;
- }
- BrowserList::RemoveObserver(this);
- // This will delete |this|.
- delegate_->OnRemovedFromProfile(this);
-}
-
void SettingsChangeGlobalError::OnBubbleViewDidClose(Browser* browser) {
if (!closed_by_button_) {
BrowserThread::PostDelayedTask(
@@ -185,38 +186,36 @@ void SettingsChangeGlobalError::OnBubbleViewDidClose(Browser* browser) {
}
}
-void SettingsChangeGlobalError::ShowForProfile(Profile* profile) {
- if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
- AddToProfile(profile);
- } else {
- BrowserThread::PostTask(
- BrowserThread::UI, FROM_HERE,
- base::Bind(&SettingsChangeGlobalError::AddToProfile,
- base::Unretained(this),
- profile));
- }
+void SettingsChangeGlobalError::BubbleViewAcceptButtonPressed(
+ Browser* browser) {
+ closed_by_button_ = true;
+ delegate_->OnDiscardChange(this, browser);
}
-void SettingsChangeGlobalError::AddToProfile(Profile* profile) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- profile_ = profile;
- GlobalErrorServiceFactory::GetForProfile(profile_)->AddGlobalError(this);
- BrowserList::AddObserver(this);
- Show();
+void SettingsChangeGlobalError::BubbleViewCancelButtonPressed(
+ Browser* browser) {
+ closed_by_button_ = true;
+ delegate_->OnApplyChange(this, browser);
}
-void SettingsChangeGlobalError::Show() {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- DCHECK(profile_);
- Browser* browser = BrowserList::FindTabbedBrowser(
- profile_,
- // match incognito
- true);
- if (browser)
- ShowInBrowser(browser);
+void SettingsChangeGlobalError::OnBrowserSetLastActive(
+ const Browser* browser) {
+ if (show_on_browser_activation_ && browser && browser->is_type_tabbed()) {
+ // A tabbed browser window got activated, show the error bubble again.
+ // Calling ShowBubble() immediately from here does not always work because
+ // the old browser window may still have focus.
+ // Multiple posted ShowBubble() calls are fine since the first successful
+ // one will invalidate all the weak pointers.
+ // Note that ShowBubble() will display the bubble in the last active browser
+ // (which may not be |browser| at the moment ShowBubble() is executed).
+ BrowserThread::PostTask(
+ BrowserThread::UI, FROM_HERE,
+ base::Bind(&SettingsChangeGlobalError::ShowBubble,
+ weak_factory_.GetWeakPtr()));
+ }
}
-void SettingsChangeGlobalError::ShowInBrowser(Browser* browser) {
+void SettingsChangeGlobalError::ShowBubbleInBrowser(Browser* browser) {
show_on_browser_activation_ = false;
// Cancel any previously posted tasks so that the global error
// does not get removed on timeout while still showing the bubble.
diff --git a/chrome/browser/protector/settings_change_global_error.h b/chrome/browser/protector/settings_change_global_error.h
index 1c89356..a396df9 100644
--- a/chrome/browser/protector/settings_change_global_error.h
+++ b/chrome/browser/protector/settings_change_global_error.h
@@ -34,13 +34,16 @@ class SettingsChangeGlobalError : public GlobalError,
SettingsChangeGlobalErrorDelegate* delegate);
virtual ~SettingsChangeGlobalError();
- // Displays a global error bubble for the given browser profile.
- // Can be called from any thread.
- void ShowForProfile(Profile* profile);
+ // Adds a global error to the given browser profile and shows a bubble
+ // immediately if |show_bubble| is |true|.
+ void AddToProfile(Profile* profile, bool show_bubble);
// Removes global error from its profile.
void RemoveFromProfile();
+ // Displays the bubble in the last active tabbed browser.
+ void ShowBubble();
+
// Returns the change instance to which this error refers.
BaseSettingChange* change() { return change_; }
@@ -68,17 +71,8 @@ class SettingsChangeGlobalError : public GlobalError,
virtual void OnBrowserRemoved(const Browser* browser) OVERRIDE {}
virtual void OnBrowserSetLastActive(const Browser* browser) OVERRIDE;
- // Helper called on the UI thread to add this global error to the default
- // profile (stored in |profile_|).
- void AddToProfile(Profile* profile);
-
- // Displays the bubble in the last active tabbed browser. Must be called
- // on the UI thread.
- void Show();
-
- // Displays the bubble in |browser|'s window. Must be called
- // on the UI thread.
- void ShowInBrowser(Browser* browser);
+ // Displays the bubble in |browser|'s window.
+ void ShowBubbleInBrowser(Browser* browser);
// Called when the wrench menu item has been displayed for enough time
// without user interaction.
generated by cgit v1.1 at 2025-03-04 07:59:40 +0000