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

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

Issue 243543003: [WebsiteSettings] Update permission bubble manager policy (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: weak ptr Created 6 years, 8 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 8b3a88e173d05ca27b3723554c8ec9d1a4a99571..670a864ae63054b78cdea6fc1506fbcb26a35060 100644
--- a/chrome/browser/ui/website_settings/permission_bubble_manager.cc
+++ b/chrome/browser/ui/website_settings/permission_bubble_manager.cc
@@ -7,17 +7,10 @@
#include "base/command_line.h"
#include "chrome/browser/ui/website_settings/permission_bubble_request.h"
#include "chrome/common/chrome_switches.h"
+#include "content/public/browser/browser_thread.h"
DEFINE_WEB_CONTENTS_USER_DATA_KEY(PermissionBubbleManager);
-namespace {
-
-// This is how many ms to wait to see if there's another permission request
-// we should coalesce.
-const int kPermissionsCoalesceIntervalMs = 400;
-
-}
-
// static
bool PermissionBubbleManager::Enabled() {
return CommandLine::ForCurrentProcess()->HasSwitch(
@@ -29,12 +22,9 @@ PermissionBubbleManager::PermissionBubbleManager(
: content::WebContentsObserver(web_contents),
bubble_showing_(false),
view_(NULL),
- customization_mode_(false) {
- timer_.reset(new base::Timer(FROM_HERE,
- base::TimeDelta::FromMilliseconds(kPermissionsCoalesceIntervalMs),
- base::Bind(&PermissionBubbleManager::ShowBubble, base::Unretained(this)),
- false));
-}
+ request_url_has_loaded_(false),
+ customization_mode_(false),
+ weak_factory_(this) {}
PermissionBubbleManager::~PermissionBubbleManager() {
if (view_ != NULL)
@@ -97,9 +87,8 @@ void PermissionBubbleManager::AddRequest(PermissionBubbleRequest* request) {
// TODO(gbillock): do we need to make default state a request property?
accept_states_.push_back(true);
- // Start the timer when there is both a view and a request.
- if (view_ && !timer_->IsRunning())
- timer_->Reset();
+ if (request->HasUserGesture())
+ ShowBubble();
}
void PermissionBubbleManager::CancelRequest(PermissionBubbleRequest* request) {
@@ -123,22 +112,22 @@ void PermissionBubbleManager::SetView(PermissionBubbleView* view) {
else
return;
- // Even if there are requests queued up, add a short delay before the bubble
- // appears.
- if (!requests_.empty() && !timer_->IsRunning())
- timer_->Reset();
- else
- view_->Hide();
+ ShowBubble();
}
-void PermissionBubbleManager::DidFinishLoad(
- int64 frame_id,
- const GURL& validated_url,
- bool is_main_frame,
- content::RenderViewHost* render_view_host) {
- // Allows extra time for additional requests to coalesce.
- if (timer_->IsRunning())
- timer_->Reset();
+void PermissionBubbleManager::DocumentOnLoadCompletedInMainFrame(
+ int32 page_id) {
+ request_url_has_loaded_ = true;
+ // This is scheduled because while all calls to the browser have been
+ // issued at DOMContentLoaded, they may be bouncing around in scheduled
+ // callbacks finding the UI thread still. This makes sure we allow those
+ // scheduled calls to AddRequest to complete before we show the page-load
+ // permissions bubble.
+ // TODO(gbillock): make this bind safe with a weak ptr.
+ content::BrowserThread::PostTask(
+ content::BrowserThread::UI, FROM_HERE,
+ base::Bind(&PermissionBubbleManager::ShowBubble,
+ weak_factory_.GetWeakPtr()));
}
void PermissionBubbleManager::NavigationEntryCommitted(
@@ -212,7 +201,8 @@ void PermissionBubbleManager::Closing() {
}
void PermissionBubbleManager::ShowBubble() {
- if (view_ && !bubble_showing_ && requests_.size()) {
+ if (view_ && !bubble_showing_ && request_url_has_loaded_ &&
+ requests_.size()) {
// Note: this should appear above Show() for testing, since in that
// case we may do in-line calling of finalization.
bubble_showing_ = true;
@@ -237,9 +227,7 @@ void PermissionBubbleManager::FinalizeBubble() {
requests_ = queued_requests_;
accept_states_.resize(requests_.size(), true);
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();
+ ShowBubble();
} else {
request_url_ = GURL();
}
@@ -253,10 +241,3 @@ void PermissionBubbleManager::CancelPendingQueue() {
(*requests_iter)->RequestFinished();
}
}
-
-void PermissionBubbleManager::SetCoalesceIntervalForTesting(int interval_ms) {
- timer_.reset(new base::Timer(FROM_HERE,
- base::TimeDelta::FromMilliseconds(interval_ms),
- base::Bind(&PermissionBubbleManager::ShowBubble, base::Unretained(this)),
- false));
-}

Powered by Google App Engine
This is Rietveld 408576698