Add BubbleManager to manage bubbles and ChromeBubbleManager for events.

chrome/browser/ui/website_settings/permission_bubble_manager.cc

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 a2ace674da4eeb7e68fd9d0f0648eea5cbe140ae..f102d7c193f82ce91fe0199549ee04ead17179b2 100644
--- a/chrome/browser/ui/website_settings/permission_bubble_manager.cc
+++ b/chrome/browser/ui/website_settings/permission_bubble_manager.cc
@@ -6,6 +6,11 @@
 
 #include "base/command_line.h"
 #include "base/metrics/user_metrics_action.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/chrome_bubble_manager.h"
+#include "chrome/browser/ui/chrome_bubble_manager_factory.h"
+#include "chrome/browser/ui/website_settings/permission_bubble_delegate.h"
 #include "chrome/browser/ui/website_settings/permission_bubble_request.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/browser/browser_thread.h"
@@ -79,27 +84,14 @@ PermissionBubbleManager::PermissionBubbleManager(
 #if !defined(OS_ANDROID)  // No bubbles in android tests.
       view_factory_(base::Bind(&PermissionBubbleView::Create)),
 #endif
-      view_(nullptr),
       main_frame_has_fully_loaded_(false),
       auto_response_for_test_(NONE),
       weak_factory_(this) {
 }
 
 PermissionBubbleManager::~PermissionBubbleManager() {
-  if (view_ != NULL)
-    view_->SetDelegate(NULL);
-
-  std::vector<PermissionBubbleRequest*>::iterator requests_iter;
-  for (requests_iter = requests_.begin();
-       requests_iter != requests_.end();
-       requests_iter++) {
-    (*requests_iter)->RequestFinished();
-  }
-  for (requests_iter = queued_requests_.begin();
-       requests_iter != queued_requests_.end();
-       requests_iter++) {
-    (*requests_iter)->RequestFinished();
-  }
+  CancelPendingQueues();
+  CloseBubble();
 }
 
 void PermissionBubbleManager::AddRequest(PermissionBubbleRequest* request) {
@@ -148,7 +140,7 @@ void PermissionBubbleManager::AddRequest(PermissionBubbleRequest* request) {
   }
 
   if (!require_user_gesture_ || request->HasUserGesture())
-    ScheduleShowBubble();
+    TriggerShowBubble();
 }
 
 void PermissionBubbleManager::CancelRequest(PermissionBubbleRequest* request) {
@@ -174,17 +166,23 @@ void PermissionBubbleManager::CancelRequest(PermissionBubbleRequest* request) {
 
     // We can simply erase the current entry in the request table if we aren't
     // showing the dialog, or if we are showing it and it can accept the update.
-    bool can_erase = !IsBubbleVisible() || view_->CanAcceptRequestUpdate();
+    bool can_erase = !IsBubbleVisible();
+
+    // TODO(hcarmona): Don't forget this logic. Maybe improve it? Or remove it?
+    // bool can_erase = !IsBubbleVisible() || view_->CanAcceptRequestUpdate();
+
     if (can_erase) {
       (*requests_iter)->RequestFinished();
       requests_.erase(requests_iter);
       accept_states_.erase(accepts_iter);
 
+      /*
       if (IsBubbleVisible()) {
-        view_->Hide();
+        UpdateBubble(); // TODO(hcarmona): HOW?

[groby-ooo-7-16, 2015/08/06 21:32:56]

Why not keep the old approach of calling Hide()?

[hcarmona, 2015/08/07 02:12:58]

Anyone who lets the bubble manager manage their bubble shouldn't be able
to tell the manager when to hide/show a bubble because that's stepping
on the bubble manager's responsibilities. I'm not sure what the best
approach for this case is.

         // Will redraw the bubble if it is being shown.
         TriggerShowBubble();
       }
+      */
       return;
     }
 
@@ -199,39 +197,15 @@ void PermissionBubbleManager::CancelRequest(PermissionBubbleRequest* request) {
   NOTREACHED();  // Callers should not cancel requests that are not pending.
 }
 
-void PermissionBubbleManager::HideBubble() {
-  // Disengage from the existing view if there is one.
-  if (!view_)
+void PermissionBubbleManager::CloseBubble() {
+  if (!active_bubble_)
     return;
-
-  view_->SetDelegate(nullptr);
-  view_->Hide();
-  view_.reset();
-}
-
-void PermissionBubbleManager::DisplayPendingRequests(Browser* browser) {
-  if (IsBubbleVisible())
-    return;
-
-  view_ = view_factory_.Run(browser);
-  view_->SetDelegate(this);
-
-  TriggerShowBubble();
-}
-
-void PermissionBubbleManager::UpdateAnchorPosition() {
-  if (view_)
-    view_->UpdateAnchorPosition();
+  ChromeBubbleManagerFactory::GetForBrowserContext(browser_->profile())

[groby-ooo-7-16, 2015/08/06 21:32:56]

Can we just call active_bubble_->Close() here?

[hcarmona, 2015/08/07 02:12:58]

Closing bubbles should go through the bubble manager because bubbles
don't know about the bubble manager.

+    ->CloseBubble(active_bubble_);
 }
 
 bool PermissionBubbleManager::IsBubbleVisible() {
-  return view_ && view_->IsVisible();
-}
-
-gfx::NativeWindow PermissionBubbleManager::GetBubbleWindow() {
-  if (view_)
-    return view_->GetNativeWindow();
-  return nullptr;
+  return active_bubble_;
 }
 
 void PermissionBubbleManager::RequireUserGesture(bool required) {
@@ -254,12 +228,12 @@ void PermissionBubbleManager::DocumentOnLoadCompletedInMainFrame() {
   // 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.
-  ScheduleShowBubble();
+  TriggerShowBubble();
 }
 
 void PermissionBubbleManager::DocumentLoadedInFrame(
     content::RenderFrameHost* render_frame_host) {
-  ScheduleShowBubble();
+  TriggerShowBubble();
 }
 
 void PermissionBubbleManager::NavigationEntryCommitted(
@@ -269,22 +243,19 @@ void PermissionBubbleManager::NavigationEntryCommitted(
     return;
 
   // If we have navigated to a new url or reloaded the page...
-  // GetAsReferrer strips fragment and username/password, meaning
-  // the navigation is really to the same page.
-  if ((request_url_.GetAsReferrer() !=
-       web_contents()->GetLastCommittedURL().GetAsReferrer()) ||
-      (details.type == content::NAVIGATION_TYPE_EXISTING_PAGE &&
-       !details.is_in_page)) {
-    // Kill off existing bubble and cancel any pending requests.
+  if (!details.is_in_page ||

[groby-ooo-7-16, 2015/08/06 21:32:56]

That's a separate fix, no? I suppose we're just waiting to rebase when that CL
landed?

[hcarmona, 2015/08/07 02:12:58]

Yes, reverted.

+      details.type == content::NAVIGATION_TYPE_SAME_PAGE ||
+      details.type == content::NAVIGATION_TYPE_EXISTING_PAGE) {
+    // kill off existing bubble and cancel any pending requests.
     CancelPendingQueues();
-    FinalizeBubble();
+    CloseBubble();
   }
 }
 
 void PermissionBubbleManager::WebContentsDestroyed() {
   // If the web contents has been destroyed, treat the bubble as cancelled.
   CancelPendingQueues();
-  FinalizeBubble();
+  CloseBubble();
 
   // The WebContents is going away; be aggressively paranoid and delete
   // ourselves lest other parts of the system attempt to add permission bubbles
@@ -310,7 +281,7 @@ void PermissionBubbleManager::Accept() {
     else
       (*requests_iter)->PermissionDenied();
   }
-  FinalizeBubble();
+  CloseBubble();
 }
 
 void PermissionBubbleManager::Deny() {
@@ -320,7 +291,7 @@ void PermissionBubbleManager::Deny() {
        requests_iter++) {
     (*requests_iter)->PermissionDenied();
   }
-  FinalizeBubble();
+  CloseBubble();
 }
 
 void PermissionBubbleManager::Closing() {
@@ -330,25 +301,25 @@ void PermissionBubbleManager::Closing() {
        requests_iter++) {
     (*requests_iter)->Cancelled();
   }
-  FinalizeBubble();
+  CloseBubble();
 }
 
-void PermissionBubbleManager::ScheduleShowBubble() {
-  // ::ScheduleShowBubble() will be called again when the main frame will be
-  // loaded.
-  if (!main_frame_has_fully_loaded_)
-    return;
-
-  content::BrowserThread::PostTask(

[groby-ooo-7-16, 2015/08/06 21:32:56]

We needed a thread transition here - are we now guaranteed that Finalize will
always be called on the UI thread?

[hcarmona, 2015/08/07 02:12:58]

The bubble manager will always defer show to the UI thread.

-      content::BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(&PermissionBubbleManager::TriggerShowBubble,
-                 weak_factory_.GetWeakPtr()));
+void PermissionBubbleManager::Finalize() {
+  std::vector<PermissionBubbleRequest*>::iterator requests_iter;
+  for (requests_iter = requests_.begin();

[groby-ooo-7-16, 2015/08/06 21:32:56]

C++11 please

for(PermissionBubbleRequest* request : requests_)
  request->RequestFinished();

[hcarmona, 2015/08/07 02:12:58]

Awesome, I'll update the iterators.

+       requests_iter != requests_.end();
+       requests_iter++) {
+    (*requests_iter)->RequestFinished();
+  }
+  requests_.clear();
+  accept_states_.clear();
+  if (queued_requests_.size() || queued_frame_requests_.size())
+    TriggerShowBubble();
+  else
+    request_url_ = GURL();
 }
 
 void PermissionBubbleManager::TriggerShowBubble() {
-  if (!view_)
-    return;
   if (IsBubbleVisible())
     return;
   if (!main_frame_has_fully_loaded_)
@@ -376,7 +347,14 @@ void PermissionBubbleManager::TriggerShowBubble() {
     accept_states_.resize(requests_.size(), true);
   }
 
-  view_->Show(requests_, accept_states_);
+  if (!active_bubble_) {
+    ChromeBubbleManagerFactory::GetForBrowserContext(browser_->profile())

[groby-ooo-7-16, 2015/08/06 21:32:56]

Doesn't ShowBubble return an active_bubble_?

[hcarmona, 2015/08/07 02:12:58]

Yes... this code's broken. Fixed.

+      ->ShowBubble(
+          make_scoped_ptr(new PermissionBubbleDelegate(this, view_factory_)));
+  } else {
+    NOTREACHED();

[groby-ooo-7-16, 2015/08/06 21:32:56]

Please don't handle NOTREACHED separately. If active_bubble_ truly never can be
set, just DCHECK before calling ShowBubble

[hcarmona, 2015/08/07 02:12:58]

Yes, this was debug code and it shouldn't exist.

+  }
+
   NotifyBubbleAdded();
 
   // If in testing mode, automatically respond to the bubble that was shown.
@@ -384,24 +362,6 @@ void PermissionBubbleManager::TriggerShowBubble() {
     DoAutoResponseForTesting();
 }
 
-void PermissionBubbleManager::FinalizeBubble() {
-  if (view_)
-    view_->Hide();
-
-  std::vector<PermissionBubbleRequest*>::iterator requests_iter;
-  for (requests_iter = requests_.begin();
-       requests_iter != requests_.end();
-       requests_iter++) {
-    (*requests_iter)->RequestFinished();
-  }
-  requests_.clear();
-  accept_states_.clear();
-  if (queued_requests_.size() || queued_frame_requests_.size())
-    TriggerShowBubble();
-  else
-    request_url_ = GURL();
-}
-
 void PermissionBubbleManager::CancelPendingQueues() {
   std::vector<PermissionBubbleRequest*>::iterator requests_iter;
   for (requests_iter = queued_requests_.begin();