Make TabContents own its infobar delegates....

chrome/browser/geolocation/geolocation_permission_context.cc

Index: chrome/browser/geolocation/geolocation_permission_context.cc
===================================================================
--- chrome/browser/geolocation/geolocation_permission_context.cc	(revision 65711)
+++ chrome/browser/geolocation/geolocation_permission_context.cc	(working copy)
@@ -58,25 +58,32 @@
 
   // The InfoBar will be displayed immediately if the tab is not already
   // displaying one, otherwise it'll be queued.
-  void CreateInfoBarRequest(
-      int render_process_id, int render_view_id, int bridge_id,
-      const GURL& requesting_frame, const GURL& emebedder);
+  void CreateInfoBarRequest(int render_process_id,
+                            int render_view_id,
+                            int bridge_id,
+                            const GURL& requesting_frame,
+                            const GURL& emebedder);
 
   // Cancels a specific infobar request.
-  void CancelInfoBarRequest(
-      int render_process_id, int render_view_id, int bridge_id);
+  void CancelInfoBarRequest(int render_process_id,
+                            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);
+  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.
-  void OnPermissionSet(
-      int render_process_id, int render_view_id, int bridge_id,
-      const GURL& requesting_frame, const GURL& embedder, bool allowed);
+  void OnPermissionSet(int render_process_id,
+                       int render_view_id,
+                       int bridge_id,
+                       const GURL& requesting_frame,
+                       const GURL& embedder,
+                       bool allowed);
 
  private:
   struct PendingInfoBarRequest;
@@ -85,9 +92,9 @@
   // Shows the first pending infobar for this tab.
   void ShowQueuedInfoBar(int render_process_id, int render_view_id);
 
-  // Cancels an InfoBar request and returns the next iterator position.
-  std::vector<PendingInfoBarRequest>::iterator CancelInfoBarRequestInternal(
-      std::vector<PendingInfoBarRequest>::iterator i);
+  // Cancels the pending InfoBar request at |index|.  This removes the request
+  // from the pending list.
+  void CancelInfoBarRequestInternal(size_t index);
 
   GeolocationPermissionContext* const geolocation_permission_context_;
   Profile* const profile_;
@@ -101,8 +108,11 @@
 class GeolocationConfirmInfoBarDelegate : public ConfirmInfoBarDelegate {
  public:
   GeolocationConfirmInfoBarDelegate(
-      TabContents* tab_contents, GeolocationInfoBarQueueController* controller,
-      int render_process_id, int render_view_id, int bridge_id,
+      TabContents* tab_contents,
+      GeolocationInfoBarQueueController* controller,
+      int render_process_id,
+      int render_view_id,
+      int bridge_id,
       const GURL& requesting_frame_url,
       const std::string& display_languages)
       : ConfirmInfoBarDelegate(tab_contents),
@@ -217,8 +227,11 @@
 }
 
 void GeolocationInfoBarQueueController::CreateInfoBarRequest(
-    int render_process_id, int render_view_id, int bridge_id,
-    const GURL& requesting_frame, const GURL& embedder) {
+    int render_process_id,
+    int render_view_id,
+    int bridge_id,
+    const GURL& requesting_frame,
+    const GURL& embedder) {
   // This makes sure that no duplicates are added to
   // |pending_infobar_requests_| as an artificial permission request may
   // already exist in the queue as per
@@ -227,14 +240,13 @@
   // TODO(joth): Once we have CLIENT_BASED_GEOLOCATION and
   // WTF_USE_PREEMPT_GEOLOCATION_PERMISSION set in WebKit we should be able to
   // just use a DCHECK to check if a duplicate is attempting to be added.
-  PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
-  while (i != pending_infobar_requests_.end()) {
+  for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
+       i != pending_infobar_requests_.end(); ++i) {
     if (i->Equals(render_process_id, render_view_id, bridge_id)) {
       // The request already exists.
       DCHECK(i->IsForPair(requesting_frame, embedder));
       return;
     }
-    ++i;
   }
   PendingInfoBarRequest pending_infobar_request;
   pending_infobar_request.render_process_id = render_process_id;
@@ -248,18 +260,21 @@
 }
 
 void GeolocationInfoBarQueueController::CancelInfoBarRequest(
-    int render_process_id, int render_view_id, int bridge_id) {
-  for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
-       i != pending_infobar_requests_.end(); ++i) {
-    if (i->Equals(render_process_id, render_view_id, bridge_id)) {
+    int render_process_id,
+    int render_view_id,
+    int bridge_id) {
+  for (size_t i = 0; i < pending_infobar_requests_.size(); ++i) {
+    if (pending_infobar_requests_[i].Equals(render_process_id, render_view_id,
+                                            bridge_id)) {
       CancelInfoBarRequestInternal(i);
-      break;
+      return;
     }
   }
 }
 
-void GeolocationInfoBarQueueController::OnInfoBarClosed(
-    int render_process_id, int render_view_id, int bridge_id) {
+void GeolocationInfoBarQueueController::OnInfoBarClosed(int render_process_id,
+                                                        int render_view_id,
+                                                        int bridge_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
        i != pending_infobar_requests_.end(); ++i) {
@@ -272,8 +287,12 @@
 }
 
 void GeolocationInfoBarQueueController::OnPermissionSet(
-    int render_process_id, int render_view_id, int bridge_id,
-    const GURL& requesting_frame, const GURL& embedder, bool allowed) {
+    int render_process_id,
+    int render_view_id,
+    int bridge_id,
+    const GURL& requesting_frame,
+    const GURL& embedder,
+    bool allowed) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // Persist the permission.
   ContentSetting content_setting =
@@ -282,36 +301,36 @@
       requesting_frame.GetOrigin(), embedder.GetOrigin(), content_setting);
 
   // Now notify all pending requests that the permission has been set.
-  for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
-       i != pending_infobar_requests_.end();) {
-    if (i->IsForPair(requesting_frame, embedder)) {
+  for (size_t i = 0; i < pending_infobar_requests_.size(); ) {
+    PendingInfoBarRequest& request = pending_infobar_requests_[i];
+    if (request.IsForPair(requesting_frame, embedder)) {
       // There was a pending request for the same [frame, embedder].
-      if (i->Equals(render_process_id, render_view_id, bridge_id)) {
+      if (request.Equals(render_process_id, render_view_id, bridge_id)) {
         // The request that set permission will be removed by TabContents
         // itself, that is, we should not try to cancel the infobar that has
         // just notified us.
-        i->infobar_delegate = NULL;
+        request.infobar_delegate = NULL;
       }
       // Cancel it first, and then notify the permission.
       // Note: if the pending request had an infobar, TabContents will
       // eventually close it and we will pump the queue via OnInfoBarClosed().
-      PendingInfoBarRequest other_request = *i;
-      i = CancelInfoBarRequestInternal(i);
+      PendingInfoBarRequest copied_request(request);
+      CancelInfoBarRequestInternal(i);  // |request| is now garbage!
       geolocation_permission_context_->NotifyPermissionSet(
-          other_request.render_process_id, other_request.render_view_id,
-          other_request.bridge_id, other_request.requesting_frame, allowed);
+          copied_request.render_process_id, copied_request.render_view_id,
+          copied_request.bridge_id, copied_request.requesting_frame, allowed);
     } else {
       ++i;
     }
   }
 }
 
-void GeolocationInfoBarQueueController::ShowQueuedInfoBar(
-    int render_process_id, int render_view_id) {
+void GeolocationInfoBarQueueController::ShowQueuedInfoBar(int render_process_id,
+                                                          int render_view_id) {
   TabContents* tab_contents =
       tab_util::GetTabContentsByID(render_process_id, render_view_id);
   for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
-       i != pending_infobar_requests_.end();) {
+       i != pending_infobar_requests_.end(); ) {
     if (!i->IsForTab(render_process_id, render_view_id)) {
       ++i;
       continue;
@@ -333,42 +352,39 @@
   }
 }
 
-std::vector<GeolocationInfoBarQueueController::PendingInfoBarRequest>::iterator
-    GeolocationInfoBarQueueController::CancelInfoBarRequestInternal(
-        std::vector<PendingInfoBarRequest>::iterator i) {
-  TabContents* tab_contents =
-      tab_util::GetTabContentsByID(i->render_process_id, i->render_view_id);
-  if (tab_contents && i->infobar_delegate) {
-    // TabContents will destroy the InfoBar, which will remove from our vector
-    // asynchronously.
-    tab_contents->RemoveInfoBar(i->infobar_delegate);
-    return ++i;
+void GeolocationInfoBarQueueController::CancelInfoBarRequestInternal(size_t i) {
+  const PendingInfoBarRequest& request = pending_infobar_requests_[i];
+  TabContents* tab_contents = tab_util::GetTabContentsByID(
+      request.render_process_id, request.render_view_id);
+  InfoBarDelegate* delegate = request.infobar_delegate;
+  if (tab_contents && delegate) {
+    // TabContents will destroy the InfoBar, which will synchronously remove it
+    // from our vector, so we need to increment the iterator first.
+    tab_contents->RemoveInfoBar(delegate);
   } else {
-    // Remove it directly from the pending vector.
-    return pending_infobar_requests_.erase(i);
+    pending_infobar_requests_.erase(pending_infobar_requests_.begin() + i);
   }
 }
 
 GeolocationPermissionContext::GeolocationPermissionContext(
     Profile* profile)
     : profile_(profile),
-      ALLOW_THIS_IN_INITIALIZER_LIST(
-          geolocation_infobar_queue_controller_(
-             new GeolocationInfoBarQueueController(this, profile))) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(geolocation_infobar_queue_controller_(
+         new GeolocationInfoBarQueueController(this, profile))) {
 }
 
 GeolocationPermissionContext::~GeolocationPermissionContext() {
 }
 
 void GeolocationPermissionContext::RequestGeolocationPermission(
-    int render_process_id, int render_view_id, int bridge_id,
+    int render_process_id,
+    int render_view_id,
+    int bridge_id,
     const GURL& requesting_frame) {
   if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    BrowserThread::PostTask(
-        BrowserThread::UI, FROM_HERE,
-        NewRunnableMethod(this,
-            &GeolocationPermissionContext::RequestGeolocationPermission,
-            render_process_id, render_view_id, bridge_id, requesting_frame));
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, NewRunnableMethod(
+        this, &GeolocationPermissionContext::RequestGeolocationPermission,
+        render_process_id, render_view_id, bridge_id, requesting_frame));
     return;
   }
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -394,8 +410,8 @@
   if (!tab_contents) {
     // The tab may have gone away, or the request may not be from a tab at all.
     LOG(WARNING) << "Attempt to use geolocation tabless renderer: "
-        << render_process_id << "," << render_view_id << "," << bridge_id
-        << " (can't prompt user without a visible tab)";
+                 << render_process_id << "," << render_view_id << ","
+                 << bridge_id << " (can't prompt user without a visible tab)";
     NotifyPermissionSet(render_process_id, render_view_id, bridge_id,
                         requesting_frame, false);
     return;
@@ -404,8 +420,8 @@
   GURL embedder = tab_contents->GetURL();
   if (!requesting_frame.is_valid() || !embedder.is_valid()) {
     LOG(WARNING) << "Attempt to use geolocation from an invalid URL: "
-        << requesting_frame << "," << embedder
-        << " (geolocation is not supported in popups)";
+                 << requesting_frame << "," << embedder
+                 << " (geolocation is not supported in popups)";
     NotifyPermissionSet(render_process_id, render_view_id, bridge_id,
                         requesting_frame, false);
     return;
@@ -428,13 +444,17 @@
 }
 
 void GeolocationPermissionContext::CancelGeolocationPermissionRequest(
-    int render_process_id, int render_view_id, int bridge_id,
+    int render_process_id,
+    int render_view_id,
+    int bridge_id,
     const GURL& requesting_frame) {
   CancelPendingInfoBarRequest(render_process_id, render_view_id, bridge_id);
 }
 
 void GeolocationPermissionContext::StartUpdatingRequested(
-    int render_process_id, int render_view_id, int bridge_id,
+    int render_process_id,
+    int render_view_id,
+    int bridge_id,
     const GURL& requesting_frame) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   // Note we cannot store the arbitrator as a member as it is not thread safe.
@@ -452,13 +472,18 @@
 }
 
 void GeolocationPermissionContext::StopUpdatingRequested(
-    int render_process_id, int render_view_id, int bridge_id) {
+    int render_process_id,
+    int render_view_id,
+    int bridge_id) {
   CancelPendingInfoBarRequest(render_process_id, render_view_id, bridge_id);
 }
 
 void GeolocationPermissionContext::NotifyPermissionSet(
-    int render_process_id, int render_view_id, int bridge_id,
-    const GURL& requesting_frame, bool allowed) {
+    int render_process_id,
+    int render_view_id,
+    int bridge_id,
+    const GURL& requesting_frame,
+    bool allowed) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   TabContents* tab_contents =
@@ -472,17 +497,13 @@
                                                  allowed);
   }
 
-  CallRenderViewHost(
-      render_process_id, render_view_id,
-      &RenderViewHost::Send,
+  CallRenderViewHost(render_process_id, render_view_id, &RenderViewHost::Send,
       new ViewMsg_Geolocation_PermissionSet(render_view_id, bridge_id,
           allowed));
   if (allowed) {
-    BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE,
-        NewRunnableMethod(this,
-            &GeolocationPermissionContext::NotifyArbitratorPermissionGranted,
-            requesting_frame));
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, NewRunnableMethod(
+        this, &GeolocationPermissionContext::NotifyArbitratorPermissionGranted,
+        requesting_frame));
   }
 }
 
@@ -493,16 +514,16 @@
 }
 
 void GeolocationPermissionContext::CancelPendingInfoBarRequest(
-    int render_process_id, int render_view_id, int bridge_id) {
+    int render_process_id,
+    int render_view_id,
+    int bridge_id) {
   if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    BrowserThread::PostTask(
-        BrowserThread::UI, FROM_HERE,
-        NewRunnableMethod(this,
-            &GeolocationPermissionContext::CancelPendingInfoBarRequest,
-            render_process_id, render_view_id, bridge_id));
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, NewRunnableMethod(
+        this, &GeolocationPermissionContext::CancelPendingInfoBarRequest,
+        render_process_id, render_view_id, bridge_id));
      return;
   }
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  geolocation_infobar_queue_controller_->CancelInfoBarRequest(
-      render_process_id, render_view_id, bridge_id);
+  geolocation_infobar_queue_controller_->CancelInfoBarRequest(render_process_id,
+      render_view_id, bridge_id);
 }