[Media Router] Remove desktop capture prompts. (issue 2400803003 by mfoltz@chromium.org)
55 views
Skip to first unread message
mfo...@chromium.org
Oct 7, 2016, 3:21:45 PM10/7/16
to m...@chromium.org, ser...@chromium.org, chromium...@chromium.org, feature-me...@chromium.org, mcasas+...@chromium.org, miu+...@chromium.org
Reviewers: miu, Sergey Ulanov
CL: https://codereview.chromium.org/2400803003/
Description:
Updates desktop_capture_access_handler to bypass prompts for external
components (including the Media Router) when the capture id is obtained
through the capture picker.
It also attempts to simplify the logic for determining which desktop capture
requests are default approved.
Finally it updates desktop capture resource strings to remove references to
the Google Cast extension.
BUG=567443
Affected files (+53, -27 lines):
M chrome/app/generated_resources.grd
M chrome/browser/media/webrtc/desktop_capture_access_handler.h
M chrome/browser/media/webrtc/desktop_capture_access_handler.cc
Index: chrome/app/generated_resources.grd
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index c559dc1c5b036a597e16dce5f42191e935f65da6..2eaaae5d5a689f2fcedaf40f26e35373b8e4b91c 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -9114,34 +9114,34 @@ I don't think this site should be blocked!
Do you want <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> to share your screen and audio output?
</message>
<message name="IDS_MEDIA_SCREEN_CAPTURE_NOTIFICATION_TEXT" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by the same app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing your screen.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing your screen.
</message>
<message name="IDS_MEDIA_SCREEN_CAPTURE_NOTIFICATION_TEXT_DELEGATED" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by a tab instead of the app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing your screen with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing your screen with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
</message>
<message name="IDS_MEDIA_SCREEN_CAPTURE_WITH_AUDIO_NOTIFICATION_TEXT" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by the same app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing your screen and audio.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing your screen and audio.
</message>
<message name="IDS_MEDIA_SCREEN_CAPTURE_WITH_AUDIO_NOTIFICATION_TEXT_DELEGATED" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by a tab instead of the app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing your screen and audio with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing your screen and audio with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
</message>
<message name="IDS_MEDIA_WINDOW_CAPTURE_NOTIFICATION_TEXT" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by the same app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing a window.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing a window.
</message>
<message name="IDS_MEDIA_WINDOW_CAPTURE_NOTIFICATION_TEXT_DELEGATED" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by a tab instead of the app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing a window with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing a window with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
</message>
<message name="IDS_MEDIA_TAB_CAPTURE_NOTIFICATION_TEXT" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by the same app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing a Chrome tab.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing a Chrome tab.
</message>
<message name="IDS_MEDIA_TAB_CAPTURE_NOTIFICATION_TEXT_DELEGATED" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by a tab instead of the app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing a Chrome tab with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing a Chrome tab with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
</message>
<message name="IDS_MEDIA_TAB_CAPTURE_WITH_AUDIO_NOTIFICATION_TEXT" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by the same app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing a Chrome tab and audio.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing a Chrome tab and audio.
</message>
<message name="IDS_MEDIA_TAB_CAPTURE_WITH_AUDIO_NOTIFICATION_TEXT_DELEGATED" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by a tab instead of the app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing a Chrome tab and audio with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing a Chrome tab and audio with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
</message>
<message name="IDS_MEDIA_SCREEN_CAPTURE_NOTIFICATION_STOP" desc="Text shown on the button that stops screen capture.">
Stop sharing
Index: chrome/browser/media/webrtc/desktop_capture_access_handler.cc
diff --git a/chrome/browser/media/webrtc/desktop_capture_access_handler.cc b/chrome/browser/media/webrtc/desktop_capture_access_handler.cc
index 557e24d610a8a481e6e64d3f6433caea4f2a3331..5c3d2e0ac963c5c76a6811aeadddbb5d5bde49f7 100644
--- a/chrome/browser/media/webrtc/desktop_capture_access_handler.cc
+++ b/chrome/browser/media/webrtc/desktop_capture_access_handler.cc
@@ -62,6 +62,15 @@ base::string16 GetApplicationTitle(content::WebContents* web_contents,
return base::UTF8ToUTF16(title);
}
+// Returns whether an on-screen notification should appear after desktop capture
+// is approved for |extension|. Component extensions do not display a
+// notification.
+bool ShouldDisplayNotification(const extensions::Extension* extension) {
+ return !(extension &&
+ (extension->location() == extensions::Manifest::COMPONENT ||
+ extension->location() == extensions::Manifest::EXTERNAL_COMPONENT));
+}
+
base::string16 GetStopSharingUIString(
const base::string16& application_title,
const base::string16& registered_extension_name,
@@ -147,6 +156,13 @@ std::unique_ptr<content::MediaStreamUI> GetDevicesForDesktopCapture(
DCHECK_CURRENTLY_ON(BrowserThread::UI);
std::unique_ptr<content::MediaStreamUI> ui;
+ DVLOG(2) << __FUNCTION__ << ": media_id " << media_id.ToString()
+ << ", capture_audio " << capture_audio << ", mute_system_audio "
+ << mute_system_audio << ", display_notification "
+ << display_notification << ", application_title "
+ << application_title << ", extension_name "
+ << registered_extension_name;
+
// Add selected desktop source to the list.
devices->push_back(content::MediaStreamDevice(
content::MEDIA_DESKTOP_VIDEO_CAPTURE, media_id.ToString(), "Screen"));
@@ -230,10 +246,6 @@ void DesktopCaptureAccessHandler::ProcessScreenCaptureAccessRequest(
loopback_audio_supported = true;
#endif
- bool component_extension = false;
- component_extension =
- extension && extension->location() == extensions::Manifest::COMPONENT;
-
bool screen_capture_enabled =
base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableUserMediaScreenCapturing) ||
@@ -274,12 +286,9 @@ void DesktopCaptureAccessHandler::ProcessScreenCaptureAccessRequest(
#endif
web_contents = NULL;
- bool whitelisted_extension =
- IsExtensionWhitelistedForScreenCapture(extension);
-
- // For whitelisted or component extensions, bypass message box.
- bool user_approved = false;
- if (!whitelisted_extension && !component_extension) {
+ // Some extensions do not require user approval.
+ bool is_approved = IsDefaultApproved(extension);
+ if (!is_approved) {
base::string16 application_name =
base::UTF8ToUTF16(request.security_origin.spec());
if (extension)
@@ -294,10 +303,10 @@ void DesktopCaptureAccessHandler::ProcessScreenCaptureAccessRequest(
l10n_util::GetStringFUTF16(
IDS_MEDIA_SCREEN_CAPTURE_CONFIRMATION_TITLE, application_name),
confirmation_text);
- user_approved = (result == chrome::MESSAGE_BOX_RESULT_YES);
+ is_approved = (result == chrome::MESSAGE_BOX_RESULT_YES);
}
- if (user_approved || component_extension || whitelisted_extension) {
+ if (is_approved) {
content::DesktopMediaID screen_id;
#if defined(OS_CHROMEOS)
screen_id = content::DesktopMediaID::RegisterAuraWindow(
@@ -312,9 +321,8 @@ void DesktopCaptureAccessHandler::ProcessScreenCaptureAccessRequest(
(request.audio_type == content::MEDIA_DESKTOP_AUDIO_CAPTURE &&
loopback_audio_supported);
- // Unless we're being invoked from a component extension, register to
- // display the notification for stream capture.
- bool display_notification = !component_extension;
+ // Determine if the extension is required to display a notification.
+ const bool display_notification = ShouldDisplayNotification(extension);
ui = GetDevicesForDesktopCapture(&devices, screen_id, capture_audio, true,
display_notification, application_title,
@@ -331,6 +339,14 @@ void DesktopCaptureAccessHandler::ProcessScreenCaptureAccessRequest(
callback.Run(devices, result, std::move(ui));
}
+bool DesktopCaptureAccessHandler::IsDefaultApproved(
+ const extensions::Extension* extension) {
+ return extension &&
+ (extension->location() == extensions::Manifest::COMPONENT ||
+ extension->location() == extensions::Manifest::EXTERNAL_COMPONENT ||
+ IsExtensionWhitelistedForScreenCapture(extension));
+}
+
bool DesktopCaptureAccessHandler::SupportsStreamType(
const content::MediaStreamType type,
const extensions::Extension* extension) {
@@ -425,11 +441,13 @@ void DesktopCaptureAccessHandler::HandleRequest(
(check_audio_permission ? audio_permitted : true) && audio_requested &&
audio_supported;
+ // Determine if the extension is required to display a notification.
+ const bool display_notification = ShouldDisplayNotification(extension);
+
ui = GetDevicesForDesktopCapture(&devices, media_id, capture_audio, false,
- true,
+ display_notification,
GetApplicationTitle(web_contents, extension),
base::UTF8ToUTF16(original_extension_name));
UpdateExtensionTrusted(request, extension);
callback.Run(devices, content::MEDIA_DEVICE_OK, std::move(ui));
}
-
Index: chrome/browser/media/webrtc/desktop_capture_access_handler.h
diff --git a/chrome/browser/media/webrtc/desktop_capture_access_handler.h b/chrome/browser/media/webrtc/desktop_capture_access_handler.h
index c7b58e1a7a16248541aace29cb6d57de11599aae..7a87b3ed39389630dfb1f233e94264a9a32c7bf2 100644
--- a/chrome/browser/media/webrtc/desktop_capture_access_handler.h
+++ b/chrome/browser/media/webrtc/desktop_capture_access_handler.h
@@ -10,6 +10,10 @@
#include "chrome/browser/media/capture_access_handler_base.h"
#include "chrome/browser/media/media_access_handler.h"
+namespace extensions {
+class Extension;
+}
+
// MediaAccessHandler for DesktopCapture API.
class DesktopCaptureAccessHandler : public CaptureAccessHandlerBase {
public:
@@ -36,6 +40,10 @@ class DesktopCaptureAccessHandler : public CaptureAccessHandlerBase {
const content::MediaResponseCallback& callback,
const extensions::Extension* extension);
+ // Returns whether desktop capture is always approved for |extension|.
+ // Currently component extensions and some whitelisted extensions are default
+ // approved.
+ bool IsDefaultApproved(const extensions::Extension* extension);
};
#endif // CHROME_BROWSER_MEDIA_WEBRTC_DESKTOP_CAPTURE_ACCESS_HANDLER_H_
CL: https://codereview.chromium.org/2400803003/
Description:
Updates desktop_capture_access_handler to bypass prompts for external
components (including the Media Router) when the capture id is obtained
through the capture picker.
It also attempts to simplify the logic for determining which desktop capture
requests are default approved.
Finally it updates desktop capture resource strings to remove references to
the Google Cast extension.
BUG=567443
Affected files (+53, -27 lines):
M chrome/app/generated_resources.grd
M chrome/browser/media/webrtc/desktop_capture_access_handler.h
M chrome/browser/media/webrtc/desktop_capture_access_handler.cc
Index: chrome/app/generated_resources.grd
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index c559dc1c5b036a597e16dce5f42191e935f65da6..2eaaae5d5a689f2fcedaf40f26e35373b8e4b91c 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -9114,34 +9114,34 @@ I don't think this site should be blocked!
Do you want <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> to share your screen and audio output?
</message>
<message name="IDS_MEDIA_SCREEN_CAPTURE_NOTIFICATION_TEXT" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by the same app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing your screen.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing your screen.
</message>
<message name="IDS_MEDIA_SCREEN_CAPTURE_NOTIFICATION_TEXT_DELEGATED" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by a tab instead of the app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing your screen with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing your screen with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
</message>
<message name="IDS_MEDIA_SCREEN_CAPTURE_WITH_AUDIO_NOTIFICATION_TEXT" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by the same app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing your screen and audio.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing your screen and audio.
</message>
<message name="IDS_MEDIA_SCREEN_CAPTURE_WITH_AUDIO_NOTIFICATION_TEXT_DELEGATED" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by a tab instead of the app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing your screen and audio with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing your screen and audio with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
</message>
<message name="IDS_MEDIA_WINDOW_CAPTURE_NOTIFICATION_TEXT" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by the same app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing a window.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing a window.
</message>
<message name="IDS_MEDIA_WINDOW_CAPTURE_NOTIFICATION_TEXT_DELEGATED" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by a tab instead of the app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing a window with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing a window with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
</message>
<message name="IDS_MEDIA_TAB_CAPTURE_NOTIFICATION_TEXT" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by the same app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing a Chrome tab.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing a Chrome tab.
</message>
<message name="IDS_MEDIA_TAB_CAPTURE_NOTIFICATION_TEXT_DELEGATED" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by a tab instead of the app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing a Chrome tab with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing a Chrome tab with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
</message>
<message name="IDS_MEDIA_TAB_CAPTURE_WITH_AUDIO_NOTIFICATION_TEXT" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by the same app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing a Chrome tab and audio.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing a Chrome tab and audio.
</message>
<message name="IDS_MEDIA_TAB_CAPTURE_WITH_AUDIO_NOTIFICATION_TEXT_DELEGATED" desc="label used in screen capture notification UI to show the screen sharing status when the stream is consumed by a tab instead of the app initiating the window picker">
- <ph name="APP_NAME">$1<ex>Google Cast</ex></ph> is sharing a Chrome tab and audio with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
+ <ph name="APP_NAME">$1<ex>html5rocks.com</ex></ph> is sharing a Chrome tab and audio with <ph name="TAB_NAME">$2<ex>https://google.com</ex></ph>.
</message>
<message name="IDS_MEDIA_SCREEN_CAPTURE_NOTIFICATION_STOP" desc="Text shown on the button that stops screen capture.">
Stop sharing
Index: chrome/browser/media/webrtc/desktop_capture_access_handler.cc
diff --git a/chrome/browser/media/webrtc/desktop_capture_access_handler.cc b/chrome/browser/media/webrtc/desktop_capture_access_handler.cc
index 557e24d610a8a481e6e64d3f6433caea4f2a3331..5c3d2e0ac963c5c76a6811aeadddbb5d5bde49f7 100644
--- a/chrome/browser/media/webrtc/desktop_capture_access_handler.cc
+++ b/chrome/browser/media/webrtc/desktop_capture_access_handler.cc
@@ -62,6 +62,15 @@ base::string16 GetApplicationTitle(content::WebContents* web_contents,
return base::UTF8ToUTF16(title);
}
+// Returns whether an on-screen notification should appear after desktop capture
+// is approved for |extension|. Component extensions do not display a
+// notification.
+bool ShouldDisplayNotification(const extensions::Extension* extension) {
+ return !(extension &&
+ (extension->location() == extensions::Manifest::COMPONENT ||
+ extension->location() == extensions::Manifest::EXTERNAL_COMPONENT));
+}
+
base::string16 GetStopSharingUIString(
const base::string16& application_title,
const base::string16& registered_extension_name,
@@ -147,6 +156,13 @@ std::unique_ptr<content::MediaStreamUI> GetDevicesForDesktopCapture(
DCHECK_CURRENTLY_ON(BrowserThread::UI);
std::unique_ptr<content::MediaStreamUI> ui;
+ DVLOG(2) << __FUNCTION__ << ": media_id " << media_id.ToString()
+ << ", capture_audio " << capture_audio << ", mute_system_audio "
+ << mute_system_audio << ", display_notification "
+ << display_notification << ", application_title "
+ << application_title << ", extension_name "
+ << registered_extension_name;
+
// Add selected desktop source to the list.
devices->push_back(content::MediaStreamDevice(
content::MEDIA_DESKTOP_VIDEO_CAPTURE, media_id.ToString(), "Screen"));
@@ -230,10 +246,6 @@ void DesktopCaptureAccessHandler::ProcessScreenCaptureAccessRequest(
loopback_audio_supported = true;
#endif
- bool component_extension = false;
- component_extension =
- extension && extension->location() == extensions::Manifest::COMPONENT;
-
bool screen_capture_enabled =
base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableUserMediaScreenCapturing) ||
@@ -274,12 +286,9 @@ void DesktopCaptureAccessHandler::ProcessScreenCaptureAccessRequest(
#endif
web_contents = NULL;
- bool whitelisted_extension =
- IsExtensionWhitelistedForScreenCapture(extension);
-
- // For whitelisted or component extensions, bypass message box.
- bool user_approved = false;
- if (!whitelisted_extension && !component_extension) {
+ // Some extensions do not require user approval.
+ bool is_approved = IsDefaultApproved(extension);
+ if (!is_approved) {
base::string16 application_name =
base::UTF8ToUTF16(request.security_origin.spec());
if (extension)
@@ -294,10 +303,10 @@ void DesktopCaptureAccessHandler::ProcessScreenCaptureAccessRequest(
l10n_util::GetStringFUTF16(
IDS_MEDIA_SCREEN_CAPTURE_CONFIRMATION_TITLE, application_name),
confirmation_text);
- user_approved = (result == chrome::MESSAGE_BOX_RESULT_YES);
+ is_approved = (result == chrome::MESSAGE_BOX_RESULT_YES);
}
- if (user_approved || component_extension || whitelisted_extension) {
+ if (is_approved) {
content::DesktopMediaID screen_id;
#if defined(OS_CHROMEOS)
screen_id = content::DesktopMediaID::RegisterAuraWindow(
@@ -312,9 +321,8 @@ void DesktopCaptureAccessHandler::ProcessScreenCaptureAccessRequest(
(request.audio_type == content::MEDIA_DESKTOP_AUDIO_CAPTURE &&
loopback_audio_supported);
- // Unless we're being invoked from a component extension, register to
- // display the notification for stream capture.
- bool display_notification = !component_extension;
+ // Determine if the extension is required to display a notification.
+ const bool display_notification = ShouldDisplayNotification(extension);
ui = GetDevicesForDesktopCapture(&devices, screen_id, capture_audio, true,
display_notification, application_title,
@@ -331,6 +339,14 @@ void DesktopCaptureAccessHandler::ProcessScreenCaptureAccessRequest(
callback.Run(devices, result, std::move(ui));
}
+bool DesktopCaptureAccessHandler::IsDefaultApproved(
+ const extensions::Extension* extension) {
+ return extension &&
+ (extension->location() == extensions::Manifest::COMPONENT ||
+ extension->location() == extensions::Manifest::EXTERNAL_COMPONENT ||
+ IsExtensionWhitelistedForScreenCapture(extension));
+}
+
bool DesktopCaptureAccessHandler::SupportsStreamType(
const content::MediaStreamType type,
const extensions::Extension* extension) {
@@ -425,11 +441,13 @@ void DesktopCaptureAccessHandler::HandleRequest(
(check_audio_permission ? audio_permitted : true) && audio_requested &&
audio_supported;
+ // Determine if the extension is required to display a notification.
+ const bool display_notification = ShouldDisplayNotification(extension);
+
ui = GetDevicesForDesktopCapture(&devices, media_id, capture_audio, false,
- true,
+ display_notification,
GetApplicationTitle(web_contents, extension),
base::UTF8ToUTF16(original_extension_name));
UpdateExtensionTrusted(request, extension);
callback.Run(devices, content::MEDIA_DEVICE_OK, std::move(ui));
}
-
Index: chrome/browser/media/webrtc/desktop_capture_access_handler.h
diff --git a/chrome/browser/media/webrtc/desktop_capture_access_handler.h b/chrome/browser/media/webrtc/desktop_capture_access_handler.h
index c7b58e1a7a16248541aace29cb6d57de11599aae..7a87b3ed39389630dfb1f233e94264a9a32c7bf2 100644
--- a/chrome/browser/media/webrtc/desktop_capture_access_handler.h
+++ b/chrome/browser/media/webrtc/desktop_capture_access_handler.h
@@ -10,6 +10,10 @@
#include "chrome/browser/media/capture_access_handler_base.h"
#include "chrome/browser/media/media_access_handler.h"
+namespace extensions {
+class Extension;
+}
+
// MediaAccessHandler for DesktopCapture API.
class DesktopCaptureAccessHandler : public CaptureAccessHandlerBase {
public:
@@ -36,6 +40,10 @@ class DesktopCaptureAccessHandler : public CaptureAccessHandlerBase {
const content::MediaResponseCallback& callback,
const extensions::Extension* extension);
+ // Returns whether desktop capture is always approved for |extension|.
+ // Currently component extensions and some whitelisted extensions are default
+ // approved.
+ bool IsDefaultApproved(const extensions::Extension* extension);
};
#endif // CHROME_BROWSER_MEDIA_WEBRTC_DESKTOP_CAPTURE_ACCESS_HANDLER_H_
m...@chromium.org
Oct 8, 2016, 11:00:09 PM10/8/16
to mfo...@chromium.org, ser...@chromium.org, chromium...@chromium.org, feature-me...@chromium.org, mcasas+...@chromium.org, miu+...@chromium.org
lgtm
https://codereview.chromium.org/2400803003/diff/40001/chrome/browser/media/webrtc/desktop_capture_access_handler.cc
File chrome/browser/media/webrtc/desktop_capture_access_handler.cc
(right):
https://codereview.chromium.org/2400803003/diff/40001/chrome/browser/media/webrtc/desktop_capture_access_handler.cc#newcode289
chrome/browser/media/webrtc/desktop_capture_access_handler.cc:289: //
https://codereview.chromium.org/2400803003/diff/40001/chrome/browser/media/webrtc/desktop_capture_access_handler.cc
File chrome/browser/media/webrtc/desktop_capture_access_handler.cc
(right):
https://codereview.chromium.org/2400803003/diff/40001/chrome/browser/media/webrtc/desktop_capture_access_handler.cc#newcode289
chrome/browser/media/webrtc/desktop_capture_access_handler.cc:289: //
Some extensions do not require user approval.
Consider adding: "because they already provide the user-approval UI."
https://codereview.chromium.org/2400803003/
https://codereview.chromium.org/2400803003/
ser...@chromium.org
Oct 10, 2016, 1:56:16 PM10/10/16
to mfo...@chromium.org, m...@chromium.org, chromium...@chromium.org, feature-me...@chromium.org, mcasas+...@chromium.org, miu+...@chromium.org
lgtm when my comment is addressed
https://codereview.chromium.org/2400803003/diff/40001/chrome/browser/media/webrtc/desktop_capture_access_handler.cc
File chrome/browser/media/webrtc/desktop_capture_access_handler.cc
(right):
https://codereview.chromium.org/2400803003/diff/40001/chrome/browser/media/webrtc/desktop_capture_access_handler.cc#newcode342
chrome/browser/media/webrtc/desktop_capture_access_handler.cc:342: bool
DesktopCaptureAccessHandler::IsDefaultApproved(
does this need to be a class method instead of just a static function?
https://codereview.chromium.org/2400803003/
chrome/browser/media/webrtc/desktop_capture_access_handler.cc:342: bool
DesktopCaptureAccessHandler::IsDefaultApproved(
does this need to be a class method instead of just a static function?
https://codereview.chromium.org/2400803003/
mfo...@chromium.org
Oct 11, 2016, 12:14:21 PM10/11/16
to m...@chromium.org, ser...@chromium.org, chromium...@chromium.org, feature-me...@chromium.org, mcasas+...@chromium.org, miu+...@chromium.org
On 2016/10/09 at 03:00:09, miu wrote:
> lgtm
> lgtm
>
>
https://codereview.chromium.org/2400803003/diff/40001/chrome/browser/media/webrtc/desktop_capture_access_handler.cc
> File chrome/browser/media/webrtc/desktop_capture_access_handler.cc (right):
>
>
https://codereview.chromium.org/2400803003/diff/40001/chrome/browser/media/webrtc/desktop_capture_access_handler.cc#newcode289
> chrome/browser/media/webrtc/desktop_capture_access_handler.cc:289: // Some
extensions do not require user approval.
> Consider adding: "because they already provide the user-approval UI."
> chrome/browser/media/webrtc/desktop_capture_access_handler.cc:289: // Some
extensions do not require user approval.
> Consider adding: "because they already provide the user-approval UI."
mfo...@chromium.org
Oct 11, 2016, 12:15:01 PM10/11/16
to m...@chromium.org, ser...@chromium.org, chromium...@chromium.org, feature-me...@chromium.org, mcasas+...@chromium.org, miu+...@chromium.org
On 2016/10/10 at 17:56:16, sergeyu wrote:
> lgtm when my comment is addressed
>
>
> lgtm when my comment is addressed
>
>
https://codereview.chromium.org/2400803003/diff/40001/chrome/browser/media/webrtc/desktop_capture_access_handler.cc
> File chrome/browser/media/webrtc/desktop_capture_access_handler.cc (right):
>
>
> File chrome/browser/media/webrtc/desktop_capture_access_handler.cc (right):
>
>
https://codereview.chromium.org/2400803003/diff/40001/chrome/browser/media/webrtc/desktop_capture_access_handler.cc#newcode342
> chrome/browser/media/webrtc/desktop_capture_access_handler.cc:342: bool
DesktopCaptureAccessHandler::IsDefaultApproved(
> does this need to be a class method instead of just a static function?
> chrome/browser/media/webrtc/desktop_capture_access_handler.cc:342: bool
DesktopCaptureAccessHandler::IsDefaultApproved(
> does this need to be a class method instead of just a static function?
It calls a static protected method of the base class; converted to static.
https://codereview.chromium.org/2400803003/
https://codereview.chromium.org/2400803003/
commit-bot@chromium.org via codereview.chromium.org
Oct 11, 2016, 12:55:23 PM10/11/16
to mfo...@chromium.org, m...@chromium.org, ser...@chromium.org, commi...@chromium.org, chromium...@chromium.org, feature-me...@chromium.org, mcasas+...@chromium.org, miu+...@chromium.org
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2400803003/80001
https://codereview.chromium.org/2400803003/
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2400803003/80001
https://codereview.chromium.org/2400803003/
commit-bot@chromium.org via codereview.chromium.org
Oct 11, 2016, 1:43:08 PM10/11/16
to mfo...@chromium.org, m...@chromium.org, ser...@chromium.org, commi...@chromium.org, chromium...@chromium.org, feature-me...@chromium.org, mcasas+...@chromium.org, miu+...@chromium.org
commit-bot@chromium.org via codereview.chromium.org
Oct 11, 2016, 1:46:15 PM10/11/16
to mfo...@chromium.org, m...@chromium.org, ser...@chromium.org, commi...@chromium.org, chromium...@chromium.org, feature-me...@chromium.org, mcasas+...@chromium.org, miu+...@chromium.org
Patchset 5 (id:??) landed as
https://crrev.com/382a7a5e909abcfd3e57f8a5176c2c38e69aa983
Cr-Commit-Position: refs/heads/master@{#424476}
https://codereview.chromium.org/2400803003/
https://crrev.com/382a7a5e909abcfd3e57f8a5176c2c38e69aa983
Cr-Commit-Position: refs/heads/master@{#424476}
https://codereview.chromium.org/2400803003/
Reply all
Reply to author
Forward
0 new messages