TabSpecificContentSettings no longer knows about MediaStreamDeviceController

chrome/browser/media/media_stream_devices_controller.cc

 // Copyright (c) 2012 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/media/media_stream_devices_controller.h"
 
 #include "base/metrics/histogram.h"
 #include "base/prefs/scoped_user_pref_update.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
@@ skipping 66 matching lines @@
 }
 
 enum DevicePermissionActions {
   kAllowHttps = 0,
   kAllowHttp,
   kDeny,
   kCancel,
   kPermissionActionsMax  // Must always be last!
 };
 
+// This is a wrapper around the call to
+// TabSpecificContentSettings::OnMediaStreamPermissionSet, precomputing the
+// information from |request_permissions| to a form which is understood by
+// TabSpecificContentSettings.
+void OnMediaStreamPermissionSet(
+    TabSpecificContentSettings* content_settings,
+    content::WebContents* web_contents,
+    const GURL& request_origin,
+    const MediaStreamDevicesController::MediaStreamTypeSettingsMap&
+        request_permissions) {
+  TabSpecificContentSettings::MicrophoneCameraState microphone_camera_state =
+      TabSpecificContentSettings::MICROPHONE_CAMERA_NOT_ACCESSED;
+  std::string selected_audio_device;
+  std::string selected_video_device;
+  std::string requested_audio_device;
+  std::string requested_video_device;
+
+  PrefService* prefs = Profile::FromBrowserContext(
+                           web_contents->GetBrowserContext())->GetPrefs();

[no longer working on chromium, 2014/10/29 10:09:42]

indentation.

[vabr (Chromium), 2014/10/29 10:30:21]

I believe this is indented correctly.
See "Arguments may optionally all be placed on subsequent lines with a four
space indent" in
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Calls.

Also, this is the result of "git cl format".

[no longer working on chromium, 2014/10/30 09:59:19]

In some cases git cl format is not inline with the chromium style, though most
of them are nits.
The existing chromium code usually goes for four indentation in the new line,
you can find lots of examples, ex:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/his...

As they are nits, probably they don't matter in most of the cases. But I have to
point out that the script is not that smart, I have been running into cases that
the script is wrong. The idea is that we would like to use one style instead of
two. That says, I would be happy to adapt to clang-format when there is a
refactoring doing automated formatting on the existing code.

[vabr (Chromium), 2014/10/30 10:28:39]

My point here was that this particular formatting was inline with chromium
style, as documented by the link in my original comment.
This is true, and because of the "be consistent" clause, and out of respect to
OWNERS view :), I followed your advice here.
Done.
Exactly! :) And because the style guide is often left intentionally relaxed,
permitting different styles, it would be convenient to have a canonical format
provided by an automated tool, as long as the tool does not violate the style
guide.

BTW., if you see the script violate the style guide, that's a bug and should be
reported.
This sounds very reasonable. Until then, I'm happy to stick to the consistency
rule.

+  MediaStreamDevicesController::MediaStreamTypeSettingsMap::const_iterator it =

[Bernhard Bauer, 2014/10/29 09:35:17]

This seems like a good place for auto :)

[vabr (Chromium), 2014/10/29 10:30:21]

Auto-awesome! :)

Done.

+      request_permissions.find(content::MEDIA_DEVICE_AUDIO_CAPTURE);
+  if (it != request_permissions.end()) {
+    requested_audio_device = it->second.requested_device_id;
+    selected_audio_device =

[no longer working on chromium, 2014/10/29 10:09:42]

I think the correct format should be 
selected_audio_device = requested_audio_device.empty() ?
    prefs->GetString(prefs::kDefaultAudioCaptureDevice) :
    requested_audio_device;

[vabr (Chromium), 2014/10/29 10:30:21]

I'm yielding, because of consistency with the rest of the file.
But please note that clang-format will keep trying to change that, during all
future refactorings involving automated formatting.

Since the style guide does not seem to indicate what is the preferred way of
wrapping for the ternary operator (while it says the binary operators should go
before line breaks), it might be worth considering sticking to the clang-format
to keep indenting to automated tools.

Done.

+        requested_audio_device.empty()
+            ? prefs->GetString(prefs::kDefaultAudioCaptureDevice)
+            : requested_audio_device;
+    DCHECK_NE(MediaStreamDevicesController::MEDIA_NONE, it->second.permission);
+    bool mic_allowed =
+        it->second.permission == MediaStreamDevicesController::MEDIA_ALLOWED;
+    microphone_camera_state |=
+        TabSpecificContentSettings::MICROPHONE_ACCESSED |
+        (mic_allowed ? 0 : TabSpecificContentSettings::MICROPHONE_BLOCKED);
+  }
+
+  it = request_permissions.find(content::MEDIA_DEVICE_VIDEO_CAPTURE);
+  if (it != request_permissions.end()) {
+    requested_video_device = it->second.requested_device_id;
+    selected_video_device =

[no longer working on chromium, 2014/10/29 10:09:42]

ditto

[vabr (Chromium), 2014/10/29 10:30:21]

Done.

+        requested_video_device.empty()
+            ? prefs->GetString(prefs::kDefaultVideoCaptureDevice)
+            : requested_video_device;
+    DCHECK_NE(MediaStreamDevicesController::MEDIA_NONE, it->second.permission);
+    bool cam_allowed =
+        it->second.permission == MediaStreamDevicesController::MEDIA_ALLOWED;
+    microphone_camera_state |=
+        TabSpecificContentSettings::CAMERA_ACCESSED |
+        (cam_allowed ? 0 : TabSpecificContentSettings::CAMERA_BLOCKED);
+  }
+
+  content_settings->OnMediaStreamPermissionSet(request_origin,
+                                               microphone_camera_state,
+                                               selected_audio_device,
+                                               selected_video_device,
+                                               requested_audio_device,
+                                               requested_video_device);
+}
+
 }  // namespace
 
 MediaStreamDevicesController::MediaStreamTypeSettings::MediaStreamTypeSettings(
     Permission permission, const std::string& requested_device_id):
     permission(permission), requested_device_id(requested_device_id) {}
 
 MediaStreamDevicesController::MediaStreamTypeSettings::
     MediaStreamTypeSettings(): permission(MEDIA_NONE) {}
 
 MediaStreamDevicesController::MediaStreamTypeSettings::
@@ skipping 475 matching lines @@
         CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA,
         std::string(),
         content_setting);
   }
 }
 
 void MediaStreamDevicesController::NotifyUIRequestAccepted() const {
   if (!content_settings_)
     return;
 
-  content_settings_->OnMediaStreamPermissionSet(request_.security_origin,
-                                                request_permissions_);
+  OnMediaStreamPermissionSet(content_settings_,
+                             web_contents_,
+                             request_.security_origin,
+                             request_permissions_);
 }
 
 void MediaStreamDevicesController::NotifyUIRequestDenied() {
   if (!content_settings_)
     return;
 
   if (IsDeviceAudioCaptureRequestedAndAllowed()) {
     request_permissions_[content::MEDIA_DEVICE_AUDIO_CAPTURE].permission =
         MEDIA_BLOCKED_BY_USER;
   }
   if (IsDeviceVideoCaptureRequestedAndAllowed()) {
     request_permissions_[content::MEDIA_DEVICE_VIDEO_CAPTURE].permission =
         MEDIA_BLOCKED_BY_USER;
   }
 
-  content_settings_->OnMediaStreamPermissionSet(request_.security_origin,
-                                                request_permissions_);
+  OnMediaStreamPermissionSet(content_settings_,
+                             web_contents_,
+                             request_.security_origin,
+                             request_permissions_);
 }
 
 bool MediaStreamDevicesController::IsDeviceAudioCaptureRequestedAndAllowed()
     const {
   MediaStreamTypeSettingsMap::const_iterator it =
       request_permissions_.find(content::MEDIA_DEVICE_AUDIO_CAPTURE);
   return (it != request_permissions_.end() && IsCaptureDeviceRequestAllowed() &&
           it->second.permission == MEDIA_ALLOWED);
 }
 
 bool MediaStreamDevicesController::IsDeviceVideoCaptureRequestedAndAllowed()
     const {
   MediaStreamTypeSettingsMap::const_iterator it =
       request_permissions_.find(content::MEDIA_DEVICE_VIDEO_CAPTURE);
   return (it != request_permissions_.end() && IsCaptureDeviceRequestAllowed() &&
           it->second.permission == MEDIA_ALLOWED);
 }
 
 bool MediaStreamDevicesController::IsCaptureDeviceRequestAllowed() const {
 #if defined(OS_ANDROID)
   // Don't approve device requests if the tab was hidden.
   // TODO(qinmin): Add a test for this. http://crbug.com/396869.
   return web_contents_->GetRenderWidgetHostView()->IsShowing();
 #endif
   return true;
 }