Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3875)

Unified Diff: chrome/browser/ui/website_settings/permission_bubble_manager.cc

Issue 150103013: Add basic coalescing behavior to the permission bubble manager. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/website_settings/permission_bubble_manager.cc
diff --git a/chrome/browser/ui/website_settings/permission_bubble_manager.cc b/chrome/browser/ui/website_settings/permission_bubble_manager.cc
index 2382613605e0c54a58fad7d316a9c5663c388a8e..faa3780c47593d4414893c0d3b66459621398c18 100644
--- a/chrome/browser/ui/website_settings/permission_bubble_manager.cc
+++ b/chrome/browser/ui/website_settings/permission_bubble_manager.cc
@@ -10,6 +10,12 @@
DEFINE_WEB_CONTENTS_USER_DATA_KEY(PermissionBubbleManager);
+namespace {
+// This is how many ms to wait to see if there's another permission request
Greg Billock 2014/02/07 20:14:24 add a blank line around namespace lines
leng 2014/02/08 00:11:18 Done.
+// we should coalesce.
+const int kPermissionsCoalesceIntervalMs = 400;
+}
+
// static
bool PermissionBubbleManager::Enabled() {
return CommandLine::ForCurrentProcess()->HasSwitch(
@@ -24,16 +30,22 @@ void PermissionBubbleManager::AddRequest(PermissionBubbleRequest* request) {
return;
}
+ if (bubble_showing_) {
+ for (di = queued_requests_.begin(); di != queued_requests_.end(); di++) {
+ if (*di == request)
+ return;
+ }
+ queued_requests_.push_back(request);
+ return;
+ }
+
requests_.push_back(request);
// TODO(gbillock): do we need to make default state a request property?
accept_state_.push_back(false);
- // TODO(gbillock): need significantly more complex logic here to deal
- // with various states of the manager.
-
- if (view_ && !bubble_showing_) {
- view_->Show(requests_, accept_state_, customization_mode_);
- bubble_showing_ = true;
+ // Start the timer when there is both a view and a request.
+ if (view_ && !timer_->IsRunning()) {
+ timer_->Reset();
}
}
@@ -54,9 +66,9 @@ void PermissionBubbleManager::SetView(PermissionBubbleView* view) {
view_->SetDelegate(this);
view_->Show(requests_, accept_state_, customization_mode_);
} else if (!requests_.empty()) {
- bubble_showing_ = true;
- view_->SetDelegate(this);
- view_->Show(requests_, accept_state_, customization_mode_);
+ if (!timer_->IsRunning()) {
+ timer_->Reset();
+ }
} else {
view_->Hide();
return;
@@ -69,6 +81,11 @@ PermissionBubbleManager::PermissionBubbleManager(
bubble_showing_(false),
view_(NULL),
customization_mode_(false) {
+ timer_.reset(new base::Timer(FROM_HERE,
+ base::TimeDelta::FromMilliseconds(kPermissionsCoalesceIntervalMs),
+ base::Bind(&PermissionBubbleManager::OnTimerExpired,
+ base::Unretained(this)),
+ false));
}
PermissionBubbleManager::~PermissionBubbleManager() {}
@@ -122,11 +139,34 @@ void PermissionBubbleManager::Closing() {
FinalizeBubble();
}
+void PermissionBubbleManager::OnTimerExpired() {
+ if (view_ && !bubble_showing_ && requests_.size()) {
+ view_->SetDelegate(this);
+ view_->Show(requests_, accept_state_, customization_mode_);
+ bubble_showing_ = true;
+ }
+}
+
void PermissionBubbleManager::FinalizeBubble() {
std::vector<PermissionBubbleRequest*>::iterator di;
for (di = requests_.begin(); di != requests_.end(); di++)
(*di)->RequestFinished();
requests_.clear();
accept_state_.clear();
+ bubble_showing_ = false;
+ if (queued_requests_.size()) {
+ requests_ = queued_requests_;
Greg Billock 2014/02/07 20:14:24 Need to reset the accept_state_ here, too. (Set th
leng 2014/02/08 00:11:18 Done. I also changed the variable name to be plura
+ queued_requests_.clear();
+ // TODO(leng): Explore other options of showing the next bubble. The
+ // advantage of this is that it uses the same code path as the first bubble.
+ timer_->Reset();
+ }
}
+void PermissionBubbleManager::SetCoalesceIntervalForTesting(int interval_ms) {
+ timer_.reset(new base::Timer(FROM_HERE,
+ base::TimeDelta::FromMilliseconds(interval_ms),
+ base::Bind(&PermissionBubbleManager::OnTimerExpired,
+ base::Unretained(this)),
+ false));
+}

Powered by Google App Engine
This is Rietveld 408576698