Dismiss EME infobar when WebMediaPlayer is destroyed.

chrome/browser/content_settings/permission_queue_controller.cc

Index: chrome/browser/content_settings/permission_queue_controller.cc
diff --git a/chrome/browser/content_settings/permission_queue_controller.cc b/chrome/browser/content_settings/permission_queue_controller.cc
index 480f6a78cdd460252c6a04cc7b1bb79d7297c399..5ec64f78662b9093167d2f40472e7ea1d7849378 100644
--- a/chrome/browser/content_settings/permission_queue_controller.cc
+++ b/chrome/browser/content_settings/permission_queue_controller.cc
@@ -50,8 +50,8 @@ class PermissionQueueController::PendingInfoBarRequest {
 
   const PermissionRequestID& id() const { return id_; }
   const GURL& requesting_frame() const { return requesting_frame_; }
-  bool has_infobar() const { return !!infobar_; }
-  InfoBar* infobar() { return infobar_; }
+  bool has_infobar() const { return !!info_bar_; }

[ddorwin, 2014/01/07 19:07:14]

Why the change to a two-part name? See comment below.

[Kibeom Kim (inactive), 2014/01/07 19:31:24]

(previous comments:
https://codereview.chromium.org/107413006/diff/400001/chrome/browser/content_...
)

Changed back to infobar_ and also s/PendingInfoBarRequest/PendingInfobarRequest

+  InfoBar* infobar() { return info_bar_; }
 
   void RunCallback(bool allowed);
   void CreateInfoBar(PermissionQueueController* controller,
@@ -63,7 +63,7 @@ class PermissionQueueController::PendingInfoBarRequest {
   GURL requesting_frame_;
   GURL embedder_;
   PermissionDecidedCallback callback_;
-  InfoBar* infobar_;
+  InfoBar* info_bar_;
 
   // Purposefully do not disable copying, as this is stored in STL containers.
 };
@@ -79,7 +79,7 @@ PermissionQueueController::PendingInfoBarRequest::PendingInfoBarRequest(
       requesting_frame_(requesting_frame),
       embedder_(embedder),
       callback_(callback),
-      infobar_(NULL) {
+      info_bar_(NULL) {
 }
 
 PermissionQueueController::PendingInfoBarRequest::~PendingInfoBarRequest() {
@@ -104,18 +104,18 @@ void PermissionQueueController::PendingInfoBarRequest::CreateInfoBar(
   // http://crbug.com/266743
   switch (type_) {
     case CONTENT_SETTINGS_TYPE_GEOLOCATION:
-      infobar_ = GeolocationInfoBarDelegate::Create(
+      info_bar_ = GeolocationInfoBarDelegate::Create(
           GetInfoBarService(id_), controller, id_, requesting_frame_,
           display_languages);
       break;
     case CONTENT_SETTINGS_TYPE_MIDI_SYSEX:
-      infobar_ = MIDIPermissionInfoBarDelegate::Create(
+      info_bar_ = MIDIPermissionInfoBarDelegate::Create(
           GetInfoBarService(id_), controller, id_, requesting_frame_,
           display_languages);
       break;
 #if defined(OS_ANDROID)
     case CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER:
-      infobar_ = ProtectedMediaIdentifierInfoBarDelegate::Create(
+      info_bar_ = ProtectedMediaIdentifierInfoBarDelegate::Create(
           GetInfoBarService(id_), controller, id_, requesting_frame_,
           display_languages);
       break;
@@ -137,8 +137,8 @@ PermissionQueueController::PermissionQueueController(Profile* profile,
 PermissionQueueController::~PermissionQueueController() {
   // Cancel all outstanding requests.
   in_shutdown_ = true;
-  while (!pending_infobar_requests_.empty())
-    CancelInfoBarRequest(pending_infobar_requests_.front().id());
+  while (!pending_info_bar_requests_.empty())
+    CancelInfoBarRequest(pending_info_bar_requests_.front().id());
 }
 
 void PermissionQueueController::CreateInfoBarRequest(
@@ -150,11 +150,11 @@ void PermissionQueueController::CreateInfoBarRequest(
 
   // We shouldn't get duplicate requests.
   for (PendingInfoBarRequests::const_iterator i(
-           pending_infobar_requests_.begin());
-       i != pending_infobar_requests_.end(); ++i)
+           pending_info_bar_requests_.begin());
+       i != pending_info_bar_requests_.end(); ++i)
     DCHECK(!i->id().Equals(id));
 
-  pending_infobar_requests_.push_back(PendingInfoBarRequest(
+  pending_info_bar_requests_.push_back(PendingInfoBarRequest(
       type_, id, requesting_frame, embedder, callback));
   if (!AlreadyShowingInfoBarForTab(id))
     ShowQueuedInfoBarForTab(id);
@@ -164,18 +164,47 @@ void PermissionQueueController::CancelInfoBarRequest(
     const PermissionRequestID& id) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
-  for (PendingInfoBarRequests::iterator i(pending_infobar_requests_.begin());
-       i != pending_infobar_requests_.end(); ++i) {
+  for (PendingInfoBarRequests::iterator i(pending_info_bar_requests_.begin());
+       i != pending_info_bar_requests_.end(); ++i) {
     if (i->id().Equals(id)) {
       if (i->has_infobar())
         GetInfoBarService(id)->RemoveInfoBar(i->infobar());
       else
-        pending_infobar_requests_.erase(i);
+        pending_info_bar_requests_.erase(i);
       return;
     }
   }
 }
 
+void PermissionQueueController::CancelInfoBarRequests(int group_id) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  // If we remove an info bar in the following loop, the next pending info bar

[ddorwin, 2014/01/07 19:07:14]

nit: "info bar" is inconsistent with description and used only 57 times compared
to 863 in the source.

[Kibeom Kim (inactive), 2014/01/07 19:31:24]

Done.

+  // will be shown. Therefore, we erase all the pending info bars first and
+  // remove an info bar later.
+  PendingInfoBarRequests info_bar_requests_to_cancel;
+  for (PendingInfoBarRequests::iterator i = pending_info_bar_requests_.begin();
+       i != pending_info_bar_requests_.end();) {
+    if (i->id().group_id() == group_id) {
+      if (i->has_infobar()) {
+        // |i| will be erased from |pending_info_bar_requests_|
+        // in |PermissionQueueController::Observe| when the info bar is removed.
+        info_bar_requests_to_cancel.push_back(*i);
+        ++i;
+      } else {
+        i = pending_info_bar_requests_.erase(i);
+      }
+    } else {
+      ++i;
+    }
+  }
+
+  for (PendingInfoBarRequests::iterator i = info_bar_requests_to_cancel.begin();
+       i != info_bar_requests_to_cancel.end();
+       ++i)

[markusheintz_, 2014/01/07 12:22:45]

nit: I know there exist several opinions on this, but I prefer having { } around
the body of the for loop because the condition spans a cross several lines.

[Kibeom Kim (inactive), 2014/01/07 19:31:24]

Done.

+    GetInfoBarService(i->id())->RemoveInfoBar(i->infobar());
+}
+
 void PermissionQueueController::OnPermissionSet(
     const PermissionRequestID& id,
     const GURL& requesting_frame,
@@ -190,9 +219,9 @@ void PermissionQueueController::OnPermissionSet(
   // Cancel this request first, then notify listeners.  TODO(pkasting): Why
   // is this order important?
   PendingInfoBarRequests requests_to_notify;
-  PendingInfoBarRequests infobars_to_remove;
-  for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
-       i != pending_infobar_requests_.end(); ) {
+  PendingInfoBarRequests info_bars_to_remove;
+  for (PendingInfoBarRequests::iterator i = pending_info_bar_requests_.begin();
+       i != pending_info_bar_requests_.end(); ) {
     if (i->IsForPair(requesting_frame, embedder)) {
       requests_to_notify.push_back(*i);
       if (i->id().Equals(id)) {
@@ -204,11 +233,11 @@ void PermissionQueueController::OnPermissionSet(
       } 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);
+        info_bars_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_info_bar_requests_.erase(i);
       }
     } else {
       ++i;
@@ -216,8 +245,8 @@ void PermissionQueueController::OnPermissionSet(
   }
 
   // Remove all infobars for the same |requesting_frame| and |embedder|.
-  for (PendingInfoBarRequests::iterator i = infobars_to_remove.begin();
-       i != infobars_to_remove.end(); ++i)
+  for (PendingInfoBarRequests::iterator i = info_bars_to_remove.begin();
+       i != info_bars_to_remove.end(); ++i)
     GetInfoBarService(i->id())->RemoveInfoBar(i->infobar());
 
   // Send out the permission notifications.
@@ -240,12 +269,12 @@ void PermissionQueueController::Observe(
   // pending_infobar_requests_ will not have received any new entries between
   // the NotificationService's call to InfoBarContainer::Observe and this
   // method.
-  InfoBar* infobar = content::Details<InfoBar::RemovedDetails>(details)->first;
-  for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
-       i != pending_infobar_requests_.end(); ++i) {
-    if (i->infobar() == infobar) {
+  InfoBar* info_bar = content::Details<InfoBar::RemovedDetails>(details)->first;
+  for (PendingInfoBarRequests::iterator i = pending_info_bar_requests_.begin();
+       i != pending_info_bar_requests_.end(); ++i) {
+    if (i->infobar() == info_bar) {
       PermissionRequestID id(i->id());
-      pending_infobar_requests_.erase(i);
+      pending_info_bar_requests_.erase(i);
       ShowQueuedInfoBarForTab(id);
       return;
     }
@@ -255,8 +284,8 @@ void PermissionQueueController::Observe(
 bool PermissionQueueController::AlreadyShowingInfoBarForTab(
     const PermissionRequestID& id) const {
   for (PendingInfoBarRequests::const_iterator i(
-           pending_infobar_requests_.begin());
-       i != pending_infobar_requests_.end(); ++i) {
+           pending_info_bar_requests_.begin());
+       i != pending_info_bar_requests_.end(); ++i) {
     if (i->id().IsForSameTabAs(id) && i->has_infobar())
       return true;
   }
@@ -276,32 +305,32 @@ void PermissionQueueController::ShowQueuedInfoBarForTab(
   //
   // Similarly, if we're being destroyed, we should also avoid showing further
   // infobars.
-  InfoBarService* infobar_service = GetInfoBarService(id);
-  if (!infobar_service || in_shutdown_) {
+  InfoBarService* info_bar_service = GetInfoBarService(id);
+  if (!info_bar_service || in_shutdown_) {
     ClearPendingInfoBarRequestsForTab(id);
     return;
   }
 
-  for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
-       i != pending_infobar_requests_.end(); ++i) {
+  for (PendingInfoBarRequests::iterator i = pending_info_bar_requests_.begin();
+       i != pending_info_bar_requests_.end(); ++i) {
     if (i->id().IsForSameTabAs(id) && !i->has_infobar()) {
-      RegisterForInfoBarNotifications(infobar_service);
+      RegisterForInfoBarNotifications(info_bar_service);
       i->CreateInfoBar(
           this, profile_->GetPrefs()->GetString(prefs::kAcceptLanguages));
       return;
     }
   }
 
-  UnregisterForInfoBarNotifications(infobar_service);
+  UnregisterForInfoBarNotifications(info_bar_service);
 }
 
 void PermissionQueueController::ClearPendingInfoBarRequestsForTab(
     const PermissionRequestID& id) {
-  for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
-       i != pending_infobar_requests_.end(); ) {
+  for (PendingInfoBarRequests::iterator i = pending_info_bar_requests_.begin();
+       i != pending_info_bar_requests_.end(); ) {
     if (i->id().IsForSameTabAs(id)) {
       DCHECK(!i->has_infobar());
-      i = pending_infobar_requests_.erase(i);
+      i = pending_info_bar_requests_.erase(i);
     } else {
       ++i;
     }
@@ -309,24 +338,24 @@ void PermissionQueueController::ClearPendingInfoBarRequestsForTab(
 }
 
 void PermissionQueueController::RegisterForInfoBarNotifications(
-    InfoBarService* infobar_service) {
+    InfoBarService* info_bar_service) {
   if (!registrar_.IsRegistered(
       this, chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED,
-      content::Source<InfoBarService>(infobar_service))) {
+      content::Source<InfoBarService>(info_bar_service))) {
     registrar_.Add(this,
                    chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED,
-                   content::Source<InfoBarService>(infobar_service));
+                   content::Source<InfoBarService>(info_bar_service));
   }
 }
 
 void PermissionQueueController::UnregisterForInfoBarNotifications(
-    InfoBarService* infobar_service) {
+    InfoBarService* info_bar_service) {
   if (registrar_.IsRegistered(
       this, chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED,
-      content::Source<InfoBarService>(infobar_service))) {
+      content::Source<InfoBarService>(info_bar_service))) {
     registrar_.Remove(this,
                       chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED,
-                      content::Source<InfoBarService>(infobar_service));
+                      content::Source<InfoBarService>(info_bar_service));
   }
 }