Remove MediaSettingChangedInfobar and show latest state in bubble

chrome/browser/content_settings/tab_specific_content_settings.cc

Index: chrome/browser/content_settings/tab_specific_content_settings.cc
diff --git a/chrome/browser/content_settings/tab_specific_content_settings.cc b/chrome/browser/content_settings/tab_specific_content_settings.cc
index cca7a210a7b9bd774c3790967c6ee6ea54d43c80..78b4cdcf17bc0df14091ffae2a0d17436232d564 100644
--- a/chrome/browser/content_settings/tab_specific_content_settings.cc
+++ b/chrome/browser/content_settings/tab_specific_content_settings.cc
@@ -8,6 +8,7 @@
 
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
+#include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browsing_data/browsing_data_appcache_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_cookie_helper.h"
@@ -23,6 +24,7 @@
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_controller.h"
@@ -71,6 +73,7 @@ TabSpecificContentSettings::TabSpecificContentSettings(WebContents* tab)
       previous_protocol_handler_(ProtocolHandler::EmptyProtocolHandler()),
       pending_protocol_handler_setting_(CONTENT_SETTING_DEFAULT),
       load_plugins_link_enabled_(true),
+      microphone_camera_state_(MICROPHONE_CAMERA_NOT_ACCESSED),
       observer_(this) {
   ClearBlockedContentSettingsExceptForCookies();
   ClearCookieSpecificContentSettings();
@@ -272,20 +275,21 @@ bool TabSpecificContentSettings::IsContentAllowed(
 void TabSpecificContentSettings::OnContentBlocked(ContentSettingsType type) {
   DCHECK(type != CONTENT_SETTINGS_TYPE_GEOLOCATION)
       << "Geolocation settings handled by OnGeolocationPermissionSet";
+  DCHECK(type != CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC &&
+         type != CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA)
+      << "Media stream settings handled by OnMediaStreamPermissionSet";
   if (type < 0 || type >= CONTENT_SETTINGS_NUM_TYPES)
     return;
 
-  // Media is different from other content setting types since it allows new
-  // setting to kick in without reloading the page, and the UI for media is
-  // always reflecting the newest permission setting.
   switch (type) {
-    case CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC:
-    case CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA:
 #if defined(OS_ANDROID)
+    // Media is different from other content setting types since it allows new
+    // setting to kick in without reloading the page, and the UI for media is

[Peter Kasting, 2014/09/22 23:11:22]

Nit: setting -> settings

"the UI for media" is a bit unclear as well.

[robwu, 2014/09/24 00:03:04]

I have just deleted this comment since it is no longer relevant.
It was added in https://codereview.chromium.org/13375004/patch/6001/7001.
The comment is certainly not relevant to the remaining media setting
(CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER) because there is no "reload
page" UI for it (the setting was added in
https://codereview.chromium.org/download/issue23531021_266001.diff)

[Peter Kasting, 2014/09/24 01:33:00]

OK, but in that case, as a reader I'm still going to wonder why this particular
type is special and needs its own handling, so there needs to be some ind of
explanation here and below.

[robwu, 2014/09/24 23:38:44]

According to IsContentAllowed, this field is only significant for a handful of
types. CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER is not among them.

content_allowed_[] is also used internally within
TabSpecificContentSettings::OnContentAllowed. If it is false, then the flag is
flipped to true and a change notification is triggered. By setting
content_allowed_[] to true in OnContentBlocked, the notification is suppressed.
HOWEVER, the OnContentAllowed method also has a special case for
CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER, where access_blocked_[] is
used to check whether the permissions has changed.

I have clarified the latter in a comment in OnContentAllowed, and added a
comment about the questionable use of content_allowed_. I don't want to make any
more logical changes to this part of the code, because the change/bugfix is not
related to this patch. If what I found turns out to be a real bug, then I will
address that in a separate patch.

+    // always reflecting the newest permission setting.

[Peter Kasting, 2014/09/22 23:11:22]

Nit: is always reflecting -> always reflects

     case CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER:
-#endif
       content_allowed_[type] = false;
       break;
+#endif
     default:
       content_allowed_[type] = true;
       break;
@@ -315,13 +319,13 @@ void TabSpecificContentSettings::OnContentBlocked(ContentSettingsType type) {
 void TabSpecificContentSettings::OnContentAllowed(ContentSettingsType type) {
   DCHECK(type != CONTENT_SETTINGS_TYPE_GEOLOCATION)
       << "Geolocation settings handled by OnGeolocationPermissionSet";
+  DCHECK(type != CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC &&
+         type != CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA)
+      << "Media stream settings handled by OnMediaStreamPermissionSet";
   bool access_changed = false;
   switch (type) {
-    case CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC:
-    case CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA:
 #if defined(OS_ANDROID)
     case CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER:
-#endif
       // The setting for media is overwritten here because media does not need
       // to reload the page to have the new setting kick in. See issue/175993.
       if (content_blocked_[type]) {
@@ -329,6 +333,7 @@ void TabSpecificContentSettings::OnContentAllowed(ContentSettingsType type) {
         access_changed = true;
       }
       break;
+#endif
     default:
       break;
   }
@@ -478,25 +483,30 @@ void TabSpecificContentSettings::OnProtectedMediaIdentifierPermissionSet(
 
 TabSpecificContentSettings::MicrophoneCameraState
 TabSpecificContentSettings::GetMicrophoneCameraState() const {
-  if (IsContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC) &&
-      IsContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA)) {
-    return MICROPHONE_CAMERA_ACCESSED;
-  } else if (IsContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC)) {
-    return MICROPHONE_ACCESSED;
-  } else if (IsContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA)) {
-    return CAMERA_ACCESSED;
-  }
-
-  if (IsContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC) &&
-      IsContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA)) {
-    return MICROPHONE_CAMERA_BLOCKED;
-  } else if (IsContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC)) {
-    return MICROPHONE_BLOCKED;
-  } else if (IsContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA)) {
-    return CAMERA_BLOCKED;
+  return microphone_camera_state_;
+}
+
+bool TabSpecificContentSettings::IsMicrophoneCameraStateChanged() const {
+  switch (microphone_camera_state_) {
+    case MICROPHONE_CAMERA_ACCESSED:
+      return !IsContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC) ||
+          !IsContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA);
+    case MICROPHONE_ACCESSED:
+      return !IsContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC);
+    case CAMERA_ACCESSED:
+      return !IsContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA);
+    case MICROPHONE_CAMERA_BLOCKED:
+      return !IsContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC) ||
+          !IsContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA);
+    case MICROPHONE_BLOCKED:
+      return !IsContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC);
+    case CAMERA_BLOCKED:
+      return !IsContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA);
+    case MICROPHONE_CAMERA_NOT_ACCESSED:
+      return false;
+    default:
+      NOTREACHED();
   }
-
-  return MICROPHONE_CAMERA_NOT_ACCESSED;
 }
 
 void TabSpecificContentSettings::OnMediaStreamPermissionSet(
@@ -505,22 +515,31 @@ void TabSpecificContentSettings::OnMediaStreamPermissionSet(
         request_permissions) {
   media_stream_access_origin_ = request_origin;
 
+  PrefService* prefs =
+      Profile::FromBrowserContext(web_contents()->GetBrowserContext())->
+      GetPrefs();
   MediaStreamDevicesController::MediaStreamTypeSettingsMap::const_iterator it =
       request_permissions.find(content::MEDIA_DEVICE_AUDIO_CAPTURE);
   if (it != request_permissions.end()) {
     media_stream_requested_audio_device_ = it->second.requested_device_id;
+    media_stream_selected_audio_device_ =
+        media_stream_requested_audio_device_.empty() ?
+        prefs->GetString(prefs::kDefaultAudioCaptureDevice) :

[Peter Kasting, 2014/09/22 23:11:22]

Nit: I'd indent this line and the next 4 more  (2 places)

[robwu, 2014/09/24 00:03:04]

Done.

+        media_stream_requested_audio_device_;
     switch (it->second.permission) {

[Peter Kasting, 2014/09/22 23:11:23]

Nit: Shorter:

  DCHECK_NE(MediaStreamDevicesController::MEDIA_NONE, it->second.permission);
  const bool mic_allowed =
      it->second.permission == MediaStreamDevicesController::MEDIA_ALLOWED;
  content_allowed_[CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC] = mic_allowed;
  content_blocked_[CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC] = !mic_allowed;

(Same comment below)

[robwu, 2014/09/24 00:03:04]

Done.

       case MediaStreamDevicesController::MEDIA_NONE:
         NOTREACHED();
         break;
       case MediaStreamDevicesController::MEDIA_ALLOWED:
-        OnContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC);
+        content_allowed_[CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC] = true;
+        content_blocked_[CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC] = false;
         break;
       // TODO(grunell): UI should show for what reason access has been blocked.
       case MediaStreamDevicesController::MEDIA_BLOCKED_BY_POLICY:
       case MediaStreamDevicesController::MEDIA_BLOCKED_BY_USER_SETTING:
       case MediaStreamDevicesController::MEDIA_BLOCKED_BY_USER:
-        OnContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC);
+        content_allowed_[CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC] = false;
+        content_blocked_[CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC] = true;
         break;
     }
   }
@@ -528,21 +547,51 @@ void TabSpecificContentSettings::OnMediaStreamPermissionSet(
   it = request_permissions.find(content::MEDIA_DEVICE_VIDEO_CAPTURE);
   if (it != request_permissions.end()) {
     media_stream_requested_video_device_ = it->second.requested_device_id;
+    media_stream_selected_video_device_ =
+        media_stream_requested_video_device_.empty() ?
+        prefs->GetString(prefs::kDefaultVideoCaptureDevice) :
+        media_stream_requested_video_device_;
     switch (it->second.permission) {
       case MediaStreamDevicesController::MEDIA_NONE:
         NOTREACHED();
         break;
       case MediaStreamDevicesController::MEDIA_ALLOWED:
-        OnContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA);
+        content_allowed_[CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA] = true;
+        content_blocked_[CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA] = false;
         break;
       // TODO(grunell): UI should show for what reason access has been blocked.
       case MediaStreamDevicesController::MEDIA_BLOCKED_BY_POLICY:
       case MediaStreamDevicesController::MEDIA_BLOCKED_BY_USER_SETTING:
       case MediaStreamDevicesController::MEDIA_BLOCKED_BY_USER:
-        OnContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA);
+        content_allowed_[CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA] = false;
+        content_blocked_[CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA] = true;
         break;
     }
   }
+
+  MicrophoneCameraState prev_microphone_camera_state = microphone_camera_state_;
+  if (IsContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC) &&

[Peter Kasting, 2014/09/22 23:11:23]

Nit: Slightly shorter and does fewer redundant checks:

  const ContentSettingsType camera = CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA;
  if (IsContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC))
    microphone_camera_state_ = IsContentAllowed(camera) ?
        MICROPHONE_CAMERA_ACCESSED : MICROPHONE_ACCESSED;
  } else if (IsContentAllowed(camera)) {
    microphone_camera_state_ = CAMERA_ACCESSED;
  } else if (IsContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC) {
    microphone_camera_state_ = IsContentBlocked(camera) ?
      MICROPHONE_CAMERA_BLOCKED : MICROPHONE_BLOCKED;
  } else if (IsContentBlocked(camera)) {
    microphone_camera_state_ = CAMERA_BLOCKED;
  }

[robwu, 2014/09/24 00:03:04]

Done.

+      IsContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA)) {
+    microphone_camera_state_ = MICROPHONE_CAMERA_ACCESSED;
+  } else if (IsContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC)) {
+    microphone_camera_state_ = MICROPHONE_ACCESSED;
+  } else if (IsContentAllowed(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA)) {
+    microphone_camera_state_ = CAMERA_ACCESSED;
+  } else if (IsContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC) &&
+             IsContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA)) {
+    microphone_camera_state_ = MICROPHONE_CAMERA_BLOCKED;
+  } else if (IsContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC)) {
+    microphone_camera_state_ = MICROPHONE_BLOCKED;
+  } else if (IsContentBlocked(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA)) {
+    microphone_camera_state_ = CAMERA_BLOCKED;
+  }
+
+  if (microphone_camera_state_ != prev_microphone_camera_state) {
+    content::NotificationService::current()->Notify(
+        chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
+        content::Source<WebContents>(web_contents()),
+        content::NotificationService::NoDetails());
+  }
 }
 
 void TabSpecificContentSettings::OnMidiSysExAccessed(
@@ -565,6 +614,7 @@ void TabSpecificContentSettings::ClearBlockedContentSettingsExceptForCookies() {
     content_allowed_[i] = false;
     content_blockage_indicated_to_user_[i] = false;
   }
+  microphone_camera_state_ = MICROPHONE_CAMERA_NOT_ACCESSED;
   load_plugins_link_enabled_ = true;
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
@@ -707,9 +757,30 @@ void TabSpecificContentSettings::OnContentSettingChanged(
       details.primary_pattern().Matches(entry_url)) {
     Profile* profile =
         Profile::FromBrowserContext(web_contents()->GetBrowserContext());
+    const HostContentSettingsMap* map = profile->GetHostContentSettingsMap();
+
+    if (content_type == CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC ||
+        content_type == CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA) {
+      const GURL media_origin = media_stream_access_origin();
+      ContentSetting setting = map->GetContentSetting(media_origin,
+                                                      media_origin,
+                                                      content_type,
+                                                      std::string());
+      if (setting == CONTENT_SETTING_BLOCK) {

[Peter Kasting, 2014/09/22 23:11:22]

Nit: Shorter:

      content_allowed_[content_type] = setting == CONTENT_SETTING_ALLOW;
      content_blocked_[content_type] = setting == CONTENT_SETTING_BLOCK;
      DCHECK(content_allowed_[content_type] || content_blocked_[content_type] ||
          (setting == CONTENT_SETTING_ASK));

(The final DCHECK is optional, I don't know whether it's really critical)

[robwu, 2014/09/24 00:03:04]

Done.

+        content_allowed_[content_type] = false;
+        content_blocked_[content_type] = true;
+      } else if (setting == CONTENT_SETTING_ALLOW) {
+        content_allowed_[content_type] = true;
+        content_blocked_[content_type] = false;
+      } else if (setting == CONTENT_SETTING_ASK) {
+        content_allowed_[content_type] = false;
+        content_blocked_[content_type] = false;
+      } else {
+        NOTREACHED();
+      }
+    }
     RendererContentSettingRules rules;
-    GetRendererContentSettingRules(profile->GetHostContentSettingsMap(),
-                                   &rules);
+    GetRendererContentSettingRules(map, &rules);
     Send(new ChromeViewMsg_SetContentSettingRules(rules));
   }
 }