Show queued geolocation InfoBars when InfoBar is hidden.

chrome/browser/geolocation/chrome_geolocation_permission_context.cc

Index: chrome/browser/geolocation/chrome_geolocation_permission_context.cc
diff --git a/chrome/browser/geolocation/chrome_geolocation_permission_context.cc b/chrome/browser/geolocation/chrome_geolocation_permission_context.cc
index 1943ed53ec4e70b1856321589ad4841f5385d78b..6ca801c37139a720c6d3a86adb02c2455025ae75 100644
--- a/chrome/browser/geolocation/chrome_geolocation_permission_context.cc
+++ b/chrome/browser/geolocation/chrome_geolocation_permission_context.cc
@@ -14,18 +14,21 @@
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/google/google_util.h"
+#include "chrome/browser/infobars/infobar.h"
 #include "chrome/browser/infobars/infobar_tab_helper.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/tab_contents/confirm_infobar_delegate.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
+#include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/pref_names.h"
 #include "content/browser/renderer_host/render_view_host.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
@@ -77,12 +80,6 @@ class GeolocationInfoBarQueueController : content::NotificationObserver {
                             int render_view_id,
                             int bridge_id);
 
-  // Called by the InfoBarDelegate to notify it's closed. It'll display a new
-  // InfoBar if there's any request pending for this tab.
-  void OnInfoBarClosed(int render_process_id,
-                       int render_view_id,
-                       int bridge_id);
-
   // Called by the InfoBarDelegate to notify permission has been set.
   // It'll notify and dismiss any other pending InfoBar request for the same
   // |requesting_frame| and embedder.
@@ -93,7 +90,7 @@ class GeolocationInfoBarQueueController : content::NotificationObserver {
                        const GURL& embedder,
                        bool allowed);
 
-  // content::NotificationObserver
+  // content::NotificationObserver (NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED)

[tfarina, 2012/03/01 16:14:23]

nit: we usually don't specify in which notification we're interested here.
This line of comment is just to give a hint to the reader  from where this
Observe function was overridden from.

[John Knottenbelt, 2012/03/02 11:47:26]

Done.

   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details);
@@ -134,8 +131,10 @@ class GeolocationConfirmInfoBarDelegate : public ConfirmInfoBarDelegate {
       const GURL& requesting_frame_url,
       const std::string& display_languages);
 
+  int render_process_id() const { return render_process_id_; }
+  int render_view_id() const { return render_view_id_; }
+
  private:
-  virtual ~GeolocationConfirmInfoBarDelegate();
 
   // ConfirmInfoBarDelegate:
   virtual bool ShouldExpire(
@@ -184,11 +183,6 @@ GeolocationConfirmInfoBarDelegate::GeolocationConfirmInfoBarDelegate(
       committed_entry->GetUniqueID() : 0;
 }
 
-GeolocationConfirmInfoBarDelegate::~GeolocationConfirmInfoBarDelegate() {
-  controller_->OnInfoBarClosed(render_process_id_, render_view_id_,
-                               bridge_id_);
-}
-
 bool GeolocationConfirmInfoBarDelegate::ShouldExpire(
     const content::LoadCommittedDetails& details) const {
   if (details.did_replace_entry || !details.is_navigation_to_different_page())
@@ -282,7 +276,7 @@ struct GeolocationInfoBarQueueController::PendingInfoBarRequest {
   GURL requesting_frame;
   GURL embedder;
   base::Callback<void(bool)> callback;
-  InfoBarDelegate* infobar_delegate;
+  GeolocationConfirmInfoBarDelegate* infobar_delegate;
 };
 
 GeolocationInfoBarQueueController::PendingInfoBarRequest::PendingInfoBarRequest(
@@ -353,7 +347,6 @@ bool GeolocationInfoBarQueueController::RequestEquals::operator()(
   return request.Equals(render_process_id_, render_view_id_, bridge_id_);
 }
 
-
 // GeolocationInfoBarQueueController ------------------------------------------
 
 GeolocationInfoBarQueueController::GeolocationInfoBarQueueController(
@@ -400,20 +393,6 @@ void GeolocationInfoBarQueueController::CancelInfoBarRequest(
     CancelInfoBarRequestInternal(i);
 }
 
-void GeolocationInfoBarQueueController::OnInfoBarClosed(int render_process_id,
-                                                        int render_view_id,
-                                                        int bridge_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  PendingInfoBarRequests::iterator i = std::find_if(
-      pending_infobar_requests_.begin(), pending_infobar_requests_.end(),
-      RequestEquals(render_process_id, render_view_id, bridge_id));
-  if (i != pending_infobar_requests_.end())
-    pending_infobar_requests_.erase(i);
-
-  ShowQueuedInfoBar(render_process_id, render_view_id);
-}
-
 void GeolocationInfoBarQueueController::OnPermissionSet(
     int render_process_id,
     int render_view_id,
@@ -462,19 +441,27 @@ void GeolocationInfoBarQueueController::OnPermissionSet(
 }
 
 void GeolocationInfoBarQueueController::Observe(
-    int type, const content::NotificationSource& source,
+    int type,
+    const content::NotificationSource& source,
     const content::NotificationDetails& details) {
-  registrar_.Remove(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
-                    source);
-  WebContents* web_contents = content::Source<WebContents>(source).ptr();
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[Peter Kasting, 2012/03/01 19:46:54]

Nit: I don't think this DCHECK is all that important -- a ton of stuff across
all Chrome would break badly if these notifications got sent on a non-UI thread.

[John Knottenbelt, 2012/03/02 11:47:26]

Done.

[John Knottenbelt, 2012/03/02 11:47:26]

Done.

+  DCHECK(type == chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED);

[Peter Kasting, 2012/03/01 19:46:54]

Nit: Can be DCHECK_EQ(expected, actual)

[John Knottenbelt, 2012/03/02 11:47:26]

Done.

+  // It's possible that we receive this notification for a non-geolocation
+  // confirm info bar. We need to check that the infobar delegate

[Peter Kasting, 2012/03/01 19:46:54]

Nit: Nix "confirm".  Actually, "We will receive this notification for all
infobar closures, so we need to check whether this is the geolocation infobar
we're tracking" might be clearer?

[John Knottenbelt, 2012/03/02 11:47:26]

Done.

+  // corresponds to the one we think is showing.
+  InfoBarRemovedDetails* removed_details =

[Peter Kasting, 2012/03/01 19:46:54]

Nit: Can inline this into the next line

[John Knottenbelt, 2012/03/02 11:47:26]

Done.

+      content::Details<InfoBarRemovedDetails>(details).ptr();
+  InfoBarDelegate* delegate = removed_details->first;
   for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
-       i != pending_infobar_requests_.end();) {
-    if (i->infobar_delegate == NULL &&
-        web_contents == tab_util::GetWebContentsByID(i->render_process_id,
-                                                     i->render_view_id)) {
-      i = pending_infobar_requests_.erase(i);
-    } else {
-      ++i;
+       i != pending_infobar_requests_.end(); ++i) {
+    GeolocationConfirmInfoBarDelegate* confirm_delegate = i->infobar_delegate;
+    if (confirm_delegate == delegate) {
+      pending_infobar_requests_.erase(i);
+      registrar_.Remove(this, chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED,
+                        source);
+      ShowQueuedInfoBar(confirm_delegate->render_process_id(),
+                        confirm_delegate->render_view_id());
+      return;
     }
   }
 }
@@ -494,15 +481,17 @@ void GeolocationInfoBarQueueController::ShowQueuedInfoBar(int render_process_id,
         continue;
       }
 
+      InfoBarTabHelper* helper = wrapper->infobar_tab_helper();

[tfarina, 2012/03/01 16:14:23]

nit: Move this into the if below and use it in line 497 as well?

[John Knottenbelt, 2012/03/02 11:47:26]

Done.

+
       if (!i->infobar_delegate) {
-        if (!registrar_.IsRegistered(
-                this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
-                content::Source<WebContents>(tab_contents))) {
-          registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
-                         content::Source<WebContents>(tab_contents));
-        }
+        DCHECK(!registrar_.IsRegistered(
+            this, chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED,
+            content::Source<InfoBarTabHelper>(helper)));
+        registrar_.Add(
+            this, chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED,
+            content::Source<InfoBarTabHelper>(helper));
         i->infobar_delegate = new GeolocationConfirmInfoBarDelegate(
-            wrapper->infobar_tab_helper(), this, render_process_id,
+            helper, this, render_process_id,
             render_view_id, i->bridge_id, i->requesting_frame,
             profile_->GetPrefs()->GetString(prefs::kAcceptLanguages));
         wrapper->infobar_tab_helper()->AddInfoBar(i->infobar_delegate);