ScreenCapture for Android phase1, part II

chrome/browser/media/permission_bubble_media_access_handler.cc

Index: chrome/browser/media/permission_bubble_media_access_handler.cc
diff --git a/chrome/browser/media/permission_bubble_media_access_handler.cc b/chrome/browser/media/permission_bubble_media_access_handler.cc
index 4d2f7019f85d78f77070cf0875755cd086c66147..2de9fe557bd45c60d9b17f2e64c8730ff22fa4b3 100644
--- a/chrome/browser/media/permission_bubble_media_access_handler.cc
+++ b/chrome/browser/media/permission_bubble_media_access_handler.cc
@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/command_line.h"
 #include "base/metrics/field_trial.h"
 #include "chrome/browser/media/media_permission.h"
 #include "chrome/browser/media/media_stream_device_permissions.h"
@@ -18,6 +19,7 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/content_switches.h"
 
 #if BUILDFLAG(ANDROID_JAVA_UI)
 #include <vector>
@@ -77,8 +79,14 @@ PermissionBubbleMediaAccessHandler::~PermissionBubbleMediaAccessHandler() {
 bool PermissionBubbleMediaAccessHandler::SupportsStreamType(
     const content::MediaStreamType type,
     const extensions::Extension* extension) {
+#if BUILDFLAG(ANDROID_JAVA_UI)
+  return type == content::MEDIA_DEVICE_VIDEO_CAPTURE ||
+         type == content::MEDIA_DEVICE_AUDIO_CAPTURE ||
+         type == content::MEDIA_DESKTOP_VIDEO_CAPTURE;
+#else
   return type == content::MEDIA_DEVICE_VIDEO_CAPTURE ||
          type == content::MEDIA_DEVICE_AUDIO_CAPTURE;
+#endif
 }
 
 bool PermissionBubbleMediaAccessHandler::CheckMediaAccessPermission(
@@ -106,6 +114,18 @@ void PermissionBubbleMediaAccessHandler::HandleRequest(
     const extensions::Extension* extension) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
+#if BUILDFLAG(ANDROID_JAVA_UI)

[gone, 2016/08/10 19:04:03]

Why are you using ANDROID_JAVA_UI instead of OS_ANDROID?

[braveyao, 2016/08/12 23:37:44]

It's because only ANDROID_JAVA_UI is used in this file. 
Should I replace all of them with OS_ANDROID?

+  if (request.video_type == content::MEDIA_DESKTOP_VIDEO_CAPTURE &&
+      !base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableUserMediaScreenCapturing)) {
+    // If screen capturing isn't enabled on Android, we'll use "invalid state"
+    // as result, same as on desktop.
+    callback.Run(content::MediaStreamDevices(),
+                 content::MEDIA_DEVICE_INVALID_STATE, nullptr);
+    return;
+  }
+#endif
+
   RequestsQueue& queue = pending_requests_[web_contents];
   queue.push_back(PendingAccessRequest(request, callback));
 
@@ -134,7 +154,8 @@ void PermissionBubbleMediaAccessHandler::ProcessQueuedAccessRequest(
           base::Bind(
               &PermissionBubbleMediaAccessHandler::OnAccessRequestResponse,
               base::Unretained(this), web_contents)));
-  if (!controller->IsAskingForAudio() && !controller->IsAskingForVideo()) {
+  if (!controller->IsAskingForAudio() && !controller->IsAskingForVideo() &&
+      !controller->IsAskingForScreenCapture()) {

[gone, 2016/08/10 19:04:03]

Why is this new check necessary?  Nothing below (not even the comment) changed.

[braveyao, 2016/08/12 23:37:44]

It's necessary. Otherwise a screen capture will satisfy "if
(!controller->IsAskingForAudio() && !controller->IsAskingForVideo())" and hit
the 'return' in the if section. Then no infobar will be created.

 #if BUILDFLAG(ANDROID_JAVA_UI)
     // If either audio or video was previously allowed and Chrome no longer has
     // the necessary permissions, show a infobar to attempt to address this