ScreenCapture for Android phase1, part II

chrome/browser/media/media_stream_devices_controller.cc

Index: chrome/browser/media/media_stream_devices_controller.cc
diff --git a/chrome/browser/media/media_stream_devices_controller.cc b/chrome/browser/media/media_stream_devices_controller.cc
index 62144e7c308eaf60e18acc8b73213dcef83ba819..4426f18bbe9f66df954d75fb731b7190cb2bc1c8 100644
--- a/chrome/browser/media/media_stream_devices_controller.cc
+++ b/chrome/browser/media/media_stream_devices_controller.cc
@@ -15,6 +15,7 @@
 #include "base/values.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
+#include "chrome/browser/media/desktop_streams_registry.h"
 #include "chrome/browser/media/media_capture_devices_dispatcher.h"
 #include "chrome/browser/media/media_permission.h"
 #include "chrome/browser/media/media_stream_capture_indicator.h"
@@ -40,6 +41,7 @@
 #include "content/public/common/origin_util.h"
 #include "extensions/common/constants.h"
 #include "grit/theme_resources.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_capture_types.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if BUILDFLAG(ANDROID_JAVA_UI)
@@ -198,6 +200,11 @@ MediaStreamDevicesController::MediaStreamDevicesController(
           web_contents, content_settings_types)) {
     return;
   }
+
+  // Always show infobar for screen capture on Android.
+  if (request.video_type == content::MEDIA_DESKTOP_VIDEO_CAPTURE) {

[Sergey Ulanov, 2016/08/01 18:20:36]

nit: Other single-line if statements in this file don't have braces, remove them
here for consistency

[braveyao, 2016/08/03 00:23:47]

Done.

+    return;
+  }
 #endif
 
   // Otherwise we can run the callback immediately.
@@ -239,9 +246,15 @@ bool MediaStreamDevicesController::IsAskingForVideo() const {
   return old_video_setting_ == CONTENT_SETTING_ASK;
 }
 
+bool MediaStreamDevicesController::IsAskingForScreenCapture() const {
+  return request_.video_type == content::MEDIA_DESKTOP_VIDEO_CAPTURE;
+}
+
 base::string16 MediaStreamDevicesController::GetMessageText() const {
   int message_id = IDS_MEDIA_CAPTURE_AUDIO_AND_VIDEO;
-  if (!IsAskingForAudio())
+  if (IsAskingForScreenCapture())
+    message_id = IDS_MEDIA_SCREEN_CAPTURE_CONFIRMATION_TEXT;

[Sergey Ulanov, 2016/08/01 18:20:36]

please add braces here and for else-if cases below.

[braveyao, 2016/08/03 00:23:47]

Done.

+  else if (!IsAskingForAudio())
     message_id = IDS_MEDIA_CAPTURE_VIDEO_ONLY;
   else if (!IsAskingForVideo())
     message_id = IDS_MEDIA_CAPTURE_AUDIO_ONLY;
@@ -284,21 +297,31 @@ GURL MediaStreamDevicesController::GetOrigin() const {
 void MediaStreamDevicesController::PermissionGranted() {
   RecordPermissionAction(request_, profile_,
                          base::Bind(PermissionUmaUtil::PermissionGranted));
-  RunCallback(GetNewSetting(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC,
-                            old_audio_setting_, CONTENT_SETTING_ALLOW),
-              GetNewSetting(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA,
-                            old_video_setting_, CONTENT_SETTING_ALLOW),
-              content::MEDIA_DEVICE_PERMISSION_DENIED);
+  if (IsAskingForScreenCapture()) {
+    RunCallback(old_audio_setting_, old_video_setting_,
+                content::MEDIA_DEVICE_OK);
+  } else {
+    RunCallback(GetNewSetting(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC,
+                              old_audio_setting_, CONTENT_SETTING_ALLOW),
+                GetNewSetting(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA,
+                              old_video_setting_, CONTENT_SETTING_ALLOW),
+                content::MEDIA_DEVICE_PERMISSION_DENIED);
+  }
 }
 
 void MediaStreamDevicesController::PermissionDenied() {
   RecordPermissionAction(request_, profile_,
                          base::Bind(PermissionUmaUtil::PermissionDenied));
-  RunCallback(GetNewSetting(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC,
-                            old_audio_setting_, CONTENT_SETTING_BLOCK),
-              GetNewSetting(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA,
-                            old_video_setting_, CONTENT_SETTING_BLOCK),
-              content::MEDIA_DEVICE_PERMISSION_DENIED);
+  if (IsAskingForScreenCapture()) {
+    RunCallback(old_audio_setting_, old_video_setting_,
+                content::MEDIA_DEVICE_PERMISSION_DENIED);
+  } else {
+    RunCallback(GetNewSetting(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC,
+                              old_audio_setting_, CONTENT_SETTING_BLOCK),
+                GetNewSetting(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA,
+                              old_video_setting_, CONTENT_SETTING_BLOCK),
+                content::MEDIA_DEVICE_PERMISSION_DENIED);
+  }
 }
 
 void MediaStreamDevicesController::GroupedRequestFinished(bool audio_accepted,
@@ -468,8 +491,16 @@ void MediaStreamDevicesController::RunCallback(
   // If either audio or video are allowed then the callback should report
   // success, otherwise we report |denial_reason|.
   content::MediaStreamRequestResult request_result = content::MEDIA_DEVICE_OK;
-  if (audio_setting != CONTENT_SETTING_ALLOW &&
-      video_setting != CONTENT_SETTING_ALLOW) {
+  if (IsAskingForScreenCapture()) {
+    request_result = denial_reason;
+
+    content::DesktopMediaID screen_id;
+    screen_id = content::DesktopMediaID(content::DesktopMediaID::TYPE_SCREEN,
+                                        webrtc::kFullDesktopScreenId);
+    devices.push_back(content::MediaStreamDevice(

[Sergey Ulanov, 2016/08/01 18:20:36]

This reads as if we always allow screen capture, even when access was denied.
This looks looks wrong to me. 
Overall the logic in this function isn't very clear. Maybe update the comment
above?

[braveyao, 2016/08/03 00:23:47]

Done.

+        content::MEDIA_DESKTOP_VIDEO_CAPTURE, screen_id.ToString(), "Screen"));
+  } else if (audio_setting != CONTENT_SETTING_ALLOW &&
+             video_setting != CONTENT_SETTING_ALLOW) {
     DCHECK_NE(content::MEDIA_DEVICE_OK, denial_reason);
     request_result = denial_reason;
   } else if (devices.empty()) {