Allow duplicate infobar requests

chrome/browser/content_settings/permission_queue_controller.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/content_settings/permission_queue_controller.h"
 
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
 #include "chrome/browser/geolocation/geolocation_infobar_delegate.h"
@@ skipping 135 matching lines @@
 }
 
 void PermissionQueueController::CreateInfoBarRequest(
     const PermissionRequestID& id,
     const GURL& requesting_frame,
     const GURL& embedder,
     const std::string& accept_button_label,
     PermissionDecidedCallback callback) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
-  // We shouldn't get duplicate requests.
-  for (PendingInfobarRequests::const_iterator i(
-           pending_infobar_requests_.begin());
-       i != pending_infobar_requests_.end(); ++i)
-    DCHECK(!i->id().Equals(id));
-
   pending_infobar_requests_.push_back(PendingInfobarRequest(
       type_, id, requesting_frame, embedder,
       accept_button_label, callback));
   if (!AlreadyShowingInfoBarForTab(id))
     ShowQueuedInfoBarForTab(id);
 }
 
 void PermissionQueueController::CancelInfoBarRequest(
     const PermissionRequestID& id) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
@@ skipping 10 matching lines @@
   }
 }
 
 void PermissionQueueController::OnPermissionSet(
     const PermissionRequestID& id,
     const GURL& requesting_frame,
     const GURL& embedder,
     bool update_content_setting,
     bool allowed) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-
   if (update_content_setting)
     UpdateContentSetting(requesting_frame, embedder, allowed);
 
   // Cancel this request first, then notify listeners.  TODO(pkasting): Why
   // is this order important?
   PendingInfobarRequests requests_to_notify;
   PendingInfobarRequests infobars_to_remove;
+  std::vector<PendingInfobarRequests::iterator> pending_requests_to_remove;
   for (PendingInfobarRequests::iterator i = pending_infobar_requests_.begin();
-       i != pending_infobar_requests_.end(); ) {
-    if (i->IsForPair(requesting_frame, embedder)) {
-      requests_to_notify.push_back(*i);
-      if (i->id().Equals(id)) {
-        // The infobar that called us is i->infobar(), and its delegate is
-        // currently in either Accept() or Cancel(). This means that
-        // RemoveInfoBar() will be called later on, and that will trigger a
-        // notification we're observing.
-        ++i;
-      } else if (i->has_infobar()) {
-        // This infobar is for the same frame/embedder pair, but in a different
-        // tab. We should remove it now that we've got an answer for it.
-        infobars_to_remove.push_back(*i);
-        ++i;
-      } else {
-        // We haven't created an infobar yet, just remove the pending request.
-        i = pending_infobar_requests_.erase(i);
-      }
+       i != pending_infobar_requests_.end(); ++i) {
+    if (!i->IsForPair(requesting_frame, embedder))
+      continue;

[xhwang, 2014/06/11 00:17:40]

You can have here:

if (!i->has_infobar()) {
  pending_requests_to_remove.push_back(i);
  continue;
}

[qinmin, 2014/06/11 00:42:35]

Done.

+    requests_to_notify.push_back(*i);
+    if (i->id().Equals(id) && i->has_infobar()) {
+      // The infobar that called us is i->infobar(), and its delegate is
+      // currently in either Accept() or Cancel(). This means that
+      // RemoveInfoBar() will be called later on, and that will trigger a
+      // notification we're observing.
+      continue;
+    }
+    if (i->has_infobar()) {
+      // This infobar is for the same frame/embedder pair, but in a different
+      // tab. We should remove it now that we've got an answer for it.
+      infobars_to_remove.push_back(*i);
     } else {
-      ++i;
+      // We haven't created an infobar yet, just record the pending request
+      // index and remove it later.
+      pending_requests_to_remove.push_back(i);
     }
   }
 
   // Remove all infobars for the same |requesting_frame| and |embedder|.
   for (PendingInfobarRequests::iterator i = infobars_to_remove.begin();
        i != infobars_to_remove.end(); ++i)
     GetInfoBarService(i->id())->RemoveInfoBar(i->infobar());
 
   // Send out the permission notifications.
   for (PendingInfobarRequests::iterator i = requests_to_notify.begin();
        i != requests_to_notify.end(); ++i)
     i->RunCallback(allowed);
+
+  // Remove the pending requests in reverse order.
+  for (int i = pending_requests_to_remove.size() - 1; i >= 0; --i)
+    pending_infobar_requests_.erase(pending_requests_to_remove[i]);
 }
 
 void PermissionQueueController::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   DCHECK_EQ(chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED, type);
   // We will receive this notification for all infobar closures, so we need to
   // check whether this is the geolocation infobar we're tracking. Note that the
   // InfoBarContainer (if any) may have received this notification before us and
@@ skipping 107 matching lines @@
 
   ContentSetting content_setting =
       allowed ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
   profile_->GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::FromURLNoWildcard(requesting_frame.GetOrigin()),
       ContentSettingsPattern::FromURLNoWildcard(embedder.GetOrigin()),
       type_,
       std::string(),
       content_setting);
 }