Introduce GeolocationPermissionRequestID, a wrapper struct to contain the (render process ID, rende…

chrome/browser/geolocation/geolocation_infobar_queue_controller.cc

Index: chrome/browser/geolocation/geolocation_infobar_queue_controller.cc
===================================================================
--- chrome/browser/geolocation/geolocation_infobar_queue_controller.cc	(revision 163741)
+++ chrome/browser/geolocation/geolocation_infobar_queue_controller.cc	(working copy)
@@ -16,6 +16,7 @@
 #include "chrome/common/content_settings.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/geolocation.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
@@ -28,23 +29,15 @@
 
 struct GeolocationInfoBarQueueController::PendingInfoBarRequest {
  public:
-  PendingInfoBarRequest(int render_process_id,
-                        int render_view_id,
-                        int bridge_id,
+  PendingInfoBarRequest(const content::GeolocationPermissionRequestID& id,
                         const GURL& requesting_frame,
                         const GURL& embedder,
                         PermissionDecidedCallback callback);
 
-  bool IsForTab(int p_render_process_id, int p_render_view_id) const;
   bool IsForPair(const GURL& p_requesting_frame,
                  const GURL& p_embedder) const;
-  bool Equals(int p_render_process_id,
-              int p_render_view_id,
-              int p_bridge_id) const;
 
-  int render_process_id;
-  int render_view_id;
-  int bridge_id;
+  content::GeolocationPermissionRequestID id;
   GURL requesting_frame;
   GURL embedder;
   PermissionDecidedCallback callback;
@@ -52,73 +45,24 @@
 };
 
 GeolocationInfoBarQueueController::PendingInfoBarRequest::PendingInfoBarRequest(
-    int render_process_id,
-    int render_view_id,
-    int bridge_id,
+    const content::GeolocationPermissionRequestID& id,
     const GURL& requesting_frame,
     const GURL& embedder,
     PermissionDecidedCallback callback)
-    : render_process_id(render_process_id),
-      render_view_id(render_view_id),
-      bridge_id(bridge_id),
+    : id(id),
       requesting_frame(requesting_frame),
       embedder(embedder),
       callback(callback),
       infobar_delegate(NULL) {
 }
 
-bool GeolocationInfoBarQueueController::PendingInfoBarRequest::IsForTab(
-    int p_render_process_id,
-    int p_render_view_id) const {
-  return (render_process_id == p_render_process_id) &&
-      (render_view_id == p_render_view_id);
-}
-
 bool GeolocationInfoBarQueueController::PendingInfoBarRequest::IsForPair(
     const GURL& p_requesting_frame,
     const GURL& p_embedder) const {
   return (requesting_frame == p_requesting_frame) && (embedder == p_embedder);
 }
 
-bool GeolocationInfoBarQueueController::PendingInfoBarRequest::Equals(
-    int p_render_process_id,
-    int p_render_view_id,
-    int p_bridge_id) const {
-  return IsForTab(p_render_process_id, p_render_view_id) &&
-     (bridge_id == p_bridge_id);
-}
 
-
-// GeolocationInfoBarQueueController::RequestEquals ---------------------------
-
-// Useful predicate for checking PendingInfoBarRequest equality.
-class GeolocationInfoBarQueueController::RequestEquals
-    : public std::unary_function<PendingInfoBarRequest, bool> {
- public:
-  RequestEquals(int render_process_id, int render_view_id, int bridge_id);
-
-  bool operator()(const PendingInfoBarRequest& request) const;
-
- private:
-  int render_process_id_;
-  int render_view_id_;
-  int bridge_id_;
-};
-
-GeolocationInfoBarQueueController::RequestEquals::RequestEquals(
-    int render_process_id,
-    int render_view_id,
-    int bridge_id)
-    : render_process_id_(render_process_id),
-      render_view_id_(render_view_id),
-      bridge_id_(bridge_id) {
-}
-
-bool GeolocationInfoBarQueueController::RequestEquals::operator()(
-    const PendingInfoBarRequest& request) const {
-  return request.Equals(render_process_id_, render_view_id_, bridge_id_);
-}
-
 // GeolocationInfoBarQueueController ------------------------------------------
 
 GeolocationInfoBarQueueController::GeolocationInfoBarQueueController(
@@ -130,65 +74,50 @@
 }
 
 void GeolocationInfoBarQueueController::CreateInfoBarRequest(
-    int render_process_id,
-    int render_view_id,
-    int bridge_id,
+    const content::GeolocationPermissionRequestID& id,
     const GURL& requesting_frame,
     const GURL& embedder,
     PermissionDecidedCallback callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // We shouldn't get duplicate requests.
-  DCHECK(std::find_if(pending_infobar_requests_.begin(),
-      pending_infobar_requests_.end(),
-      RequestEquals(render_process_id, render_view_id, bridge_id)) ==
-      pending_infobar_requests_.end());
+  for (PendingInfoBarRequests::const_iterator i(
+           pending_infobar_requests_.begin());
+       i != pending_infobar_requests_.end(); ++i)
+    DCHECK(!i->id.Equals(id));
 
-  InfoBarTabHelper* helper = GetInfoBarHelper(render_process_id,
-                                              render_view_id);
-  if (!helper) {
-    // We won't be able to create any infobars, shortcut and remove any pending
-    // requests.
-    ClearPendingInfoBarRequestsForTab(render_process_id, render_view_id);
-    return;
+  pending_infobar_requests_.push_back(PendingInfoBarRequest(
+      id, requesting_frame, embedder, callback));
+  if (!AlreadyShowingInfoBarForTab(id) && !ShowQueuedInfoBarForTab(id)) {
+    // We can only get here if there's no InfoBarTabHelper.
+    ClearPendingInfoBarRequestsForTab(id);
   }
-  pending_infobar_requests_.push_back(PendingInfoBarRequest(render_process_id,
-      render_view_id, bridge_id, requesting_frame, embedder, callback));
-  if (!AlreadyShowingInfoBar(render_process_id, render_view_id))
-    ShowQueuedInfoBar(render_process_id, render_view_id, helper);
 }
 
 void GeolocationInfoBarQueueController::CancelInfoBarRequest(
-    int render_process_id,
-    int render_view_id,
-    int bridge_id) {
+    const content::GeolocationPermissionRequestID& 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));
-  // TODO(pkasting): Can this conditional become a DCHECK()?
-  if (i == pending_infobar_requests_.end())
-    return;
-  InfoBarDelegate* delegate = i->infobar_delegate;
-  if (!delegate) {
-    pending_infobar_requests_.erase(i);
-    return;
+  for (PendingInfoBarRequests::iterator i(pending_infobar_requests_.begin());
+       i != pending_infobar_requests_.end(); ++i) {
+    if (i->id.Equals(id)) {
+      if (i->infobar_delegate)
+        GetInfoBarHelper(id)->RemoveInfoBar(i->infobar_delegate);
+      else
+        pending_infobar_requests_.erase(i);
+      return;
+    }
   }
-  InfoBarTabHelper* helper = GetInfoBarHelper(render_process_id,
-                                              render_view_id);
-  helper->RemoveInfoBar(delegate);
 }
 
 void GeolocationInfoBarQueueController::OnPermissionSet(
-    int render_process_id,
-    int render_view_id,
-    int bridge_id,
+    const content::GeolocationPermissionRequestID& id,
     const GURL& requesting_frame,
     const GURL& embedder,
     bool update_content_setting,
     bool allowed) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
   if (update_content_setting) {
     ContentSetting content_setting =
         allowed ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
@@ -199,6 +128,7 @@
         std::string(),
         content_setting);
   }
+
   // Cancel this request first, then notify listeners.  TODO(pkasting): Why
   // is this order important?
   PendingInfoBarRequests requests_to_notify;
@@ -207,7 +137,7 @@
        i != pending_infobar_requests_.end(); ) {
     if (i->IsForPair(requesting_frame, embedder)) {
       requests_to_notify.push_back(*i);
-      if (i->Equals(render_process_id, render_view_id, bridge_id)) {
+      if (i->id.Equals(id)) {
         // The delegate that called us is i, and it's currently in either
         // Accept() or Cancel(). This means that the owning InfoBar will call
         // RemoveInfoBar() later on, and that will trigger a notification we're
@@ -229,17 +159,13 @@
 
   // Remove all InfoBars for the same |requesting_frame| and |embedder|.
   for (PendingInfoBarRequests::iterator i = infobars_to_remove.begin();
-       i != infobars_to_remove.end(); ++i ) {
-    InfoBarTabHelper* helper = GetInfoBarHelper(i->render_process_id,
-                                                i->render_view_id);
-    helper->RemoveInfoBar(i->infobar_delegate);
-  }
+       i != infobars_to_remove.end(); ++i )
+    GetInfoBarHelper(i->id)->RemoveInfoBar(i->infobar_delegate);
 
   // Send out the permission notifications.
   for (PendingInfoBarRequests::iterator i = requests_to_notify.begin();
-       i != requests_to_notify.end(); ++i ) {
+       i != requests_to_notify.end(); ++i)
     i->callback.Run(allowed);
-  }
 }
 
 void GeolocationInfoBarQueueController::Observe(
@@ -262,59 +188,66 @@
        i != pending_infobar_requests_.end(); ++i) {
     GeolocationConfirmInfoBarDelegate* confirm_delegate = i->infobar_delegate;
     if (confirm_delegate == delegate) {
-      InfoBarTabHelper* helper =
-          content::Source<InfoBarTabHelper>(source).ptr();
-      int render_process_id = i->render_process_id;
-      int render_view_id = i->render_view_id;
+      content::GeolocationPermissionRequestID id = i->id;
       pending_infobar_requests_.erase(i);
-      ShowQueuedInfoBar(render_process_id, render_view_id, helper);
+      DCHECK_EQ(content::Source<InfoBarTabHelper>(source).ptr(),
+                GetInfoBarHelper(id));
+      ShowQueuedInfoBarForTab(id);
       return;
     }
   }
 }
 
 GeolocationConfirmInfoBarDelegate*
-GeolocationInfoBarQueueController::CreateInfoBarDelegate(
-      InfoBarTabHelper* infobar_helper,
-      GeolocationInfoBarQueueController* controller,
-      int render_process_id,
-      int render_view_id,
-      int bridge_id,
-      const GURL& requesting_frame_url,
-      const std::string& display_languages) {
+    GeolocationInfoBarQueueController::CreateInfoBarDelegate(
+    InfoBarTabHelper* infobar_helper,
+    GeolocationInfoBarQueueController* controller,
+    const content::GeolocationPermissionRequestID& id,
+    const GURL& requesting_frame_url,
+    const std::string& display_languages) {
   return GeolocationConfirmInfoBarDelegateFactory::Create(
-      infobar_helper, controller, render_process_id, render_view_id, bridge_id,
-      requesting_frame_url, display_languages);
+      infobar_helper, controller, id, requesting_frame_url, display_languages);
 }
 
-void GeolocationInfoBarQueueController::ShowQueuedInfoBar(
-    int render_process_id,
-    int render_view_id,
-    InfoBarTabHelper* helper) {
-  DCHECK(helper);
-  DCHECK(!AlreadyShowingInfoBar(render_process_id, render_view_id));
+bool GeolocationInfoBarQueueController::AlreadyShowingInfoBarForTab(
+    const content::GeolocationPermissionRequestID& id) {
   for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
        i != pending_infobar_requests_.end(); ++i) {
-    if (i->IsForTab(render_process_id, render_view_id) &&
-        !i->infobar_delegate) {
+    if (i->id.IsForSameTabAs(id) && i->infobar_delegate)
+      return true;
+  }
+  return false;
+}
+
+bool GeolocationInfoBarQueueController::ShowQueuedInfoBarForTab(
+    const content::GeolocationPermissionRequestID& id) {
+  DCHECK(!AlreadyShowingInfoBarForTab(id));
+
+  InfoBarTabHelper* helper = GetInfoBarHelper(id);
+  if (!helper)
+    return false;
+
+  for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
+       i != pending_infobar_requests_.end(); ++i) {
+    if (i->id.IsForSameTabAs(id) && !i->infobar_delegate) {
       RegisterForInfoBarNotifications(helper);
       i->infobar_delegate = CreateInfoBarDelegate(
-          helper, this, render_process_id,
-          render_view_id, i->bridge_id, i->requesting_frame,
+          helper, this, i->id, i->requesting_frame,
           profile_->GetPrefs()->GetString(prefs::kAcceptLanguages));
       helper->AddInfoBar(i->infobar_delegate);
-      return;
+      return true;
     }
   }
+
   UnregisterForInfoBarNotifications(helper);
+  return false;
 }
 
 void GeolocationInfoBarQueueController::ClearPendingInfoBarRequestsForTab(
-    int render_process_id,
-    int render_view_id) {
+    const content::GeolocationPermissionRequestID& id) {
   for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
        i != pending_infobar_requests_.end(); ) {
-    if (i->IsForTab(render_process_id, render_view_id))
+    if (i->id.IsForSameTabAs(id))
       i = pending_infobar_requests_.erase(i);
     else
       ++i;
@@ -322,24 +255,12 @@
 }
 
 InfoBarTabHelper* GeolocationInfoBarQueueController::GetInfoBarHelper(

[John Knottenbelt, 2012/10/24 10:59:10]

Can this be a non-member function local to this file?

[Peter Kasting, 2012/10/24 19:50:48]

I think so.  Will change.

-    int render_process_id,
-    int render_view_id) {
+    const content::GeolocationPermissionRequestID& id) {
   WebContents* web_contents =
-      tab_util::GetWebContentsByID(render_process_id, render_view_id);
+      tab_util::GetWebContentsByID(id.render_process_id, id.render_view_id);
   return web_contents ? InfoBarTabHelper::FromWebContents(web_contents) : NULL;
 }
 
-bool GeolocationInfoBarQueueController::AlreadyShowingInfoBar(
-    int render_process_id,
-    int render_view_id) {
-  for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
-       i != pending_infobar_requests_.end(); ++i) {
-    if (i->IsForTab(render_process_id, render_view_id) && i->infobar_delegate)
-      return true;
-  }
-  return false;
-}
-
 void GeolocationInfoBarQueueController::RegisterForInfoBarNotifications(
     InfoBarTabHelper* helper) {
   if (!registrar_.IsRegistered(