diff options
Diffstat (limited to 'chromium/chrome/browser/media')
48 files changed, 1236 insertions, 483 deletions
diff --git a/chromium/chrome/browser/media/feeds/media_feeds_store.mojom b/chromium/chrome/browser/media/feeds/media_feeds_store.mojom index ec77b81c049..bfb3795fb67 100644 --- a/chromium/chrome/browser/media/feeds/media_feeds_store.mojom +++ b/chromium/chrome/browser/media/feeds/media_feeds_store.mojom @@ -10,6 +10,7 @@ import "mojo/public/mojom/base/time.mojom"; import "url/mojom/origin.mojom"; import "ui/gfx/geometry/mojom/geometry.mojom"; import "url/mojom/url.mojom"; +import "mojo/public/mojom/base/unguessable_token.mojom"; struct MediaFeed { // The ID of the field in storage. @@ -67,13 +68,21 @@ struct MediaFeed { // have been deleted and the feed status has been reset to default. ResetReason reset_reason; - // An associated origin is an origin associated with the feed. If the cookies - // are deleted or expire on the origin then we will delete the feed items. - // https://wicg.github.io/media-feeds/#dfn-associated-origin - array<url.mojom.Origin> associated_origins; + // Token used to invalidate ongoing fetches for the feed. If there is a token + // mismatch when feed fetch data returns (the reset token when the fetch began + // does not match the feed's current reset token), the data is stale and + // shouldn't be stored because the feed has been reset. + mojo_base.mojom.UnguessableToken? reset_token; // Contains details about the user signed into the website. UserIdentifier? user_identifier; + + // If set then changes to the cookie name provided on the feed origin or any + // associated origin will trigger the feed to be reset. + string cookie_name_filter; + + // The result of safe search checking this media feed. + SafeSearchResult safe_search_result; }; // Contains details about the user signed into the website. @@ -345,10 +354,22 @@ struct MediaImage { array<ContentAttribute> content_attributes; }; +// Whether the feed item can be considered "family friendly". If this is not +// specified then this will be |kUnknown|. This enum is committed to storage +// so do not change the numbering. +enum IsFamilyFriendly { + kUnknown = 0, + kYes = 1, + kNo = 2, +}; + // The Media Feed Item is an individual item stored in a Media Feed. It // represents a single recommendation such as a video or TV show. // https://wicg.github.io/media-feeds/index.html#media-feed-item struct MediaFeedItem { + // The ID of the feed item in storage. + int64 id; + // The type of this feed item such as a video or TV show. MediaFeedItemType type; @@ -362,7 +383,7 @@ struct MediaFeedItem { mojo_base.mojom.Time date_published; // Whether the media item is considered "family friendly". - bool is_family_friendly; + IsFamilyFriendly is_family_friendly; // The action status for this feed item. MediaFeedItemActionStatus action_status; @@ -414,6 +435,12 @@ struct DebugInformation { // The value of the Media Feeds Safe Search checking consent pref. bool safe_search_pref_value; + + // Whether the MediaFeedsBackgroundFetching feature is enabled. + bool background_fetching_feature_enabled; + + // The value of the Media Feeds background fetching pref. + bool background_fetching_pref_value; }; // MediaFeedStore allows the Media Feeds WebUI to access data from the Media @@ -425,6 +452,9 @@ interface MediaFeedsStore { // Updates the Safe Search checking enabled pref to |value|. SetSafeSearchEnabledPref(bool value) => (); + // Updates the background fetching enabled pref to |value|. + SetBackgroundFetchingPref(bool value) => (); + // Gets all the discovered media feeds. GetMediaFeeds() => (array<MediaFeed> feeds); @@ -433,6 +463,7 @@ interface MediaFeedsStore { // Fetch the details of a previously-discovered media feed and store them in // media history. The feed ID is the unique ID for the feed from the Media - // History store. - FetchMediaFeed(int64 feed_id) => (); + // History store. The logs contain information about any problems fetching or + // parsing the feed. + FetchMediaFeed(int64 feed_id) => (string logs); }; diff --git a/chromium/chrome/browser/media/kaleidoscope/BUILD.gn b/chromium/chrome/browser/media/kaleidoscope/BUILD.gn new file mode 100644 index 00000000000..45446236046 --- /dev/null +++ b/chromium/chrome/browser/media/kaleidoscope/BUILD.gn @@ -0,0 +1,44 @@ +# Copyright 2020 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. + +import("//chrome/browser/buildflags.gni") +import("//tools/grit/grit_rule.gni") + +# If the ENABLE_KALEIDOSCOPE flag is enabled and src-internal is available then +# include the internal resources. Otherwise, empty resources are used. +if (enable_kaleidoscope) { + grit("kaleidoscope_resources") { + source = "kaleidoscope_internal_resources.grd" + outputs = [ + "grit/kaleidoscope_resources.h", + "kaleidoscope_resources.pak", + ] + grit_flags = [ + "-E", + "root_gen_dir=" + rebase_path(root_gen_dir, root_build_dir), + ] + deps = [ + "//chrome/browser/media/kaleidoscope/internal:kaleidoscope_strings", + "//chrome/browser/media/kaleidoscope/internal/resources:content", + "//chrome/browser/media/kaleidoscope/mojom:mojom_js", + "//url/mojom:url_mojom_gurl_js", + ] + } +} else { + grit("kaleidoscope_resources") { + source = "kaleidoscope_resources.grd" + outputs = [ + "grit/kaleidoscope_resources.h", + "kaleidoscope_resources.pak", + ] + grit_flags = [ + "-E", + "root_gen_dir=" + rebase_path(root_gen_dir, root_build_dir), + ] + deps = [ + "//chrome/browser/media/kaleidoscope/mojom:mojom_js", + "//url/mojom:url_mojom_gurl_js", + ] + } +} diff --git a/chromium/chrome/browser/media/kaleidoscope/kaleidoscope_internal_resources.grd b/chromium/chrome/browser/media/kaleidoscope/kaleidoscope_internal_resources.grd new file mode 100644 index 00000000000..8ca771da653 --- /dev/null +++ b/chromium/chrome/browser/media/kaleidoscope/kaleidoscope_internal_resources.grd @@ -0,0 +1,31 @@ +<?xml version="1.0" encoding="UTF-8"?> +<grit latest_public_release="0" current_release="1" output_all_resource_defines="false"> + <outputs> + <output filename="grit/kaleidoscope_resources.h" type="rc_header"> + <emit emit_type='prepend'></emit> + </output> + <output filename="kaleidoscope_resources.pak" type="data_package" /> + </outputs> + <release seq="1"> + <includes> + <include name="IDR_KALEIDOSCOPE_CONTENT_CSS" file="internal/resources/content.css" type="chrome_html" flattenhtml="true" compress="gzip" /> + <include name="IDR_KALEIDOSCOPE_CONTENT_HTML" file="internal/resources/content.html" type="BINDATA" compress="gzip" /> + <include name="IDR_KALEIDOSCOPE_CONTENT_JS" file="${root_gen_dir}/chrome/browser/media/kaleidoscope/internal/resources/ks-content.js" use_base_dir="false" type="BINDATA" compress="gzip" /> + <include name="IDR_KALEIDOSCOPE_HTML" file="internal/resources/kaleidoscope.html" type="BINDATA" compress="gzip" /> + <include name="IDR_KALEIDOSCOPE_JS" file="internal/resources/kaleidoscope.js" type="BINDATA" compress="gzip" /> + <include name="IDR_KALEIDOSCOPE_MESSAGES_JS" file="internal/resources/messages.js" type="BINDATA" compress="gzip" /> + <include name="IDR_KALEIDOSCOPE_UTILS_JS" file="internal/resources/utils.js" type="BINDATA" compress="gzip" /> + <include name="IDR_GEOMETRY_MOJOM_LITE_JS" file="${root_gen_dir}/ui/gfx/geometry/mojom/geometry.mojom-lite.js" use_base_dir="false" type="BINDATA" compress="gzip" /> + <include name="IDR_KALEIDOSCOPE_MOJOM_LITE_JS" file="${root_gen_dir}/chrome/browser/media/kaleidoscope/mojom/kaleidoscope.mojom-lite.js" use_base_dir="false" type="BINDATA" compress="gzip" /> + + <!-- Strings --> + <include name="IDR_KALEIDOSCOPE_LOCALE_EN" file="${root_gen_dir}/chrome/browser/media/kaleidoscope/internal/resources/_locales/en/messages.json" use_base_dir="false" type="BINDATA" compress="gzip" /> + + <!-- Google Sans --> + <include name="IDR_GOOGLE_SANS_CSS" file="internal/resources/fonts/fonts.css" type="BINDATA" compress="gzip" /> + <include name="IDR_GOOGLE_SANS_BOLD" file="internal/resources/fonts/GoogleSans-Bold.woff2" type="BINDATA" compress="gzip" /> + <include name="IDR_GOOGLE_SANS_MEDIUM" file="internal/resources/fonts/GoogleSans-Medium.woff2" type="BINDATA" compress="gzip" /> + <include name="IDR_GOOGLE_SANS_REGULAR" file="internal/resources/fonts/GoogleSans-Regular.woff2" type="BINDATA" compress="gzip" /> + </includes> + </release> +</grit> diff --git a/chromium/chrome/browser/media/kaleidoscope/kaleidoscope_resources.grd b/chromium/chrome/browser/media/kaleidoscope/kaleidoscope_resources.grd new file mode 100644 index 00000000000..72d8ab5339e --- /dev/null +++ b/chromium/chrome/browser/media/kaleidoscope/kaleidoscope_resources.grd @@ -0,0 +1,14 @@ +<?xml version="1.0" encoding="UTF-8"?> +<grit latest_public_release="0" current_release="1" output_all_resource_defines="false"> + <outputs> + <output filename="grit/kaleidoscope_resources.h" type="rc_header"> + <emit emit_type='prepend'></emit> + </output> + <output filename="kaleidoscope_resources.pak" type="data_package" /> + </outputs> + <release seq="1"> + <includes> + <!-- TODO: Add resources. --> + </includes> + </release> +</grit> diff --git a/chromium/chrome/browser/media/kaleidoscope/mojom/BUILD.gn b/chromium/chrome/browser/media/kaleidoscope/mojom/BUILD.gn new file mode 100644 index 00000000000..e3ce1eb00b5 --- /dev/null +++ b/chromium/chrome/browser/media/kaleidoscope/mojom/BUILD.gn @@ -0,0 +1,11 @@ +# Copyright 2019 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. + +import("//mojo/public/tools/bindings/mojom.gni") + +mojom("mojom") { + sources = [ "kaleidoscope.mojom" ] + + public_deps = [ "//chrome/browser/media/feeds:mojo_bindings" ] +} diff --git a/chromium/chrome/browser/media/kaleidoscope/mojom/kaleidoscope.mojom b/chromium/chrome/browser/media/kaleidoscope/mojom/kaleidoscope.mojom new file mode 100644 index 00000000000..779ef7edf33 --- /dev/null +++ b/chromium/chrome/browser/media/kaleidoscope/mojom/kaleidoscope.mojom @@ -0,0 +1,30 @@ +// Copyright 2019 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. + +module media.mojom; + +import "chrome/browser/media/feeds/media_feeds_store.mojom"; + +// The credentials required to make Google API calls from JS. +struct Credentials { + // Chrome's API Key. + string api_key; + + // An OAuth access token scoped to the Kaleidoscope API for the currently + // logged in user. If the user is not signed in then this will be empty. + string? access_token; +}; + +// Provides data for the kaleidoscope page. +interface KaleidoscopeDataProvider { + // Returns all the Media Feeds that Kaleidoscope might decide to show. + GetTopMediaFeeds() => (array<media_feeds.mojom.MediaFeed> feeds); + + // Returns all the items from a Media Feed that Kaleidoscope might decide to + // show. + GetMediaFeedContents(int64 feed_id) => (array<media_feeds.mojom.MediaFeedItem> items); + + // Retrieves the current credentials. + GetCredentials() => (Credentials credentials); +}; diff --git a/chromium/chrome/browser/media/kaleidoscope/test/proto/BUILD.gn b/chromium/chrome/browser/media/kaleidoscope/test/proto/BUILD.gn new file mode 100644 index 00000000000..48dedc2a124 --- /dev/null +++ b/chromium/chrome/browser/media/kaleidoscope/test/proto/BUILD.gn @@ -0,0 +1,18 @@ +# Copyright 2020 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. + +import("//third_party/closure_compiler/compile_js.gni") +import("//third_party/protobuf/proto_library.gni") + +proto_library("proto") { + sources = [ "test.proto" ] + + generate_javascript = true +} + +js_library("test") { + sources = [] + + deps = [ ":proto_js" ] +} diff --git a/chromium/chrome/browser/media/router/BUILD.gn b/chromium/chrome/browser/media/router/BUILD.gn index 0a680d40f51..c6cfa9f6ac5 100644 --- a/chromium/chrome/browser/media/router/BUILD.gn +++ b/chromium/chrome/browser/media/router/BUILD.gn @@ -88,6 +88,8 @@ static_library("router") { "//ui/base:buildflags", ] + public_deps += [ "//chrome/common/media_router/mojom:logger" ] + sources += [ "data_decoder_util.cc", "data_decoder_util.h", @@ -95,6 +97,8 @@ static_library("router") { "event_page_request_manager.h", "event_page_request_manager_factory.cc", "event_page_request_manager_factory.h", + "logger_impl.cc", + "logger_impl.h", "mojo/media_route_provider_util_win.cc", "mojo/media_route_provider_util_win.h", "mojo/media_router_desktop.cc", @@ -202,6 +206,8 @@ static_library("test_support") { "test/media_router_mojo_test.h", "test/mock_dns_sd_registry.cc", "test/mock_dns_sd_registry.h", + "test/mock_logger.cc", + "test/mock_logger.h", "test/mock_mojo_media_router.cc", "test/mock_mojo_media_router.h", "test/noop_dual_media_sink_service.cc", @@ -248,11 +254,14 @@ source_set("unittests") { "discovery/mdns/dns_sd_registry_unittest.cc", "discovery/media_sink_discovery_metrics_unittest.cc", "event_page_request_manager_unittest.cc", + "logger_impl_unittest.cc", "media_router_feature_unittest.cc", "mojo/media_router_desktop_unittest.cc", "mojo/media_router_mojo_impl_unittest.cc", "mojo/media_router_mojo_metrics_unittest.cc", "mojo/media_sink_service_status_unittest.cc", + "providers/cast/activity_record_test_base.cc", + "providers/cast/activity_record_test_base.h", "providers/cast/cast_activity_manager_unittest.cc", "providers/cast/cast_activity_record_unittest.cc", "providers/cast/cast_app_availability_tracker_unittest.cc", @@ -301,6 +310,7 @@ source_set("unittests") { if (enable_openscreen) { test("openscreen_unittests") { deps = [ + "//base/test:run_all_unittests", "//base/test:test_support", "//chrome/browser", "//chrome/test:test_support", diff --git a/chromium/chrome/browser/media/webrtc/DEPS b/chromium/chrome/browser/media/webrtc/DEPS index e1fc475bafa..871f1948932 100644 --- a/chromium/chrome/browser/media/webrtc/DEPS +++ b/chromium/chrome/browser/media/webrtc/DEPS @@ -6,7 +6,6 @@ include_rules = [ ] specific_include_rules = { - # TODO(mash): Fix these. https://crbug.com/723880 "desktop_capture_access_handler\.cc": [ "+ash/shell.h", ], @@ -14,11 +13,9 @@ specific_include_rules = { "+ash/shell.h", "+ash/wm/desks/desks_util.h", ], - # TODO(mash): Fix. https://crbug.com/855147 "media_capture_devices_dispatcher\.cc": [ "+ash/shell.h", ], - # TODO(mash): Remove. https://crbug.com/723880 ".*_unittest\.cc": [ "+ash/test/ash_test_base.h", ] diff --git a/chromium/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc b/chromium/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc index 11113830667..32f52e3fbf9 100644 --- a/chromium/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc +++ b/chromium/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc @@ -11,7 +11,6 @@ #include "base/command_line.h" #include "base/files/file_util.h" #include "base/strings/string_number_conversions.h" -#include "base/task/post_task.h" #include "base/task/thread_pool.h" #include "base/time/time.h" #include "components/webrtc_logging/browser/text_log_list.h" @@ -123,8 +122,8 @@ void AudioDebugRecordingsHandler::DoStartAudioDebugRecordings( } const bool is_manual_stop = false; - base::PostDelayedTask( - FROM_HERE, {BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostDelayedTask( + FROM_HERE, base::BindOnce(&AudioDebugRecordingsHandler::DoStopAudioDebugRecordings, this, host, is_manual_stop, current_audio_debug_recordings_id_, callback, diff --git a/chromium/chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context.cc b/chromium/chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context.cc index 428a1eb5304..a2a152cce0e 100644 --- a/chromium/chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context.cc +++ b/chromium/chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context.cc @@ -4,8 +4,13 @@ #include "chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context.h" +#include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h" +#include "chrome/browser/permissions/permission_manager_factory.h" +#include "chrome/browser/profiles/profile.h" +#include "components/permissions/permission_manager.h" #include "components/permissions/permission_request_id.h" #include "components/permissions/permissions_client.h" +#include "content/public/browser/browser_thread.h" #include "third_party/blink/public/mojom/feature_policy/feature_policy_feature.mojom-shared.h" CameraPanTiltZoomPermissionContext::CameraPanTiltZoomPermissionContext( @@ -22,26 +27,62 @@ CameraPanTiltZoomPermissionContext::~CameraPanTiltZoomPermissionContext() { host_content_settings_map_->RemoveObserver(this); } -#if defined(OS_ANDROID) -void CameraPanTiltZoomPermissionContext::DecidePermission( +void CameraPanTiltZoomPermissionContext::RequestPermission( content::WebContents* web_contents, const permissions::PermissionRequestID& id, - const GURL& requesting_origin, - const GURL& embedding_origin, + const GURL& requesting_frame_origin, bool user_gesture, permissions::BrowserPermissionCallback callback) { - // User should not be prompted on Android. - NOTREACHED(); + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + + if (HasAvailableCameraPtzDevices()) { + PermissionContextBase::RequestPermission(web_contents, id, + requesting_frame_origin, + user_gesture, std::move(callback)); + return; + } + + // If there is no camera with PTZ capabilities, let's request a "regular" + // camera permission instead. + content::RenderFrameHost* frame = content::RenderFrameHost::FromID( + id.render_process_id(), id.render_frame_id()); + permissions::PermissionManager* permission_manager = + PermissionManagerFactory::GetForProfile( + Profile::FromBrowserContext(web_contents->GetBrowserContext())); + permission_manager->RequestPermission(ContentSettingsType::MEDIASTREAM_CAMERA, + frame, requesting_frame_origin, + user_gesture, std::move(callback)); +} + +bool CameraPanTiltZoomPermissionContext::IsRestrictedToSecureOrigins() const { + return true; } -#endif void CameraPanTiltZoomPermissionContext::OnContentSettingChanged( const ContentSettingsPattern& primary_pattern, const ContentSettingsPattern& secondary_pattern, ContentSettingsType content_type, const std::string& resource_identifier) { - if (content_type != ContentSettingsType::MEDIASTREAM_CAMERA) + if (content_type != ContentSettingsType::MEDIASTREAM_CAMERA && + content_type != ContentSettingsType::CAMERA_PAN_TILT_ZOOM) { + return; + } + + // Skip if the camera permission is currently being updated to match camera + // PTZ permission as OnContentSettingChanged would have been called again + // causing a reentrancy issue. + if (updating_mediastream_camera_permission_) { + updating_mediastream_camera_permission_ = false; return; + } + + // Skip if the camera PTZ permission is currently being reset when camera + // permission got blocked or reset as OnContentSettingChanged would have been + // called again causing a reentrancy issue. + if (updating_camera_ptz_permission_) { + updating_camera_ptz_permission_ = false; + return; + } // TODO(crbug.com/1078272): We should not need to deduce the url from the // primary pattern here. Modify the infrastructure to facilitate this @@ -53,6 +94,18 @@ void CameraPanTiltZoomPermissionContext::OnContentSettingChanged( ContentSetting camera_ptz_setting = host_content_settings_map_->GetContentSetting( url, url, content_settings_type(), resource_identifier); + + if (content_type == ContentSettingsType::CAMERA_PAN_TILT_ZOOM) { + // Automatically update camera permission to camera PTZ permission as any + // change to camera PTZ should be reflected to camera. + updating_mediastream_camera_permission_ = true; + host_content_settings_map_->SetContentSettingCustomScope( + primary_pattern, secondary_pattern, + ContentSettingsType::MEDIASTREAM_CAMERA, resource_identifier, + camera_ptz_setting); + return; + } + // Don't reset camera PTZ permission if it is already blocked or in a // "default" state. if (camera_ptz_setting == CONTENT_SETTING_BLOCK || @@ -67,6 +120,7 @@ void CameraPanTiltZoomPermissionContext::OnContentSettingChanged( mediastream_camera_setting == CONTENT_SETTING_ASK) { // Automatically reset camera PTZ permission if camera permission // gets blocked or reset. + updating_camera_ptz_permission_ = true; host_content_settings_map_->SetContentSettingCustomScope( primary_pattern, secondary_pattern, ContentSettingsType::CAMERA_PAN_TILT_ZOOM, resource_identifier, @@ -74,6 +128,16 @@ void CameraPanTiltZoomPermissionContext::OnContentSettingChanged( } } -bool CameraPanTiltZoomPermissionContext::IsRestrictedToSecureOrigins() const { - return true; +bool CameraPanTiltZoomPermissionContext::HasAvailableCameraPtzDevices() const { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + + const std::vector<blink::MediaStreamDevice> devices = + MediaCaptureDevicesDispatcher::GetInstance()->GetVideoCaptureDevices(); + for (const blink::MediaStreamDevice& device : devices) { + if (device.pan_tilt_zoom_supported.has_value() && + device.pan_tilt_zoom_supported.value()) { + return true; + } + } + return false; } diff --git a/chromium/chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context.h b/chromium/chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context.h index 71265b64b7a..14d4794f130 100644 --- a/chromium/chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context.h +++ b/chromium/chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context.h @@ -6,7 +6,6 @@ #define CHROME_BROWSER_MEDIA_WEBRTC_CAMERA_PAN_TILT_ZOOM_PERMISSION_CONTEXT_H_ #include "base/macros.h" -#include "build/build_config.h" #include "components/content_settings/core/browser/content_settings_observer.h" #include "components/content_settings/core/browser/host_content_settings_map.h" #include "components/permissions/permission_context_base.h" @@ -29,15 +28,12 @@ class CameraPanTiltZoomPermissionContext private: // PermissionContextBase -#if defined(OS_ANDROID) - void DecidePermission( + void RequestPermission( content::WebContents* web_contents, const permissions::PermissionRequestID& id, - const GURL& requesting_origin, - const GURL& embedding_origin, + const GURL& requesting_frame_origin, bool user_gesture, permissions::BrowserPermissionCallback callback) override; -#endif bool IsRestrictedToSecureOrigins() const override; // content_settings::Observer @@ -46,7 +42,14 @@ class CameraPanTiltZoomPermissionContext ContentSettingsType content_type, const std::string& resource_identifier) override; + // Returns true if at least one video capture device has PTZ capabilities. + // Otherwise returns false. + bool HasAvailableCameraPtzDevices() const; + HostContentSettingsMap* host_content_settings_map_; + + bool updating_camera_ptz_permission_ = false; + bool updating_mediastream_camera_permission_ = false; }; #endif // CHROME_BROWSER_MEDIA_WEBRTC_CAMERA_PAN_TILT_ZOOM_PERMISSION_CONTEXT_H_ diff --git a/chromium/chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context_unittest.cc b/chromium/chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context_unittest.cc index 08a1421d1df..52375853fc6 100644 --- a/chromium/chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context_unittest.cc +++ b/chromium/chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context_unittest.cc @@ -12,35 +12,23 @@ namespace { -struct ContentSettingTestParams { - const ContentSetting initial_camera_pan_tilt_zoom; - const ContentSetting mediastream_camera; - const ContentSetting expected_camera_pan_tilt_zoom; -} kTestParams[] = { - {CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK, - CONTENT_SETTING_ASK}, // Granted permission is reset if camera is - // blocked. - {CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, - CONTENT_SETTING_ASK}, // Granted permission is reset if camera is - // reset. - {CONTENT_SETTING_BLOCK, CONTENT_SETTING_ALLOW, - CONTENT_SETTING_BLOCK}, // Blocked permission is not reset if camera - // is granted. - {CONTENT_SETTING_BLOCK, CONTENT_SETTING_BLOCK, - CONTENT_SETTING_BLOCK}, // Blocked permission is not reset if camera - // is blocked. +struct TestConfig { + const ContentSetting first; // first content setting to be set + const ContentSetting second; // second content setting to be set + const ContentSetting result; // expected resulting content setting }; } // namespace -// Waits until a camera change is observed in content settings. -class CameraContentSettingsChangeWaiter : public content_settings::Observer { +// Waits until a change is observed for a specific content setting type. +class ContentSettingsChangeWaiter : public content_settings::Observer { public: - explicit CameraContentSettingsChangeWaiter(Profile* profile) - : profile_(profile) { + explicit ContentSettingsChangeWaiter(Profile* profile, + ContentSettingsType content_type) + : profile_(profile), content_type_(content_type) { HostContentSettingsMapFactory::GetForProfile(profile)->AddObserver(this); } - ~CameraContentSettingsChangeWaiter() override { + ~ContentSettingsChangeWaiter() override { HostContentSettingsMapFactory::GetForProfile(profile_)->RemoveObserver( this); } @@ -50,7 +38,7 @@ class CameraContentSettingsChangeWaiter : public content_settings::Observer { const ContentSettingsPattern& secondary_pattern, ContentSettingsType content_type, const std::string& resource_identifier) override { - if (content_type == ContentSettingsType::MEDIASTREAM_CAMERA) + if (content_type == content_type_) Proceed(); } @@ -60,14 +48,15 @@ class CameraContentSettingsChangeWaiter : public content_settings::Observer { void Proceed() { run_loop_.Quit(); } Profile* profile_; + ContentSettingsType content_type_; base::RunLoop run_loop_; - DISALLOW_COPY_AND_ASSIGN(CameraContentSettingsChangeWaiter); + DISALLOW_COPY_AND_ASSIGN(ContentSettingsChangeWaiter); }; class CameraPanTiltZoomPermissionContextTests : public ChromeRenderViewHostTestHarness, - public testing::WithParamInterface<ContentSettingTestParams> { + public testing::WithParamInterface<TestConfig> { public: CameraPanTiltZoomPermissionContextTests() = default; @@ -91,21 +80,113 @@ class CameraPanTiltZoomPermissionContextTests DISALLOW_COPY_AND_ASSIGN(CameraPanTiltZoomPermissionContextTests); }; -TEST_P(CameraPanTiltZoomPermissionContextTests, - TestResetPermissionOnCameraChange) { +class CameraContentSettingTests + : public CameraPanTiltZoomPermissionContextTests { + public: + CameraContentSettingTests() = default; +}; + +TEST_P(CameraContentSettingTests, TestResetPermissionOnCameraChange) { CameraPanTiltZoomPermissionContext permission_context(profile()); - CameraContentSettingsChangeWaiter waiter(profile()); + ContentSettingsChangeWaiter waiter(profile(), + ContentSettingsType::MEDIASTREAM_CAMERA); SetContentSetting(ContentSettingsType::CAMERA_PAN_TILT_ZOOM, - GetParam().initial_camera_pan_tilt_zoom); - SetContentSetting(ContentSettingsType::MEDIASTREAM_CAMERA, - GetParam().mediastream_camera); + GetParam().first); + SetContentSetting(ContentSettingsType::MEDIASTREAM_CAMERA, GetParam().second); waiter.Wait(); - EXPECT_EQ(GetParam().expected_camera_pan_tilt_zoom, + EXPECT_EQ(GetParam().result, GetContentSetting(ContentSettingsType::CAMERA_PAN_TILT_ZOOM)); } -INSTANTIATE_TEST_SUITE_P(ResetPermissionOnCameraChange, - CameraPanTiltZoomPermissionContextTests, - testing::ValuesIn(kTestParams)); +INSTANTIATE_TEST_SUITE_P( + ResetPermissionOnCameraChange, + CameraContentSettingTests, + testing::Values( + // Granted camera PTZ permission is reset if camera is blocked. + TestConfig{CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK, + CONTENT_SETTING_ASK}, + // Granted camera PTZ permission is reset if camera is reset. + TestConfig{CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, + CONTENT_SETTING_ASK}, + // Blocked camera PTZ permission is not reset if camera is granted. + TestConfig{CONTENT_SETTING_BLOCK, CONTENT_SETTING_ALLOW, + CONTENT_SETTING_BLOCK}, + // Blocked camera PTZ permission is not reset if camera is blocked. + TestConfig{CONTENT_SETTING_BLOCK, CONTENT_SETTING_BLOCK, + CONTENT_SETTING_BLOCK})); + +class CameraPanTiltZoomContentSettingTests + : public CameraPanTiltZoomPermissionContextTests { + public: + CameraPanTiltZoomContentSettingTests() = default; +}; + +TEST_P(CameraPanTiltZoomContentSettingTests, + TestCameraPermissionOnCameraPanTiltZoomChange) { + CameraPanTiltZoomPermissionContext permission_context(profile()); + ContentSettingsChangeWaiter waiter(profile(), + ContentSettingsType::CAMERA_PAN_TILT_ZOOM); + + SetContentSetting(ContentSettingsType::MEDIASTREAM_CAMERA, GetParam().first); + SetContentSetting(ContentSettingsType::CAMERA_PAN_TILT_ZOOM, + GetParam().second); + + waiter.Wait(); + EXPECT_EQ(GetParam().result, + GetContentSetting(ContentSettingsType::MEDIASTREAM_CAMERA)); +} + +INSTANTIATE_TEST_SUITE_P( + CameraPermissionOnCameraPanTiltZoomChange, + CameraPanTiltZoomContentSettingTests, + testing::Values( + // Asked camera permission is blocked if camera PTZ is blocked. + TestConfig{CONTENT_SETTING_ASK, CONTENT_SETTING_BLOCK, + CONTENT_SETTING_BLOCK}, + // Asked camera permission is granted if camera PTZ is granted. + TestConfig{CONTENT_SETTING_ASK, CONTENT_SETTING_ALLOW, + CONTENT_SETTING_ALLOW}, + // Asked camera permission is unchanged if camera PTZ is reset. + TestConfig{CONTENT_SETTING_ASK, CONTENT_SETTING_DEFAULT, + CONTENT_SETTING_ASK}, + // Asked camera permission is unchanged if camera PTZ is ask. + TestConfig{CONTENT_SETTING_ASK, CONTENT_SETTING_ASK, + CONTENT_SETTING_ASK}, + // Allowed camera permission is blocked if camera PTZ is blocked. + TestConfig{CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK, + CONTENT_SETTING_BLOCK}, + // Allowed camera permission is unchanged if camera PTZ is granted. + TestConfig{CONTENT_SETTING_ALLOW, CONTENT_SETTING_ALLOW, + CONTENT_SETTING_ALLOW}, + // Allowed camera permission is ask if camera PTZ is reset. + TestConfig{CONTENT_SETTING_ALLOW, CONTENT_SETTING_DEFAULT, + CONTENT_SETTING_ASK}, + // Allowed camera permission is reset if camera PTZ is ask. + TestConfig{CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, + CONTENT_SETTING_ASK}, + // Blocked camera permission is unchanged if camera PTZ is blocked. + TestConfig{CONTENT_SETTING_BLOCK, CONTENT_SETTING_BLOCK, + CONTENT_SETTING_BLOCK}, + // Blocked camera permission is allowed if camera PTZ is granted. + TestConfig{CONTENT_SETTING_BLOCK, CONTENT_SETTING_ALLOW, + CONTENT_SETTING_ALLOW}, + // Blocked camera permission is ask if camera PTZ is reset. + TestConfig{CONTENT_SETTING_BLOCK, CONTENT_SETTING_DEFAULT, + CONTENT_SETTING_ASK}, + // Blocked camera permission is reset if camera PTZ is ask. + TestConfig{CONTENT_SETTING_BLOCK, CONTENT_SETTING_ASK, + CONTENT_SETTING_ASK}, + // Default camera permission is blocked if camera PTZ is blocked. + TestConfig{CONTENT_SETTING_DEFAULT, CONTENT_SETTING_BLOCK, + CONTENT_SETTING_BLOCK}, + // Default camera permission is allowed if camera PTZ is granted. + TestConfig{CONTENT_SETTING_DEFAULT, CONTENT_SETTING_ALLOW, + CONTENT_SETTING_ALLOW}, + // Default camera permission is unchanged if camera PTZ is reset. + TestConfig{CONTENT_SETTING_DEFAULT, CONTENT_SETTING_DEFAULT, + CONTENT_SETTING_ASK}, + // Default camera permission is ask if camera PTZ is ask. + TestConfig{CONTENT_SETTING_DEFAULT, CONTENT_SETTING_ASK, + CONTENT_SETTING_ASK})); diff --git a/chromium/chrome/browser/media/webrtc/desktop_capture_access_handler.cc b/chromium/chrome/browser/media/webrtc/desktop_capture_access_handler.cc index 2d9ab99f816..e3f7e784f03 100644 --- a/chromium/chrome/browser/media/webrtc/desktop_capture_access_handler.cc +++ b/chromium/chrome/browser/media/webrtc/desktop_capture_access_handler.cc @@ -296,6 +296,7 @@ void DesktopCaptureAccessHandler::HandleRequest( const content::MediaStreamRequest& request, content::MediaResponseCallback callback, const extensions::Extension* extension) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); blink::MediaStreamDevices devices; std::unique_ptr<content::MediaStreamUI> ui; @@ -379,6 +380,17 @@ void DesktopCaptureAccessHandler::HandleRequest( } #endif + if (media_id.type == content::DesktopMediaID::TYPE_WEB_CONTENTS && + !content::WebContents::FromRenderFrameHost( + content::RenderFrameHost::FromID( + media_id.web_contents_id.render_process_id, + media_id.web_contents_id.main_render_frame_id))) { + std::move(callback).Run( + devices, blink::mojom::MediaStreamRequestResult::TAB_CAPTURE_FAILURE, + std::move(ui)); + return; + } + bool loopback_audio_supported = false; #if defined(USE_CRAS) || defined(OS_WIN) // Currently loopback audio capture is supported only on Windows and ChromeOS. diff --git a/chromium/chrome/browser/media/webrtc/desktop_media_list_base.cc b/chromium/chrome/browser/media/webrtc/desktop_media_list_base.cc index ba64a40697c..147d5be9f7f 100644 --- a/chromium/chrome/browser/media/webrtc/desktop_media_list_base.cc +++ b/chromium/chrome/browser/media/webrtc/desktop_media_list_base.cc @@ -8,13 +8,11 @@ #include <utility> #include "base/bind.h" -#include "base/task/post_task.h" #include "chrome/browser/media/webrtc/desktop_media_list.h" #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" #include "ui/gfx/image/image.h" -using content::BrowserThread; using content::DesktopMediaID; DesktopMediaListBase::DesktopMediaListBase(base::TimeDelta update_period) @@ -179,8 +177,9 @@ void DesktopMediaListBase::ScheduleNextRefresh() { DCHECK(!refresh_callback_); refresh_callback_ = base::BindOnce(&DesktopMediaListBase::ScheduleNextRefresh, weak_factory_.GetWeakPtr()); - base::PostDelayedTask(FROM_HERE, {BrowserThread::UI}, - base::BindOnce(&DesktopMediaListBase::Refresh, - weak_factory_.GetWeakPtr(), true), - update_period_); + content::GetUIThreadTaskRunner({})->PostDelayedTask( + FROM_HERE, + base::BindOnce(&DesktopMediaListBase::Refresh, weak_factory_.GetWeakPtr(), + true), + update_period_); } diff --git a/chromium/chrome/browser/media/webrtc/desktop_media_picker_factory_impl.cc b/chromium/chrome/browser/media/webrtc/desktop_media_picker_factory_impl.cc index fa7d872810a..63ad845f1cc 100644 --- a/chromium/chrome/browser/media/webrtc/desktop_media_picker_factory_impl.cc +++ b/chromium/chrome/browser/media/webrtc/desktop_media_picker_factory_impl.cc @@ -52,9 +52,16 @@ DesktopMediaPickerFactoryImpl::CreateMediaList( screen_list = std::make_unique<DesktopMediaListAsh>( content::DesktopMediaID::TYPE_SCREEN); #else // !defined(OS_CHROMEOS) + // If screen capture is not supported on the platform, then we should + // not attempt to create an instance of NativeDesktopMediaList. Doing so + // will hit a DCHECK. + std::unique_ptr<webrtc::DesktopCapturer> capturer = + content::desktop_capture::CreateScreenCapturer(); + if (!capturer) + continue; + screen_list = std::make_unique<NativeDesktopMediaList>( - content::DesktopMediaID::TYPE_SCREEN, - content::desktop_capture::CreateScreenCapturer()); + content::DesktopMediaID::TYPE_SCREEN, std::move(capturer)); #endif // !defined(OS_CHROMEOS) have_screen_list = true; source_lists.push_back(std::move(screen_list)); @@ -68,9 +75,15 @@ DesktopMediaPickerFactoryImpl::CreateMediaList( window_list = std::make_unique<DesktopMediaListAsh>( content::DesktopMediaID::TYPE_WINDOW); #else // !defined(OS_CHROMEOS) + // If window capture is not supported on the platform, then we should + // not attempt to create an instance of NativeDesktopMediaList. Doing so + // will hit a DCHECK. + std::unique_ptr<webrtc::DesktopCapturer> capturer = + content::desktop_capture::CreateWindowCapturer(); + if (!capturer) + continue; window_list = std::make_unique<NativeDesktopMediaList>( - content::DesktopMediaID::TYPE_WINDOW, - content::desktop_capture::CreateWindowCapturer()); + content::DesktopMediaID::TYPE_WINDOW, std::move(capturer)); #endif // !defined(OS_CHROMEOS) have_window_list = true; source_lists.push_back(std::move(window_list)); diff --git a/chromium/chrome/browser/media/webrtc/display_media_access_handler.cc b/chromium/chrome/browser/media/webrtc/display_media_access_handler.cc index 4ca36f534dc..3c002920c1d 100644 --- a/chromium/chrome/browser/media/webrtc/display_media_access_handler.cc +++ b/chromium/chrome/browser/media/webrtc/display_media_access_handler.cc @@ -17,6 +17,7 @@ #include "chrome/browser/media/webrtc/native_desktop_media_list.h" #include "chrome/browser/media/webrtc/tab_desktop_media_list.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/browser/safe_browsing/user_interaction_observer.h" #include "chrome/common/pref_names.h" #include "components/prefs/pref_service.h" #include "components/url_formatter/elide_url.h" @@ -100,6 +101,21 @@ void DisplayMediaAccessHandler::HandleRequest( return; } + // SafeBrowsing Delayed Warnings experiment can delay some SafeBrowsing + // warnings until user interaction. If the current page has a delayed warning, + // it'll have a user interaction observer attached. Show the warning + // immediately in that case. + safe_browsing::SafeBrowsingUserInteractionObserver* observer = + safe_browsing::SafeBrowsingUserInteractionObserver::FromWebContents( + web_contents); + if (observer) { + std::move(callback).Run( + blink::MediaStreamDevices(), + blink::mojom::MediaStreamRequestResult::PERMISSION_DENIED, nullptr); + observer->OnDesktopCaptureRequest(); + return; + } + #if defined(OS_MACOSX) // Do not allow picker UI to be shown on a page that isn't in the foreground // in Mac, because the UI implementation in Mac pops a window over any content @@ -225,6 +241,14 @@ void DisplayMediaAccessHandler::OnPickerDialogResults( blink::mojom::MediaStreamRequestResult::SYSTEM_PERMISSION_DENIED; } #endif + if (media_id.type == content::DesktopMediaID::TYPE_WEB_CONTENTS && + !content::WebContents::FromRenderFrameHost( + content::RenderFrameHost::FromID( + media_id.web_contents_id.render_process_id, + media_id.web_contents_id.main_render_frame_id))) { + request_result = + blink::mojom::MediaStreamRequestResult::TAB_CAPTURE_FAILURE; + } if (request_result == blink::mojom::MediaStreamRequestResult::OK) { const auto& visible_url = url_formatter::FormatUrlForSecurityDisplay( web_contents->GetLastCommittedURL(), diff --git a/chromium/chrome/browser/media/webrtc/fake_desktop_media_list.cc b/chromium/chrome/browser/media/webrtc/fake_desktop_media_list.cc index ee71126dc3e..3bc98e78b45 100644 --- a/chromium/chrome/browser/media/webrtc/fake_desktop_media_list.cc +++ b/chromium/chrome/browser/media/webrtc/fake_desktop_media_list.cc @@ -14,7 +14,7 @@ using content::DesktopMediaID; FakeDesktopMediaList::FakeDesktopMediaList(DesktopMediaID::Type type) - : observer_(NULL), type_(type) {} + : observer_(nullptr), type_(type) {} FakeDesktopMediaList::~FakeDesktopMediaList() {} void FakeDesktopMediaList::AddSource(int id) { diff --git a/chromium/chrome/browser/media/webrtc/media_capture_devices_dispatcher.cc b/chromium/chrome/browser/media/webrtc/media_capture_devices_dispatcher.cc index 00e2c25d5ea..1ff00413044 100644 --- a/chromium/chrome/browser/media/webrtc/media_capture_devices_dispatcher.cc +++ b/chromium/chrome/browser/media/webrtc/media_capture_devices_dispatcher.cc @@ -14,7 +14,6 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" -#include "base/task/post_task.h" #include "build/build_config.h" #include "chrome/browser/media/media_access_handler.h" #include "chrome/browser/media/webrtc/media_stream_capture_indicator.h" @@ -303,8 +302,8 @@ MediaCaptureDevicesDispatcher::GetMediaStreamCaptureIndicator() { void MediaCaptureDevicesDispatcher::OnAudioCaptureDevicesChanged() { DCHECK_CURRENTLY_ON(BrowserThread::IO); - base::PostTask( - FROM_HERE, {BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce( &MediaCaptureDevicesDispatcher::NotifyAudioDevicesChangedOnUIThread, base::Unretained(this))); @@ -312,8 +311,8 @@ void MediaCaptureDevicesDispatcher::OnAudioCaptureDevicesChanged() { void MediaCaptureDevicesDispatcher::OnVideoCaptureDevicesChanged() { DCHECK_CURRENTLY_ON(BrowserThread::IO); - base::PostTask( - FROM_HERE, {BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce( &MediaCaptureDevicesDispatcher::NotifyVideoDevicesChangedOnUIThread, base::Unretained(this))); @@ -327,8 +326,8 @@ void MediaCaptureDevicesDispatcher::OnMediaRequestStateChanged( blink::mojom::MediaStreamType stream_type, content::MediaRequestState state) { DCHECK_CURRENTLY_ON(BrowserThread::IO); - base::PostTask( - FROM_HERE, {BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce( &MediaCaptureDevicesDispatcher::UpdateMediaRequestStateOnUIThread, base::Unretained(this), render_process_id, render_frame_id, @@ -348,8 +347,8 @@ void MediaCaptureDevicesDispatcher::OnCreatingAudioStream(int render_process_id, } DCHECK_CURRENTLY_ON(BrowserThread::IO); - base::PostTask( - FROM_HERE, {BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce( &MediaCaptureDevicesDispatcher::OnCreatingAudioStreamOnUIThread, base::Unretained(this), render_process_id, render_frame_id)); @@ -445,8 +444,8 @@ void MediaCaptureDevicesDispatcher::OnSetCapturingLinkSecured( if (!blink::IsVideoScreenCaptureMediaType(stream_type)) return; - base::PostTask( - FROM_HERE, {BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce( &MediaCaptureDevicesDispatcher::UpdateVideoScreenCaptureStatus, base::Unretained(this), render_process_id, render_frame_id, diff --git a/chromium/chrome/browser/media/webrtc/media_capture_devices_dispatcher_unittest.cc b/chromium/chrome/browser/media/webrtc/media_capture_devices_dispatcher_unittest.cc index 30d6696e3ba..62ac808bd29 100644 --- a/chromium/chrome/browser/media/webrtc/media_capture_devices_dispatcher_unittest.cc +++ b/chromium/chrome/browser/media/webrtc/media_capture_devices_dispatcher_unittest.cc @@ -6,6 +6,7 @@ #include <memory> +#include "build/build_config.h" #include "chrome/browser/media/media_access_handler.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "testing/gmock/include/gmock/gmock.h" @@ -73,7 +74,8 @@ class MediaCaptureDevicesDispatcherTest MediaCaptureDevicesDispatcher* dispatcher_; }; -TEST_F(MediaCaptureDevicesDispatcherTest, LoopsAllMediaAccessHandlers) { +TEST_F(MediaCaptureDevicesDispatcherTest, + DISABLED_LoopsAllMediaAccessHandlers) { media_access_handlers().clear(); // Add two handlers. diff --git a/chromium/chrome/browser/media/webrtc/media_stream_capture_indicator.cc b/chromium/chrome/browser/media/webrtc/media_stream_capture_indicator.cc index aefe2a0905e..ce93d308a3d 100644 --- a/chromium/chrome/browser/media/webrtc/media_stream_capture_indicator.cc +++ b/chromium/chrome/browser/media/webrtc/media_stream_capture_indicator.cc @@ -54,7 +54,7 @@ const extensions::Extension* GetExtension(WebContents* web_contents) { DCHECK_CURRENTLY_ON(BrowserThread::UI); if (!web_contents) - return NULL; + return nullptr; extensions::ExtensionRegistry* registry = extensions::ExtensionRegistry::Get(web_contents->GetBrowserContext()); @@ -338,45 +338,60 @@ void MediaStreamCaptureIndicator::ExecuteCommand(int command_id, web_contents->GetDelegate()->ActivateContents(web_contents); } +bool MediaStreamCaptureIndicator::CheckUsage( + content::WebContents* web_contents, + const WebContentsDeviceUsagePredicate& pred) const { + auto it = usage_map_.find(web_contents); + if (it != usage_map_.end() && pred.Run(it->second.get())) + return true; + + for (auto* inner_contents : web_contents->GetInnerWebContents()) { + if (CheckUsage(inner_contents, pred)) + return true; + } + + return false; +} + bool MediaStreamCaptureIndicator::IsCapturingUserMedia( content::WebContents* web_contents) const { DCHECK_CURRENTLY_ON(BrowserThread::UI); - - auto it = usage_map_.find(web_contents); - return it != usage_map_.end() && - (it->second->IsCapturingAudio() || it->second->IsCapturingVideo()); + return CheckUsage( + web_contents, + base::BindRepeating([](const WebContentsDeviceUsage* usage) { + return usage->IsCapturingAudio() || usage->IsCapturingVideo(); + })); } bool MediaStreamCaptureIndicator::IsCapturingVideo( content::WebContents* web_contents) const { DCHECK_CURRENTLY_ON(BrowserThread::UI); - - auto it = usage_map_.find(web_contents); - return it != usage_map_.end() && it->second->IsCapturingVideo(); + return CheckUsage( + web_contents, + base::BindRepeating(&WebContentsDeviceUsage::IsCapturingVideo)); } bool MediaStreamCaptureIndicator::IsCapturingAudio( content::WebContents* web_contents) const { DCHECK_CURRENTLY_ON(BrowserThread::UI); - - auto it = usage_map_.find(web_contents); - return it != usage_map_.end() && it->second->IsCapturingAudio(); + return CheckUsage( + web_contents, + base::BindRepeating(&WebContentsDeviceUsage::IsCapturingAudio)); } bool MediaStreamCaptureIndicator::IsBeingMirrored( content::WebContents* web_contents) const { DCHECK_CURRENTLY_ON(BrowserThread::UI); - - auto it = usage_map_.find(web_contents); - return it != usage_map_.end() && it->second->IsMirroring(); + return CheckUsage(web_contents, + base::BindRepeating(&WebContentsDeviceUsage::IsMirroring)); } bool MediaStreamCaptureIndicator::IsCapturingDesktop( content::WebContents* web_contents) const { DCHECK_CURRENTLY_ON(BrowserThread::UI); - - auto it = usage_map_.find(web_contents); - return it != usage_map_.end() && it->second->IsCapturingDesktop(); + return CheckUsage( + web_contents, + base::BindRepeating(&WebContentsDeviceUsage::IsCapturingDesktop)); } void MediaStreamCaptureIndicator::NotifyStopped( @@ -386,6 +401,9 @@ void MediaStreamCaptureIndicator::NotifyStopped( auto it = usage_map_.find(web_contents); DCHECK(it != usage_map_.end()); it->second->NotifyStopped(); + + for (auto* inner_contents : web_contents->GetInnerWebContents()) + NotifyStopped(inner_contents); } void MediaStreamCaptureIndicator::UnregisterWebContents( @@ -430,9 +448,9 @@ void MediaStreamCaptureIndicator::MaybeDestroyStatusTrayIcon() { return; StatusTray* status_tray = g_browser_process->status_tray(); - if (status_tray != NULL) { + if (status_tray != nullptr) { status_tray->RemoveStatusIcon(status_icon_); - status_icon_ = NULL; + status_icon_ = nullptr; } } diff --git a/chromium/chrome/browser/media/webrtc/media_stream_capture_indicator.h b/chromium/chrome/browser/media/webrtc/media_stream_capture_indicator.h index 86c2011caa4..323e065fb6d 100644 --- a/chromium/chrome/browser/media/webrtc/media_stream_capture_indicator.h +++ b/chromium/chrome/browser/media/webrtc/media_stream_capture_indicator.h @@ -135,6 +135,13 @@ class MediaStreamCaptureIndicator gfx::ImageSkia* image, base::string16* tool_tip); + // Checks if |web_contents| or any portal WebContents in its tree is using + // a device for capture. The type of capture is specified using |pred|. + using WebContentsDeviceUsagePredicate = + base::RepeatingCallback<bool(const WebContentsDeviceUsage*)>; + bool CheckUsage(content::WebContents* web_contents, + const WebContentsDeviceUsagePredicate& pred) const; + // Reference to our status icon - owned by the StatusTray. If null, // the platform doesn't support status icons. StatusIcon* status_icon_ = nullptr; diff --git a/chromium/chrome/browser/media/webrtc/media_stream_capture_indicator_unittest.cc b/chromium/chrome/browser/media/webrtc/media_stream_capture_indicator_unittest.cc index 1676ddeb1ea..a7a23f52c76 100644 --- a/chromium/chrome/browser/media/webrtc/media_stream_capture_indicator_unittest.cc +++ b/chromium/chrome/browser/media/webrtc/media_stream_capture_indicator_unittest.cc @@ -7,6 +7,7 @@ #include "base/bind.h" #include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h" +#include "content/public/test/web_contents_tester.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/blink/public/mojom/mediastream/media_stream.mojom-shared.h" @@ -74,15 +75,17 @@ class MediaStreamCaptureIndicatorTest : public ChromeRenderViewHostTestHarness { void SetUp() override { ChromeRenderViewHostTestHarness::SetUp(); + content::WebContentsTester::For(web_contents()) + ->NavigateAndCommit(GURL("https://www.example.com/")); indicator_ = MediaCaptureDevicesDispatcher::GetInstance() ->GetMediaStreamCaptureIndicator(); observer_ = std::make_unique<MockObserver>(); indicator_->AddObserver(observer()); - contents_ = CreateTestWebContents(); + portal_token_ = content::WebContentsTester::For(web_contents()) + ->CreatePortal(CreateTestWebContents()); } void TearDown() override { - contents_.reset(); indicator_->RemoveObserver(observer()); observer_.reset(); indicator_.reset(); @@ -90,7 +93,10 @@ class MediaStreamCaptureIndicatorTest : public ChromeRenderViewHostTestHarness { } MediaStreamCaptureIndicator* indicator() { return indicator_.get(); } - content::WebContents* contents() { return contents_.get(); } + content::WebContents* portal_contents() { + return content::WebContentsTester::For(web_contents()) + ->GetPortalContents(portal_token_); + } MockObserver* observer() { return observer_.get(); } // Test a MediaStreamCaptureIndicator accessor and ensure that the @@ -98,11 +104,15 @@ class MediaStreamCaptureIndicatorTest : public ChromeRenderViewHostTestHarness { void TestObserverMethod(blink::mojom::MediaStreamType stream_type, MockObserverSetExpectationsMethod observer_method, AccessorMethod accessor_method); + void TestObserverMethodWithPortal( + blink::mojom::MediaStreamType stream_type, + MockObserverSetExpectationsMethod observer_method, + AccessorMethod accessor_method); private: std::unique_ptr<MockObserver> observer_; - std::unique_ptr<content::WebContents> contents_; scoped_refptr<MediaStreamCaptureIndicator> indicator_; + base::UnguessableToken portal_token_; }; } // namespace @@ -116,22 +126,50 @@ void MediaStreamCaptureIndicatorTest::TestObserverMethod( blink::MediaStreamDevice(stream_type, "fake_device", "fake_device")}; // By default all accessors should return false as there's no stream device. - EXPECT_FALSE((indicator()->*accessor_method)(contents())); + EXPECT_FALSE((indicator()->*accessor_method)(web_contents())); + std::unique_ptr<content::MediaStreamUI> ui = + indicator()->RegisterMediaStream(web_contents(), devices); + + // Make sure that the observer gets called and that the corresponding accessor + // gets called when |OnStarted| is called. + (observer()->*observer_set_expectations_method)(web_contents(), true); + ui->OnStarted(base::OnceClosure(), content::MediaStreamUI::SourceCallback()); + EXPECT_TRUE((indicator()->*accessor_method)(web_contents())); + ::testing::Mock::VerifyAndClear(observer()); + + // Removing the stream device should cause the observer to be notified that + // the observed property is now set to false. + (observer()->*observer_set_expectations_method)(web_contents(), false); + ui.reset(); + EXPECT_FALSE((indicator()->*accessor_method)(web_contents())); + ::testing::Mock::VerifyAndClear(observer()); +} + +void MediaStreamCaptureIndicatorTest::TestObserverMethodWithPortal( + blink::mojom::MediaStreamType stream_type, + MockObserverSetExpectationsMethod observer_set_expectations_method, + AccessorMethod accessor_method) { + // Create the fake stream device. + blink::MediaStreamDevices devices{ + blink::MediaStreamDevice(stream_type, "fake_device", "fake_device")}; + + // By default all accessors should return false as there's no stream device. + EXPECT_FALSE((indicator()->*accessor_method)(web_contents())); std::unique_ptr<content::MediaStreamUI> ui = - indicator()->RegisterMediaStream(contents(), devices); + indicator()->RegisterMediaStream(portal_contents(), devices); // Make sure that the observer gets called and that the corresponding accessor // gets called when |OnStarted| is called. - (observer()->*observer_set_expectations_method)(contents(), true); + (observer()->*observer_set_expectations_method)(portal_contents(), true); ui->OnStarted(base::OnceClosure(), content::MediaStreamUI::SourceCallback()); - EXPECT_TRUE((indicator()->*accessor_method)(contents())); + EXPECT_TRUE((indicator()->*accessor_method)(web_contents())); ::testing::Mock::VerifyAndClear(observer()); // Removing the stream device should cause the observer to be notified that // the observed property is now set to false. - (observer()->*observer_set_expectations_method)(contents(), false); + (observer()->*observer_set_expectations_method)(portal_contents(), false); ui.reset(); - EXPECT_FALSE((indicator()->*accessor_method)(contents())); + EXPECT_FALSE((indicator()->*accessor_method)(web_contents())); ::testing::Mock::VerifyAndClear(observer()); } @@ -141,20 +179,48 @@ TEST_F(MediaStreamCaptureIndicatorTest, IsCapturingVideo) { &MediaStreamCaptureIndicator::IsCapturingVideo); } +TEST_F(MediaStreamCaptureIndicatorTest, IsCapturingVideo_Portal) { + TestObserverMethodWithPortal( + blink::mojom::MediaStreamType::DEVICE_VIDEO_CAPTURE, + &MockObserver::SetOnIsCapturingVideoChangedExpectation, + &MediaStreamCaptureIndicator::IsCapturingVideo); +} + TEST_F(MediaStreamCaptureIndicatorTest, IsCapturingAudio) { TestObserverMethod(blink::mojom::MediaStreamType::DEVICE_AUDIO_CAPTURE, &MockObserver::SetOnIsCapturingAudioChangedExpectation, &MediaStreamCaptureIndicator::IsCapturingAudio); } +TEST_F(MediaStreamCaptureIndicatorTest, IsCapturingAudio_Portal) { + TestObserverMethodWithPortal( + blink::mojom::MediaStreamType::DEVICE_AUDIO_CAPTURE, + &MockObserver::SetOnIsCapturingAudioChangedExpectation, + &MediaStreamCaptureIndicator::IsCapturingAudio); +} + TEST_F(MediaStreamCaptureIndicatorTest, IsBeingMirrored) { TestObserverMethod(blink::mojom::MediaStreamType::GUM_TAB_VIDEO_CAPTURE, &MockObserver::SetOnIsBeingMirroredChangedExpectation, &MediaStreamCaptureIndicator::IsBeingMirrored); } +TEST_F(MediaStreamCaptureIndicatorTest, IsBeingMirrored_Portal) { + TestObserverMethodWithPortal( + blink::mojom::MediaStreamType::GUM_TAB_VIDEO_CAPTURE, + &MockObserver::SetOnIsBeingMirroredChangedExpectation, + &MediaStreamCaptureIndicator::IsBeingMirrored); +} + TEST_F(MediaStreamCaptureIndicatorTest, IsCapturingDesktop) { TestObserverMethod(blink::mojom::MediaStreamType::GUM_DESKTOP_VIDEO_CAPTURE, &MockObserver::SetOnIsCapturingDesktopChangedExpectation, &MediaStreamCaptureIndicator::IsCapturingDesktop); } + +TEST_F(MediaStreamCaptureIndicatorTest, IsCapturingDesktop_Portal) { + TestObserverMethodWithPortal( + blink::mojom::MediaStreamType::GUM_DESKTOP_VIDEO_CAPTURE, + &MockObserver::SetOnIsCapturingDesktopChangedExpectation, + &MediaStreamCaptureIndicator::IsCapturingDesktop); +} diff --git a/chromium/chrome/browser/media/webrtc/media_stream_devices_controller_browsertest.cc b/chromium/chrome/browser/media/webrtc/media_stream_devices_controller_browsertest.cc index 2f46debbeb0..7d975a48e8c 100644 --- a/chromium/chrome/browser/media/webrtc/media_stream_devices_controller_browsertest.cc +++ b/chromium/chrome/browser/media/webrtc/media_stream_devices_controller_browsertest.cc @@ -10,11 +10,13 @@ #include "base/run_loop.h" #include "base/stl_util.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h" +#include "chrome/browser/media/webrtc/camera_pan_tilt_zoom_permission_context.h" #include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h" #include "chrome/browser/media/webrtc/media_stream_capture_indicator.h" #include "chrome/browser/media/webrtc/media_stream_device_permissions.h" #include "chrome/browser/media/webrtc/permission_bubble_media_access_handler.h" #include "chrome/browser/media/webrtc/webrtc_browsertest_base.h" +#include "chrome/browser/permissions/permission_manager_factory.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" @@ -24,6 +26,7 @@ #include "components/content_settings/browser/tab_specific_content_settings.h" #include "components/content_settings/core/browser/host_content_settings_map.h" #include "components/permissions/permission_context_base.h" +#include "components/permissions/permission_manager.h" #include "components/permissions/permission_request.h" #include "components/permissions/permission_request_manager.h" #include "components/permissions/permission_util.h" @@ -106,9 +109,10 @@ class MediaStreamDevicesControllerTest : public WebRtcTestBase { prefs->SetBoolean(policy_name, access == ACCESS_ALLOWED); } - // Set the content settings for mic/cam. + // Set the content settings for mic/cam/ptz. void SetContentSettings(ContentSetting mic_setting, - ContentSetting cam_setting) { + ContentSetting cam_setting, + ContentSetting ptz_setting) { HostContentSettingsMap* content_settings = HostContentSettingsMapFactory::GetForProfile( Profile::FromBrowserContext(GetWebContents()->GetBrowserContext())); @@ -118,6 +122,9 @@ class MediaStreamDevicesControllerTest : public WebRtcTestBase { content_settings->SetContentSettingDefaultScope( example_url_, GURL(), ContentSettingsType::MEDIASTREAM_CAMERA, std::string(), cam_setting); + content_settings->SetContentSettingDefaultScope( + example_url_, GURL(), ContentSettingsType::CAMERA_PAN_TILT_ZOOM, + std::string(), ptz_setting); } // Checks whether the devices returned in OnMediaStreamResponse contains a @@ -140,6 +147,7 @@ class MediaStreamDevicesControllerTest : public WebRtcTestBase { content::MediaStreamRequest CreateRequestWithType( const std::string& audio_id, const std::string& video_id, + bool request_pan_tilt_zoom_permission, blink::MediaStreamRequestType request_type) { blink::mojom::MediaStreamType audio_type = audio_id.empty() ? blink::mojom::MediaStreamType::NO_SERVICE @@ -155,13 +163,15 @@ class MediaStreamDevicesControllerTest : public WebRtcTestBase { return content::MediaStreamRequest( render_process_id, render_frame_id, 0, example_url(), false, request_type, audio_id, video_id, audio_type, video_type, - /*disable_local_echo=*/false, - /*request_pan_tilt_zoom_permission=*/false); + /*disable_local_echo=*/false, request_pan_tilt_zoom_permission); } - content::MediaStreamRequest CreateRequest(const std::string& audio_id, - const std::string& video_id) { + content::MediaStreamRequest CreateRequest( + const std::string& audio_id, + const std::string& video_id, + bool request_pan_tilt_zoom_permission) { return CreateRequestWithType(audio_id, video_id, + request_pan_tilt_zoom_permission, blink::MEDIA_DEVICE_ACCESS); } @@ -173,6 +183,8 @@ class MediaStreamDevicesControllerTest : public WebRtcTestBase { GetContentSettings()->GetMicrophoneCameraState()); } + virtual bool IsPanTiltZoomSupported() const { return false; } + permissions::MockPermissionPromptFactory* prompt_factory() { return prompt_factory_.get(); } @@ -219,7 +231,8 @@ class MediaStreamDevicesControllerTest : public WebRtcTestBase { blink::MediaStreamDevices video_devices; blink::MediaStreamDevice fake_video_device( blink::mojom::MediaStreamType::DEVICE_VIDEO_CAPTURE, example_video_id_, - "Fake Video Device"); + "Fake Video Device", media::MEDIA_VIDEO_FACING_NONE, base::nullopt, + IsPanTiltZoomSupported()); video_devices.push_back(fake_video_device); MediaCaptureDevicesDispatcher::GetInstance()->SetTestVideoCaptureDevices( video_devices); @@ -247,6 +260,13 @@ class MediaStreamDevicesControllerTest : public WebRtcTestBase { permission_bubble_media_access_handler_; }; +class MediaStreamDevicesControllerPtzTest + : public MediaStreamDevicesControllerTest, + public ::testing::WithParamInterface<bool> { + protected: + bool IsPanTiltZoomSupported() const override { return GetParam(); } +}; + // Request and allow microphone access. IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, RequestAndAllowMic) { InitWithUrl(embedded_test_server()->GetURL("/simple.html")); @@ -256,7 +276,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, RequestAndAllowMic) { prompt_factory()->set_response_type( permissions::PermissionRequestManager::ACCEPT_ALL); RequestPermissions(GetWebContents(), - CreateRequest(example_audio_id(), std::string())); + CreateRequest(example_audio_id(), std::string(), false)); EXPECT_TRUE(GetContentSettings()->IsContentAllowed( ContentSettingsType::MEDIASTREAM_MIC)); @@ -283,7 +303,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, RequestAndAllowCam) { prompt_factory()->set_response_type( permissions::PermissionRequestManager::ACCEPT_ALL); RequestPermissions(GetWebContents(), - CreateRequest(std::string(), example_video_id())); + CreateRequest(std::string(), example_video_id(), false)); EXPECT_TRUE(GetContentSettings()->IsContentAllowed( ContentSettingsType::MEDIASTREAM_CAMERA)); @@ -310,7 +330,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, RequestAndBlockMic) { prompt_factory()->set_response_type( permissions::PermissionRequestManager::ACCEPT_ALL); RequestPermissions(GetWebContents(), - CreateRequest(example_audio_id(), std::string())); + CreateRequest(example_audio_id(), std::string(), false)); EXPECT_FALSE(GetContentSettings()->IsContentAllowed( ContentSettingsType::MEDIASTREAM_MIC)); @@ -338,7 +358,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, RequestAndBlockCam) { prompt_factory()->set_response_type( permissions::PermissionRequestManager::ACCEPT_ALL); RequestPermissions(GetWebContents(), - CreateRequest(std::string(), example_video_id())); + CreateRequest(std::string(), example_video_id(), false)); EXPECT_FALSE(GetContentSettings()->IsContentAllowed( ContentSettingsType::MEDIASTREAM_CAMERA)); @@ -367,8 +387,9 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, // settings are updated. prompt_factory()->set_response_type( permissions::PermissionRequestManager::ACCEPT_ALL); - RequestPermissions(GetWebContents(), - CreateRequest(example_audio_id(), example_video_id())); + RequestPermissions( + GetWebContents(), + CreateRequest(example_audio_id(), example_video_id(), false)); EXPECT_TRUE(GetContentSettings()->IsContentAllowed( ContentSettingsType::MEDIASTREAM_MIC)); @@ -401,8 +422,9 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, // settings are updated. prompt_factory()->set_response_type( permissions::PermissionRequestManager::ACCEPT_ALL); - RequestPermissions(GetWebContents(), - CreateRequest(example_audio_id(), example_video_id())); + RequestPermissions( + GetWebContents(), + CreateRequest(example_audio_id(), example_video_id(), false)); EXPECT_FALSE(GetContentSettings()->IsContentAllowed( ContentSettingsType::MEDIASTREAM_MIC)); @@ -438,8 +460,9 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, // settings are updated. prompt_factory()->set_response_type( permissions::PermissionRequestManager::ACCEPT_ALL); - RequestPermissions(GetWebContents(), - CreateRequest(example_audio_id(), example_video_id())); + RequestPermissions( + GetWebContents(), + CreateRequest(example_audio_id(), example_video_id(), false)); EXPECT_FALSE(GetContentSettings()->IsContentAllowed( ContentSettingsType::MEDIASTREAM_MIC)); @@ -475,8 +498,9 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, // settings are updated. prompt_factory()->set_response_type( permissions::PermissionRequestManager::ACCEPT_ALL); - RequestPermissions(GetWebContents(), - CreateRequest(example_audio_id(), example_video_id())); + RequestPermissions( + GetWebContents(), + CreateRequest(example_audio_id(), example_video_id(), false)); EXPECT_FALSE(GetContentSettings()->IsContentAllowed( ContentSettingsType::MEDIASTREAM_MIC)); @@ -513,7 +537,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, prompt_factory()->set_response_type( permissions::PermissionRequestManager::ACCEPT_ALL); RequestPermissions(GetWebContents(), - CreateRequest(example_audio_id(), std::string())); + CreateRequest(example_audio_id(), std::string(), false)); EXPECT_FALSE(GetContentSettings()->IsContentAllowed( ContentSettingsType::MEDIASTREAM_MIC)); EXPECT_TRUE(GetContentSettings()->IsContentBlocked( @@ -526,7 +550,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, // Request cam and allow SetDevicePolicy(DEVICE_TYPE_VIDEO, ACCESS_ALLOWED); RequestPermissions(GetWebContents(), - CreateRequest(std::string(), example_video_id())); + CreateRequest(std::string(), example_video_id(), false)); EXPECT_TRUE(GetContentSettings()->IsContentAllowed( ContentSettingsType::MEDIASTREAM_CAMERA)); EXPECT_FALSE(GetContentSettings()->IsContentBlocked( @@ -558,7 +582,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, prompt_factory()->set_response_type( permissions::PermissionRequestManager::ACCEPT_ALL); RequestPermissions(GetWebContents(), - CreateRequest(std::string(), example_video_id())); + CreateRequest(std::string(), example_video_id(), false)); EXPECT_TRUE(GetContentSettings()->IsContentAllowed( ContentSettingsType::MEDIASTREAM_CAMERA)); EXPECT_FALSE(GetContentSettings()->IsContentBlocked( @@ -591,7 +615,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, prompt_factory()->set_response_type( permissions::PermissionRequestManager::ACCEPT_ALL); RequestPermissions(GetWebContents(), - CreateRequest(example_audio_id(), std::string())); + CreateRequest(example_audio_id(), std::string(), false)); EXPECT_FALSE(GetContentSettings()->IsContentAllowed( ContentSettingsType::MEDIASTREAM_MIC)); EXPECT_TRUE(GetContentSettings()->IsContentBlocked( @@ -626,23 +650,30 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, // Stores the ContentSettings inputs for a particular test and has functions // which return the expected outputs for that test. struct ContentSettingsTestData { - // The initial value of the mic/cam content settings. + // The initial value of the mic/cam/ptz content settings. ContentSetting mic; ContentSetting cam; + ContentSetting ptz; // Whether the infobar should be accepted if it's shown. bool accept_infobar; - // Whether the infobar should be displayed to request mic/cam for the given - // content settings inputs. - bool ExpectMicInfobar() const { - return mic == CONTENT_SETTING_ASK && cam != CONTENT_SETTING_BLOCK; + // Whether the infobar should be displayed to request mic/cam/ptz for the + // given content settings inputs. + bool ExpectMicInfobar(bool pan_tilt_zoom_supported) const { + return mic == CONTENT_SETTING_ASK && cam != CONTENT_SETTING_BLOCK && + (!pan_tilt_zoom_supported || ptz != CONTENT_SETTING_BLOCK); + } + bool ExpectCamInfobar(bool pan_tilt_zoom_supported) const { + return cam == CONTENT_SETTING_ASK && mic != CONTENT_SETTING_BLOCK && + (!pan_tilt_zoom_supported || ptz != CONTENT_SETTING_BLOCK); } - bool ExpectCamInfobar() const { - return cam == CONTENT_SETTING_ASK && mic != CONTENT_SETTING_BLOCK; + bool ExpectPtzInfobar(bool pan_tilt_zoom_supported) const { + return pan_tilt_zoom_supported && ptz == CONTENT_SETTING_ASK && + mic != CONTENT_SETTING_BLOCK && cam != CONTENT_SETTING_BLOCK; } - // Whether or not the mic/cam should be allowed after clicking accept/deny for - // the given inputs. + // Whether or not the mic/cam/ptz should be allowed after clicking accept/deny + // for the given inputs. bool ExpectMicAllowed() const { return mic == CONTENT_SETTING_ALLOW || (mic == CONTENT_SETTING_ASK && accept_infobar); @@ -651,47 +682,97 @@ struct ContentSettingsTestData { return cam == CONTENT_SETTING_ALLOW || (cam == CONTENT_SETTING_ASK && accept_infobar); } + bool ExpectPtzAllowed() const { + return ptz == CONTENT_SETTING_ALLOW || + (ptz == CONTENT_SETTING_ASK && accept_infobar); + } // The expected media stream result after clicking accept/deny for the given // inputs. - blink::mojom::MediaStreamRequestResult ExpectedMediaStreamResult() const { - if (ExpectMicAllowed() && ExpectCamAllowed()) + blink::mojom::MediaStreamRequestResult ExpectedMediaStreamResult( + bool pan_tilt_zoom_supported) const { + if (ExpectMicAllowed() && ExpectCamAllowed() && + (!pan_tilt_zoom_supported || ptz != CONTENT_SETTING_BLOCK)) { return blink::mojom::MediaStreamRequestResult::OK; + } return blink::mojom::MediaStreamRequestResult::PERMISSION_DENIED; } }; // Test all combinations of cam/mic content settings. Then tests the result of // clicking both accept/deny on the infobar. Both cam/mic are requested. -IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, ContentSettings) { +IN_PROC_BROWSER_TEST_P(MediaStreamDevicesControllerPtzTest, ContentSettings) { InitWithUrl(embedded_test_server()->GetURL("/simple.html")); static const ContentSettingsTestData tests[] = { // Settings that won't result in an infobar. - {CONTENT_SETTING_ALLOW, CONTENT_SETTING_ALLOW, false}, - {CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK, false}, - {CONTENT_SETTING_BLOCK, CONTENT_SETTING_ALLOW, false}, - {CONTENT_SETTING_BLOCK, CONTENT_SETTING_BLOCK, false}, - {CONTENT_SETTING_BLOCK, CONTENT_SETTING_ASK, false}, - {CONTENT_SETTING_ASK, CONTENT_SETTING_BLOCK, false}, + {CONTENT_SETTING_ALLOW, CONTENT_SETTING_ALLOW, CONTENT_SETTING_ALLOW, + false}, + {CONTENT_SETTING_ALLOW, CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK, + false}, + {CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK, CONTENT_SETTING_ALLOW, + false}, + {CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK, CONTENT_SETTING_BLOCK, + false}, + {CONTENT_SETTING_BLOCK, CONTENT_SETTING_ALLOW, CONTENT_SETTING_ALLOW, + false}, + {CONTENT_SETTING_BLOCK, CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK, + false}, + {CONTENT_SETTING_BLOCK, CONTENT_SETTING_BLOCK, CONTENT_SETTING_ALLOW, + false}, + {CONTENT_SETTING_BLOCK, CONTENT_SETTING_BLOCK, CONTENT_SETTING_BLOCK, + false}, + {CONTENT_SETTING_BLOCK, CONTENT_SETTING_ASK, CONTENT_SETTING_ASK, false}, + {CONTENT_SETTING_ASK, CONTENT_SETTING_BLOCK, CONTENT_SETTING_ASK, false}, + {CONTENT_SETTING_ASK, CONTENT_SETTING_ASK, CONTENT_SETTING_BLOCK, false}, // Settings that will result in an infobar. Test both accept and deny. - {CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, false}, - {CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, true}, + {CONTENT_SETTING_ALLOW, CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, + false}, + {CONTENT_SETTING_ALLOW, CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, true}, + + {CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, CONTENT_SETTING_ALLOW, + false}, + {CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, CONTENT_SETTING_ALLOW, true}, - {CONTENT_SETTING_ASK, CONTENT_SETTING_ASK, false}, - {CONTENT_SETTING_ASK, CONTENT_SETTING_ASK, true}, + {CONTENT_SETTING_ASK, CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, false}, + {CONTENT_SETTING_ASK, CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, true}, - {CONTENT_SETTING_ASK, CONTENT_SETTING_ALLOW, false}, - {CONTENT_SETTING_ASK, CONTENT_SETTING_ALLOW, true}, + {CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, CONTENT_SETTING_ASK, false}, + {CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, CONTENT_SETTING_ASK, true}, + + {CONTENT_SETTING_ASK, CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, false}, + {CONTENT_SETTING_ASK, CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, true}, + + {CONTENT_SETTING_ASK, CONTENT_SETTING_ASK, CONTENT_SETTING_ALLOW, false}, + {CONTENT_SETTING_ASK, CONTENT_SETTING_ASK, CONTENT_SETTING_ALLOW, true}, + + {CONTENT_SETTING_ASK, CONTENT_SETTING_ASK, CONTENT_SETTING_ASK, false}, + {CONTENT_SETTING_ASK, CONTENT_SETTING_ASK, CONTENT_SETTING_ASK, true}, }; + // Prevent automatic camera permission change when camera PTZ gets updated. + CameraPanTiltZoomPermissionContext* camera_pan_tilt_zoom_permission_context = + static_cast<CameraPanTiltZoomPermissionContext*>( + PermissionManagerFactory::GetForProfile( + Profile::FromBrowserContext( + GetWebContents()->GetBrowserContext())) + ->GetPermissionContextForTesting( + ContentSettingsType::CAMERA_PAN_TILT_ZOOM)); + HostContentSettingsMap* content_settings = + HostContentSettingsMapFactory::GetForProfile( + Profile::FromBrowserContext(GetWebContents()->GetBrowserContext())); + content_settings->RemoveObserver(camera_pan_tilt_zoom_permission_context); + + const bool pan_tilt_zoom_supported = IsPanTiltZoomSupported(); for (auto& test : tests) { - SetContentSettings(test.mic, test.cam); + SetContentSettings(test.mic, test.cam, test.ptz); prompt_factory()->ResetCounts(); // Accept or deny the infobar if it's showing. - if (test.ExpectMicInfobar() || test.ExpectCamInfobar()) { + if (test.ExpectMicInfobar(pan_tilt_zoom_supported) || + test.ExpectCamInfobar(pan_tilt_zoom_supported) || + test.ExpectPtzInfobar(pan_tilt_zoom_supported)) { if (test.accept_infobar) { prompt_factory()->set_response_type( permissions::PermissionRequestManager::ACCEPT_ALL); @@ -703,33 +784,45 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, ContentSettings) { prompt_factory()->set_response_type( permissions::PermissionRequestManager::NONE); } - RequestPermissions(GetWebContents(), - CreateRequest(example_audio_id(), example_video_id())); + RequestPermissions( + GetWebContents(), + CreateRequest(example_audio_id(), example_video_id(), true)); - ASSERT_LE(prompt_factory()->TotalRequestCount(), 2); - ASSERT_EQ( - test.ExpectMicInfobar(), + ASSERT_LE(prompt_factory()->TotalRequestCount(), 3); + EXPECT_EQ( + test.ExpectMicInfobar(pan_tilt_zoom_supported), prompt_factory()->RequestTypeSeen( permissions::PermissionRequestType::PERMISSION_MEDIASTREAM_MIC)); - ASSERT_EQ( - test.ExpectCamInfobar(), + EXPECT_EQ( + test.ExpectCamInfobar(pan_tilt_zoom_supported), prompt_factory()->RequestTypeSeen( permissions::PermissionRequestType::PERMISSION_MEDIASTREAM_CAMERA)); + EXPECT_EQ( + test.ExpectPtzInfobar(pan_tilt_zoom_supported), + prompt_factory()->RequestTypeSeen(permissions::PermissionRequestType:: + PERMISSION_CAMERA_PAN_TILT_ZOOM)); // Check the media stream result is expected and the devices returned are // expected; - VerifyResultState(test.ExpectedMediaStreamResult(), - test.ExpectMicAllowed() && test.ExpectCamAllowed(), - test.ExpectMicAllowed() && test.ExpectCamAllowed()); + VerifyResultState( + test.ExpectedMediaStreamResult(pan_tilt_zoom_supported), + test.ExpectMicAllowed() && test.ExpectCamAllowed() && + (!pan_tilt_zoom_supported || test.ptz != CONTENT_SETTING_BLOCK), + test.ExpectMicAllowed() && test.ExpectCamAllowed() && + (!pan_tilt_zoom_supported || test.ptz != CONTENT_SETTING_BLOCK)); } } +INSTANTIATE_TEST_SUITE_P(All, + MediaStreamDevicesControllerPtzTest, + ::testing::Values(false, true)); + // Request and allow camera access on WebUI pages without prompting. IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, WebUIRequestAndAllowCam) { InitWithUrl(GURL(chrome::kChromeUIVersionURL)); RequestPermissions(GetWebContents(), - CreateRequest(std::string(), example_video_id())); + CreateRequest(std::string(), example_video_id(), false)); ASSERT_EQ(0, prompt_factory()->TotalRequestCount()); @@ -745,8 +838,9 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, // Test that a prompt is required. prompt_factory()->set_response_type( permissions::PermissionRequestManager::ACCEPT_ALL); - RequestPermissions(GetWebContents(), - CreateRequest(example_audio_id(), example_video_id())); + RequestPermissions( + GetWebContents(), + CreateRequest(example_audio_id(), example_video_id(), false)); ASSERT_EQ(2, prompt_factory()->TotalRequestCount()); ASSERT_TRUE(prompt_factory()->RequestTypeSeen( permissions::PermissionRequestType::PERMISSION_MEDIASTREAM_CAMERA)); @@ -757,8 +851,9 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, // Check that re-requesting allows without prompting. prompt_factory()->ResetCounts(); - RequestPermissions(GetWebContents(), - CreateRequest(example_audio_id(), example_video_id())); + RequestPermissions( + GetWebContents(), + CreateRequest(example_audio_id(), example_video_id(), false)); ASSERT_EQ(0, prompt_factory()->TotalRequestCount()); VerifyResultState(blink::mojom::MediaStreamRequestResult::OK, true, true); @@ -773,7 +868,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, RequestPermissions( GetWebContents(), - CreateRequestWithType(example_audio_id(), example_video_id(), + CreateRequestWithType(example_audio_id(), example_video_id(), false, blink::MEDIA_OPEN_DEVICE_PEPPER_ONLY)); ASSERT_EQ(0, prompt_factory()->TotalRequestCount()); @@ -788,7 +883,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, WebContentsDestroyed) { permissions::PermissionRequestManager::ACCEPT_ALL); content::MediaStreamRequest request = - CreateRequest(example_audio_id(), example_video_id()); + CreateRequest(example_audio_id(), example_video_id(), false); // Simulate a destroyed RenderFrameHost. request.render_frame_id = 0; request.render_process_id = 0; @@ -818,7 +913,8 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, // Now request permissions, but before the callback is asynchronously called, // destroy the tab. permission_bubble_media_access_handler_->HandleRequest( - prompt_contents, CreateRequest(example_audio_id(), example_video_id()), + prompt_contents, + CreateRequest(example_audio_id(), example_video_id(), false), base::BindOnce([](const blink::MediaStreamDevices& devices, blink::mojom::MediaStreamRequestResult result, std::unique_ptr<content::MediaStreamUI> ui) { @@ -857,8 +953,9 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, InitWithUrl(embedded_test_server()->GetURL("/simple.html")); SetDevicePolicy(DEVICE_TYPE_AUDIO, ACCESS_ALLOWED); SetDevicePolicy(DEVICE_TYPE_VIDEO, ACCESS_ALLOWED); - RequestPermissions(GetWebContents(), - CreateRequest(example_audio_id(), example_video_id())); + RequestPermissions( + GetWebContents(), + CreateRequest(example_audio_id(), example_video_id(), false)); ASSERT_EQ(0, prompt_factory()->TotalRequestCount()); VerifyResultState(blink::mojom::MediaStreamRequestResult::KILL_SWITCH_ON, @@ -881,7 +978,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, ChildFrameAt(GetWebContents()->GetMainFrame(), 0); content::MediaStreamRequest request = - CreateRequest(example_audio_id(), example_video_id()); + CreateRequest(example_audio_id(), example_video_id(), false); // Make the child frame the source of the request. request.render_process_id = child_frame->GetProcess()->GetID(); request.render_frame_id = child_frame->GetRoutingID(); @@ -911,7 +1008,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, ChildFrameAt(GetWebContents()->GetMainFrame(), 0); content::MediaStreamRequest request = - CreateRequest(std::string(), example_video_id()); + CreateRequest(std::string(), example_video_id(), false); // Make the child frame the source of the request. request.render_process_id = child_frame->GetProcess()->GetID(); request.render_frame_id = child_frame->GetRoutingID(); @@ -931,7 +1028,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, InitWithUrl(GURL(chrome::kChromeUIVersionURL)); RequestPermissions( GetWebContents(), - CreateRequestWithType(example_audio_id(), std::string(), + CreateRequestWithType(example_audio_id(), std::string(), false, blink::MEDIA_OPEN_DEVICE_PEPPER_ONLY)); VerifyResultState(blink::mojom::MediaStreamRequestResult::OK, true, false); } @@ -942,7 +1039,7 @@ IN_PROC_BROWSER_TEST_F(MediaStreamDevicesControllerTest, InitWithUrl(GURL(chrome::kChromeUIVersionURL)); RequestPermissions( GetWebContents(), - CreateRequestWithType(std::string(), example_video_id(), + CreateRequestWithType(std::string(), example_video_id(), false, blink::MEDIA_OPEN_DEVICE_PEPPER_ONLY)); VerifyResultState(blink::mojom::MediaStreamRequestResult::OK, false, true); } diff --git a/chromium/chrome/browser/media/webrtc/native_desktop_media_list.cc b/chromium/chrome/browser/media/webrtc/native_desktop_media_list.cc index 224d52b8353..af3a6dab055 100644 --- a/chromium/chrome/browser/media/webrtc/native_desktop_media_list.cc +++ b/chromium/chrome/browser/media/webrtc/native_desktop_media_list.cc @@ -11,7 +11,6 @@ #include "base/message_loop/message_pump_type.h" #include "base/single_thread_task_runner.h" #include "base/strings/utf_string_conversions.h" -#include "base/task/post_task.h" #include "base/threading/thread_restrictions.h" #include "build/build_config.h" #include "chrome/browser/media/webrtc/desktop_media_list.h" @@ -33,7 +32,6 @@ #include "ui/snapshot/snapshot_aura.h" #endif -using content::BrowserThread; using content::DesktopMediaID; namespace { @@ -99,6 +97,8 @@ class NativeDesktopMediaList::Worker private: typedef std::map<DesktopMediaID, uint32_t> ImageHashesMap; + bool IsCurrentFrameValid() const; + // webrtc::DesktopCapturer::Callback interface. void OnCaptureResult(webrtc::DesktopCapturer::Result result, std::unique_ptr<webrtc::DesktopFrame> frame) override; @@ -126,7 +126,9 @@ NativeDesktopMediaList::Worker::Worker( : task_runner_(task_runner), media_list_(media_list), type_(type), - capturer_(std::move(capturer)) {} + capturer_(std::move(capturer)) { + DCHECK(capturer_); +} NativeDesktopMediaList::Worker::~Worker() { DCHECK(task_runner_->BelongsToCurrentThread()); @@ -178,8 +180,8 @@ void NativeDesktopMediaList::Worker::Refresh( SourceDescription(DesktopMediaID(type_, sources[i].id), title)); } - base::PostTask(FROM_HERE, {BrowserThread::UI}, - base::BindOnce(&NativeDesktopMediaList::RefreshForAuraWindows, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(&NativeDesktopMediaList::RefreshForAuraWindows, media_list_, result, update_thumnails)); } @@ -198,7 +200,7 @@ void NativeDesktopMediaList::Worker::RefreshThumbnails( // Expect that DesktopCapturer to always captures frames synchronously. // |current_frame_| may be NULL if capture failed (e.g. because window has // been closed). - if (current_frame_) { + if (IsCurrentFrameValid()) { uint32_t frame_hash = GetFrameHash(current_frame_.get()); new_image_hashes[id] = frame_hash; @@ -207,8 +209,8 @@ void NativeDesktopMediaList::Worker::RefreshThumbnails( if (it == image_hashes_.end() || it->second != frame_hash) { gfx::ImageSkia thumbnail = ScaleDesktopFrame(std::move(current_frame_), thumbnail_size); - base::PostTask( - FROM_HERE, {BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(&NativeDesktopMediaList::UpdateSourceThumbnail, media_list_, id, thumbnail)); } @@ -217,12 +219,21 @@ void NativeDesktopMediaList::Worker::RefreshThumbnails( image_hashes_.swap(new_image_hashes); - base::PostTask( - FROM_HERE, {BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(&NativeDesktopMediaList::UpdateNativeThumbnailsFinished, media_list_)); } +bool NativeDesktopMediaList::Worker::IsCurrentFrameValid() const { + // These checks ensure invalid data isn't passed along, potentially leading to + // crashes, e.g. when we calculate the hash which assumes a positive height + // and stride. + // TODO(crbug.com/1085230): figure out why the height is sometimes negative. + return current_frame_ && current_frame_->data() && + current_frame_->stride() >= 0 && current_frame_->size().height() >= 0; +} + void NativeDesktopMediaList::Worker::OnCaptureResult( webrtc::DesktopCapturer::Result result, std::unique_ptr<webrtc::DesktopFrame> frame) { @@ -309,8 +320,8 @@ void NativeDesktopMediaList::RefreshForAuraWindows( #if defined(USE_AURA) pending_native_thumbnail_capture_ = true; #endif - base::PostTask( - FROM_HERE, {BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(&NativeDesktopMediaList::UpdateNativeThumbnailsFinished, weak_factory_.GetWeakPtr())); return; diff --git a/chromium/chrome/browser/media/webrtc/native_desktop_media_list.h b/chromium/chrome/browser/media/webrtc/native_desktop_media_list.h index cfd8378fe93..77eca3d6a99 100644 --- a/chromium/chrome/browser/media/webrtc/native_desktop_media_list.h +++ b/chromium/chrome/browser/media/webrtc/native_desktop_media_list.h @@ -21,6 +21,7 @@ class DesktopCapturer; // native windows. class NativeDesktopMediaList : public DesktopMediaListBase { public: + // |capturer| must exist. NativeDesktopMediaList(content::DesktopMediaID::Type type, std::unique_ptr<webrtc::DesktopCapturer> capturer); ~NativeDesktopMediaList() override; diff --git a/chromium/chrome/browser/media/webrtc/native_desktop_media_list_unittest.cc b/chromium/chrome/browser/media/webrtc/native_desktop_media_list_unittest.cc index 5de558d6eaf..c550d687356 100644 --- a/chromium/chrome/browser/media/webrtc/native_desktop_media_list_unittest.cc +++ b/chromium/chrome/browser/media/webrtc/native_desktop_media_list_unittest.cc @@ -211,6 +211,8 @@ class NativeDesktopMediaListTest : public ChromeViewsTestBase { gfx::AcceleratedWidget widget = host->GetAcceleratedWidget(); #if defined(OS_WIN) window.id = reinterpret_cast<DesktopMediaID::Id>(widget); +#elif defined(USE_X11) + window.id = static_cast<uint32_t>(widget); #else window.id = widget; #endif @@ -232,6 +234,8 @@ class NativeDesktopMediaListTest : public ChromeViewsTestBase { aura_window->GetHost()->GetAcceleratedWidget(); #if defined(OS_WIN) int native_id = reinterpret_cast<DesktopMediaID::Id>(widget); +#elif defined(USE_X11) + int native_id = static_cast<uint32_t>(widget); #else int native_id = widget; #endif diff --git a/chromium/chrome/browser/media/webrtc/permission_bubble_media_access_handler.cc b/chromium/chrome/browser/media/webrtc/permission_bubble_media_access_handler.cc index dacacfb851b..9e580a8037a 100644 --- a/chromium/chrome/browser/media/webrtc/permission_bubble_media_access_handler.cc +++ b/chromium/chrome/browser/media/webrtc/permission_bubble_media_access_handler.cc @@ -10,7 +10,6 @@ #include "base/bind.h" #include "base/callback_helpers.h" #include "base/metrics/field_trial.h" -#include "base/task/post_task.h" #include "build/build_config.h" #include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h" #include "chrome/browser/media/webrtc/media_stream_capture_indicator.h" @@ -417,8 +416,8 @@ void PermissionBubbleMediaAccessHandler::OnAccessRequestResponse( // Post a task to process next queued request. It has to be done // asynchronously to make sure that calling infobar is not destroyed until // after this function returns. - base::PostTask( - FROM_HERE, {BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce( &PermissionBubbleMediaAccessHandler::ProcessQueuedAccessRequest, base::Unretained(this), web_contents)); diff --git a/chromium/chrome/browser/media/webrtc/tab_desktop_media_list_unittest.cc b/chromium/chrome/browser/media/webrtc/tab_desktop_media_list_unittest.cc index 71dc81b6968..20033251c03 100644 --- a/chromium/chrome/browser/media/webrtc/tab_desktop_media_list_unittest.cc +++ b/chromium/chrome/browser/media/webrtc/tab_desktop_media_list_unittest.cc @@ -25,6 +25,7 @@ #include "content/public/browser/web_contents.h" #include "content/public/common/content_switches.h" #include "content/public/test/browser_task_environment.h" +#include "content/public/test/navigation_simulator.h" #include "content/public/test/test_renderer_host.h" #include "content/public/test/web_contents_tester.h" #include "testing/gmock/include/gmock/gmock.h" @@ -123,19 +124,13 @@ class TabDesktopMediaListTest : public testing::Test { WebContentsTester::For(contents.get()) ->SetLastActiveTime(base::TimeTicks::Now()); - // Get or create the transient NavigationEntry and add a title and a - // favicon to it. + // Get or create a NavigationEntry and add a title and a favicon to it. content::NavigationEntry* entry = - contents->GetController().GetTransientEntry(); + contents->GetController().GetLastCommittedEntry(); if (!entry) { - std::unique_ptr<content::NavigationEntry> entry_new = - content::NavigationController::CreateNavigationEntry( - GURL("chrome://blank"), content::Referrer(), base::nullopt, - ui::PAGE_TRANSITION_LINK, false, std::string(), profile_, - nullptr /* blob_url_loader_factory */); - - contents->GetController().SetTransientEntry(std::move(entry_new)); - entry = contents->GetController().GetTransientEntry(); + content::NavigationSimulator::NavigateAndCommitFromBrowser( + contents.get(), GURL("chrome://blank")); + entry = contents->GetController().GetLastCommittedEntry(); } contents->UpdateTitleForEntry(entry, base::ASCIIToUTF16("Test tab")); @@ -336,7 +331,7 @@ TEST_F(TabDesktopMediaListTest, UpdateTitle) { tab_strip_model->GetWebContentsAt(kDefaultSourceCount - 1); ASSERT_TRUE(contents); content::NavigationController& controller = contents->GetController(); - contents->UpdateTitleForEntry(controller.GetTransientEntry(), + contents->UpdateTitleForEntry(controller.GetLastCommittedEntry(), base::ASCIIToUTF16("New test tab")); EXPECT_CALL(observer_, OnSourceNameChanged(list_.get(), 0)) @@ -361,7 +356,8 @@ TEST_F(TabDesktopMediaListTest, UpdateThumbnail) { content::FaviconStatus favicon_info; favicon_info.image = CreateGrayscaleImage(gfx::Size(10, 10), 100); - contents->GetController().GetTransientEntry()->GetFavicon() = favicon_info; + contents->GetController().GetLastCommittedEntry()->GetFavicon() = + favicon_info; EXPECT_CALL(observer_, OnSourceThumbnailChanged(list_.get(), 0)) .WillOnce(QuitMessageLoop()); diff --git a/chromium/chrome/browser/media/webrtc/webrtc_disable_encryption_flag_browsertest.cc b/chromium/chrome/browser/media/webrtc/webrtc_disable_encryption_flag_browsertest.cc index ad67e37d4c3..599d2215327 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_disable_encryption_flag_browsertest.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_disable_encryption_flag_browsertest.cc @@ -48,9 +48,10 @@ class WebRtcDisableEncryptionFlagBrowserTest : public WebRtcTestBase { // Makes a call and checks that there's encryption or not in the SDP offer. // TODO(crbug.com/910216): De-flake this for ChromeOs. -// TODO(crbug.com/984879): De-flake this for MSAN Linux. +// TODO(crbug.com/984879): De-flake this for ASAN/MSAN Linux. #if defined(OS_CHROMEOS) || \ - (defined(OS_LINUX) && defined(MEMORY_SANITIZER)) || \ + (defined(OS_LINUX) && (defined(MEMORY_SANITIZER) || \ + defined(ADDRESS_SANITIZER))) || \ (defined(OS_WIN) && defined(ADDRESS_SANITIZER)) #define MAYBE_VerifyEncryption DISABLED_VerifyEncryption #else diff --git a/chromium/chrome/browser/media/webrtc/webrtc_event_log_history.cc b/chromium/chrome/browser/media/webrtc/webrtc_event_log_history.cc index 59f14e44920..98b9a55c054 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_event_log_history.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_event_log_history.cc @@ -130,12 +130,11 @@ bool WebRtcEventLogHistoryFileWriter::Init() { DCHECK(!valid_); if (base::PathExists(path_)) { - if (!base::DeleteFile(path_, /*recursive=*/false)) { + if (!base::DeleteFile(path_)) { LOG(ERROR) << "History file already exists, and could not be deleted."; return false; - } else { - LOG(WARNING) << "History file already existed; deleted."; } + LOG(WARNING) << "History file already existed; deleted."; } // Attempt to create the file. @@ -144,7 +143,7 @@ bool WebRtcEventLogHistoryFileWriter::Init() { file_.Initialize(path_, file_flags); if (!file_.IsValid() || !file_.created()) { LOG(WARNING) << "Couldn't create history file."; - if (!base::DeleteFile(path_, /*recursive=*/false)) { + if (!base::DeleteFile(path_)) { LOG(ERROR) << "Failed to delete " << path_ << "."; } return false; @@ -218,7 +217,7 @@ bool WebRtcEventLogHistoryFileWriter::WriteUploadId( } void WebRtcEventLogHistoryFileWriter::Delete() { - if (!base::DeleteFile(path_, /*recursive=*/false)) { + if (!base::DeleteFile(path_)) { LOG(ERROR) << "History file could not be deleted."; } @@ -301,7 +300,7 @@ bool WebRtcEventLogHistoryFileReader::Init() { base::File file(path_, file_flags); if (!file.IsValid()) { LOG(WARNING) << "Couldn't read history file."; - if (!base::DeleteFile(path_, /*recursive=*/false)) { + if (!base::DeleteFile(path_)) { LOG(ERROR) << "Failed to delete " << path_ << "."; } return false; diff --git a/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager.cc b/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager.cc index 08e960e8871..f17670feecb 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager.cc @@ -40,8 +40,8 @@ class PeerConnectionTrackerProxyImpl void EnableWebRtcEventLogging(const WebRtcEventLogPeerConnectionKey& key, int output_period_ms) override { - base::PostTask( - FROM_HERE, {BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce( &PeerConnectionTrackerProxyImpl::EnableWebRtcEventLoggingInternal, key, output_period_ms)); @@ -49,8 +49,8 @@ class PeerConnectionTrackerProxyImpl void DisableWebRtcEventLogging( const WebRtcEventLogPeerConnectionKey& key) override { - base::PostTask( - FROM_HERE, {BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce( &PeerConnectionTrackerProxyImpl::DisableWebRtcEventLoggingInternal, key)); @@ -412,8 +412,8 @@ void WebRtcEventLogManager::StartRemoteLogging( } if (error) { - base::PostTask(FROM_HERE, {BrowserThread::UI}, - base::BindOnce(std::move(reply), false, std::string(), + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(std::move(reply), false, std::string(), std::string(error))); return; } @@ -888,8 +888,8 @@ void WebRtcEventLogManager::StartRemoteLoggingInternal( DCHECK_EQ(result, !log_id.empty()); DCHECK_EQ(!result, !error_message.empty()); - base::PostTask( - FROM_HERE, {BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(std::move(reply), result, log_id, error_message)); } @@ -935,7 +935,7 @@ void WebRtcEventLogManager::SetLocalLogsObserverInternal( local_logs_observer_ = observer; if (reply) { - base::PostTask(FROM_HERE, {BrowserThread::UI}, std::move(reply)); + content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE, std::move(reply)); } } @@ -947,7 +947,7 @@ void WebRtcEventLogManager::SetRemoteLogsObserverInternal( remote_logs_observer_ = observer; if (reply) { - base::PostTask(FROM_HERE, {BrowserThread::UI}, std::move(reply)); + content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE, std::move(reply)); } } @@ -960,7 +960,7 @@ void WebRtcEventLogManager::SetClockForTesting(base::Clock* clock, base::OnceClosure reply) { manager->local_logs_manager_.SetClockForTesting(clock); - base::PostTask(FROM_HERE, {BrowserThread::UI}, std::move(reply)); + content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE, std::move(reply)); }; // |this| is destroyed by ~BrowserProcessImpl(), so base::Unretained(this) @@ -980,7 +980,7 @@ void WebRtcEventLogManager::SetPeerConnectionTrackerProxyForTesting( base::OnceClosure reply) { manager->pc_tracker_proxy_ = std::move(pc_tracker_proxy); - base::PostTask(FROM_HERE, {BrowserThread::UI}, std::move(reply)); + content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE, std::move(reply)); }; // |this| is destroyed by ~BrowserProcessImpl(), so base::Unretained(this) @@ -1004,7 +1004,8 @@ void WebRtcEventLogManager::SetWebRtcEventLogUploaderFactoryForTesting( remote_logs_manager.SetWebRtcEventLogUploaderFactoryForTesting( std::move(uploader_factory)); - base::PostTask(FROM_HERE, {BrowserThread::UI}, std::move(reply)); + content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE, + std::move(reply)); }; // |this| is destroyed by ~BrowserProcessImpl(), so base::Unretained(this) diff --git a/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_common.cc b/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_common.cc index 12e076407cb..b11ec75be0c 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_common.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_common.cc @@ -239,7 +239,7 @@ bool BaseLogFileWriter::Init() { file_.Initialize(path_, file_flags); if (!file_.IsValid() || !file_.created()) { LOG(WARNING) << "Couldn't create remote-bound WebRTC event log file."; - if (!base::DeleteFile(path_, /*recursive=*/false)) { + if (!base::DeleteFile(path_)) { LOG(ERROR) << "Failed to delete " << path_ << "."; } SetState(State::ERRORED); @@ -312,7 +312,7 @@ void BaseLogFileWriter::Delete() { file_.Close(); } - if (!base::DeleteFile(path_, /*recursive=*/false)) { + if (!base::DeleteFile(path_)) { LOG(ERROR) << "Failed to delete " << path_ << "."; } diff --git a/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_common_unittest.cc b/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_common_unittest.cc index 3652333ba1f..440083173d3 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_common_unittest.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_common_unittest.cc @@ -433,7 +433,7 @@ TEST_P(LogFileWriterTest, WriteDoesNotCrashIfFileRemovedExternally) { auto writer = CreateWriter(kMaxRemoteLogFileSizeBytes); ASSERT_TRUE(writer); - ASSERT_TRUE(base::DeleteFile(path_, /*recursive=*/false)); + ASSERT_TRUE(base::DeleteFile(path_)); ASSERT_FALSE(base::PathExists(path_)); // Sanity on the test itself. // It's up to the OS whether this will succeed or fail, but it must not crash. @@ -446,7 +446,7 @@ TEST_P(LogFileWriterTest, CloseDoesNotCrashIfFileRemovedExternally) { auto writer = CreateWriter(kMaxRemoteLogFileSizeBytes); ASSERT_TRUE(writer); - ASSERT_TRUE(base::DeleteFile(path_, /*recursive=*/false)); + ASSERT_TRUE(base::DeleteFile(path_)); ASSERT_FALSE(base::PathExists(path_)); // Sanity on the test itself. // It's up to the OS whether this will succeed or fail, but it must not crash. @@ -459,7 +459,7 @@ TEST_P(LogFileWriterTest, DeleteDoesNotCrashIfFileRemovedExternally) { auto writer = CreateWriter(kMaxRemoteLogFileSizeBytes); ASSERT_TRUE(writer); - ASSERT_TRUE(base::DeleteFile(path_, /*recursive=*/false)); + ASSERT_TRUE(base::DeleteFile(path_)); ASSERT_FALSE(base::PathExists(path_)); // Sanity on the test itself. // It's up to the OS whether this will succeed or fail, but it must not crash. diff --git a/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_remote.cc b/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_remote.cc index 5118eb322bc..5230c7e9959 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_remote.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_remote.cc @@ -17,7 +17,6 @@ #include "base/logging.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" -#include "base/task/post_task.h" #include "chrome/common/chrome_switches.h" #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" @@ -204,7 +203,7 @@ WebRtcRemoteEventLogManager::WebRtcRemoteEventLogManager( uploading_supported_for_connection_type_(false), scheduled_upload_tasks_(0), uploader_factory_( - std::make_unique<WebRtcEventLogUploaderImpl::Factory>()), + std::make_unique<WebRtcEventLogUploaderImpl::Factory>(task_runner)), task_runner_(task_runner), weak_ptr_factory_( std::make_unique<base::WeakPtrFactory<WebRtcRemoteEventLogManager>>( @@ -407,13 +406,14 @@ bool WebRtcRemoteEventLogManager::PeerConnectionSessionIdSet( return false; // Unknown peer connection; already closed? } - if (!peer_connection->second.empty()) { - LOG(ERROR) << "Session ID already set."; + if (peer_connection->second.empty()) { + peer_connection->second = session_id; + } else if (session_id != peer_connection->second) { + LOG(ERROR) << "Session ID already set to " << peer_connection->second + << ". Cannot change to " << session_id << "."; return false; } - peer_connection->second = session_id; - return true; } @@ -544,9 +544,10 @@ void WebRtcRemoteEventLogManager::GetHistory( std::vector<UploadList::UploadInfo> history; if (!BrowserContextEnabled(browser_context_id)) { - LOG(ERROR) << "Unknown |browser_context_id|."; - base::PostTask(FROM_HERE, {content::BrowserThread::UI}, - base::BindOnce(std::move(reply), history)); + // Either the browser context is unknown, or more likely, it's not + // enabled for remote logging. + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(std::move(reply), history)); return; } @@ -594,8 +595,8 @@ void WebRtcRemoteEventLogManager::GetHistory( }; std::sort(history.begin(), history.end(), cmp); - base::PostTask(FROM_HERE, {content::BrowserThread::UI}, - base::BindOnce(std::move(reply), history)); + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(std::move(reply), history)); } void WebRtcRemoteEventLogManager::RemovePendingLogsForNotEnabledBrowserContext( @@ -667,16 +668,16 @@ void WebRtcRemoteEventLogManager::SetWebRtcEventLogUploaderFactoryForTesting( void WebRtcRemoteEventLogManager::UploadConditionsHoldForTesting( base::OnceCallback<void(bool)> callback) { DCHECK(task_runner_->RunsTasksInCurrentSequence()); - base::PostTask(FROM_HERE, {content::BrowserThread::UI}, - base::BindOnce(std::move(callback), UploadConditionsHold())); + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(std::move(callback), UploadConditionsHold())); } void WebRtcRemoteEventLogManager::ShutDownForTesting(base::OnceClosure reply) { DCHECK(task_runner_->RunsTasksInCurrentSequence()); weak_ptr_factory_->InvalidateWeakPtrs(); weak_ptr_factory_.reset(); - base::PostTask(FROM_HERE, {content::BrowserThread::UI}, - base::BindOnce(std::move(reply))); + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(std::move(reply))); } bool WebRtcRemoteEventLogManager::AreLogParametersValid( @@ -748,7 +749,7 @@ WebRtcRemoteEventLogManager::CloseLogFile(LogFilesMap::iterator it, DCHECK(emplace_result.second); // No pre-existing entry. } else { const base::FilePath log_file_path = it->second->path(); - if (!base::DeleteFile(log_file_path, /*recursive=*/false)) { + if (!base::DeleteFile(log_file_path)) { LOG(ERROR) << "Failed to delete " << log_file_path << "."; } } @@ -834,14 +835,14 @@ void WebRtcRemoteEventLogManager::LoadLogsDirectory( } // Remove the log file itself. - if (!base::DeleteFile(log_file_path, /*recursive=*/false)) { + if (!base::DeleteFile(log_file_path)) { LOG(ERROR) << "Failed to delete " << file_to_delete.first << "."; } } // Remove expired history files. for (const base::FilePath& history_file_path : history_files_to_delete) { - if (!base::DeleteFile(history_file_path, /*recursive=*/false)) { + if (!base::DeleteFile(history_file_path)) { LOG(ERROR) << "Failed to delete " << history_file_path << "."; } } @@ -1188,18 +1189,9 @@ void WebRtcRemoteEventLogManager::MaybeCancelUpload( return; } - // Cancel the upload. - // * If the upload has asynchronously completed by now, the uploader would - // have posted a task back to our queue to delete it and move on to the - // next file; cancellation is reported as unsuccessful in that case. In that - // case, we avoid resetting |uploader_| until that callback task executes. - // * If the upload was still underway when we cancelled it, then we can - // safely reset |uploader_| and move on to the next file the next time - // ManageUploadSchedule() is called. - const bool cancelled = uploader_->Cancel(); - if (cancelled) { - uploader_.reset(); - } + // Cancel the upload. `uploader_` will be released when the callback, + // `OnWebRtcEventLogUploadComplete`, is posted back. + uploader_->Cancel(); } bool WebRtcRemoteEventLogManager::MatchesFilter( @@ -1319,6 +1311,7 @@ void WebRtcRemoteEventLogManager::MaybeStartUploading() { // TODO(crbug.com/775415): Rename the file before uploading, so that we // would not retry the upload after restarting Chrome, if the upload is // interrupted. + currently_uploaded_file_ = pending_logs_.begin()->path; uploader_ = uploader_factory_->Create(*pending_logs_.begin(), std::move(callback)); pending_logs_.erase(pending_logs_.begin()); @@ -1332,7 +1325,20 @@ void WebRtcRemoteEventLogManager::OnWebRtcEventLogUploadComplete( bool upload_successful) { DCHECK(task_runner_->RunsTasksInCurrentSequence()); DCHECK(uploader_); + + // Make sure this callback refers to the currently uploaded file. This might + // not be the case if the upload was cancelled right after succeeding, in + // which case we'll get two callbacks, one reporting success and one failure. + // It can also be that the uploader was cancelled more than once, e.g. if + // the user cleared cache while PrefService were changing. + if (!uploader_ || + uploader_->GetWebRtcLogFileInfo().path != currently_uploaded_file_) { + return; + } + uploader_.reset(); + currently_uploaded_file_.clear(); + ManageUploadSchedule(); } diff --git a/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_remote.h b/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_remote.h index ff1f93a93e6..c042e583469 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_remote.h +++ b/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_remote.h @@ -443,6 +443,12 @@ class WebRtcRemoteEventLogManager final // currently busy uploading it to a remote server. std::unique_ptr<WebRtcEventLogUploader> uploader_; + // The path to the file which is currently being uploaded. + // Used to ensure a callback from the uploader refers to the current + // file, rather than a second callback from the previously uploaded file, + // e.g. when when Cancel() is called right after the upload finishes. + base::FilePath currently_uploaded_file_; + // Provides notifications of network changes. network::NetworkConnectionTracker* network_connection_tracker_; diff --git a/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_unittest.cc b/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_unittest.cc index c4757115f34..0caf07db158 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_unittest.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_event_log_manager_unittest.cc @@ -69,7 +69,6 @@ #if defined(OS_CHROMEOS) #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h" #include "chrome/browser/chromeos/profiles/profile_helper.h" -#include "chrome/test/base/testing_profile.h" #include "components/account_id/account_id.h" #include "components/user_manager/scoped_user_manager.h" #endif @@ -189,14 +188,9 @@ class NullWebRtcEventLogUploader : public WebRtcEventLogUploader { return log_file_; } - bool Cancel() override { + void Cancel() override { EXPECT_TRUE(cancellation_expected_); - if (was_cancelled_) { // Should not be called more than once. - EXPECT_TRUE(false); - return false; - } was_cancelled_ = true; - return true; } class Factory : public WebRtcEventLogUploader::Factory { @@ -1202,14 +1196,14 @@ class WebRtcEventLogManagerTestIncognito incognito_rph_.reset(); if (incognito_profile_) { DCHECK(browser_context_); - browser_context_->DestroyOffTheRecordProfile(); + browser_context_->DestroyOffTheRecordProfile(incognito_profile_); } } void SetUp() override { WebRtcEventLogManagerTestBase::SetUp(); - incognito_profile_ = browser_context_->GetOffTheRecordProfile(); + incognito_profile_ = browser_context_->GetPrimaryOTRProfile(); incognito_rph_ = std::make_unique<MockRenderProcessHost>(incognito_profile_); } @@ -1311,7 +1305,7 @@ class FileListExpectingWebRtcEventLogUploader : public WebRtcEventLogUploader { // we cannot verify |log_file.browser_context_id| is correct. // This is unimportant to the test. - base::DeleteFile(log_file.path, false); + base::DeleteFile(log_file.path); expected_files_.pop_front(); } @@ -1345,9 +1339,8 @@ class FileListExpectingWebRtcEventLogUploader : public WebRtcEventLogUploader { return log_file_; } - bool Cancel() override { + void Cancel() override { NOTREACHED() << "Incompatible with this kind of test."; - return true; } private: @@ -1408,19 +1401,19 @@ TEST_F(WebRtcEventLogManagerTest, } TEST_F(WebRtcEventLogManagerTest, - PeerConnectionSessionIdSetReturnsFalseIfAlreadyCalledSameId) { + PeerConnectionSessionIdSetReturnsFalseIfPeerConnectionAlreadyRemoved) { const auto key = GetPeerConnectionKey(rph_.get(), kLid); ASSERT_TRUE(PeerConnectionAdded(key)); - ASSERT_TRUE(PeerConnectionSessionIdSet(key, kSessionId)); + ASSERT_TRUE(PeerConnectionRemoved(key)); EXPECT_FALSE(PeerConnectionSessionIdSet(key, kSessionId)); } TEST_F(WebRtcEventLogManagerTest, - PeerConnectionSessionIdSetReturnsFalseIfPeerConnectionAlreadyRemoved) { + PeerConnectionSessionIdSetReturnsTrueIfAlreadyCalledSameId) { const auto key = GetPeerConnectionKey(rph_.get(), kLid); ASSERT_TRUE(PeerConnectionAdded(key)); - ASSERT_TRUE(PeerConnectionRemoved(key)); - EXPECT_FALSE(PeerConnectionSessionIdSet(key, kSessionId)); + ASSERT_TRUE(PeerConnectionSessionIdSet(key, kSessionId)); + EXPECT_TRUE(PeerConnectionSessionIdSet(key, kSessionId)); } TEST_F(WebRtcEventLogManagerTest, @@ -4357,7 +4350,7 @@ TEST_F(WebRtcEventLogManagerTestPolicy, ASSERT_TRUE(base::PathExists(*log_file)); // Test sanity; exists before. - allow_remote_logging = !allow_remote_logging; + allow_remote_logging = false; profile->GetPrefs()->SetBoolean(prefs::kWebRtcEventLogCollectionAllowed, allow_remote_logging); diff --git a/chromium/chrome/browser/media/webrtc/webrtc_event_log_uploader.cc b/chromium/chrome/browser/media/webrtc/webrtc_event_log_uploader.cc index b095a44da6c..9be22d20e36 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_event_log_uploader.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_event_log_uploader.cc @@ -8,8 +8,6 @@ #include "base/files/file_util.h" #include "base/logging.h" #include "base/strings/stringprintf.h" -#include "base/task/post_task.h" -#include "base/threading/sequenced_task_runner_handle.h" #include "build/build_config.h" #include "chrome/browser/browser_process.h" #include "components/version_info/version_info.h" @@ -125,11 +123,17 @@ void OnURLLoadUploadProgress(uint64_t current, uint64_t total) { const char WebRtcEventLogUploaderImpl::kUploadURL[] = "https://clients2.google.com/cr/report"; +WebRtcEventLogUploaderImpl::Factory::Factory( + scoped_refptr<base::SequencedTaskRunner> task_runner) + : task_runner_(std::move(task_runner)) {} + +WebRtcEventLogUploaderImpl::Factory::~Factory() = default; + std::unique_ptr<WebRtcEventLogUploader> WebRtcEventLogUploaderImpl::Factory::Create(const WebRtcLogFileInfo& log_file, UploadResultCallback callback) { return std::make_unique<WebRtcEventLogUploaderImpl>( - log_file, std::move(callback), kMaxRemoteLogFileSizeBytes); + task_runner_, log_file, std::move(callback), kMaxRemoteLogFileSizeBytes); } std::unique_ptr<WebRtcEventLogUploader> @@ -138,17 +142,18 @@ WebRtcEventLogUploaderImpl::Factory::CreateWithCustomMaxSizeForTesting( UploadResultCallback callback, size_t max_log_file_size_bytes) { return std::make_unique<WebRtcEventLogUploaderImpl>( - log_file, std::move(callback), max_log_file_size_bytes); + task_runner_, log_file, std::move(callback), max_log_file_size_bytes); } WebRtcEventLogUploaderImpl::WebRtcEventLogUploaderImpl( + scoped_refptr<base::SequencedTaskRunner> task_runner, const WebRtcLogFileInfo& log_file, UploadResultCallback callback, size_t max_log_file_size_bytes) - : log_file_(log_file), + : task_runner_(std::move(task_runner)), + log_file_(log_file), callback_(std::move(callback)), - max_log_file_size_bytes_(max_log_file_size_bytes), - io_task_runner_(base::SequencedTaskRunnerHandle::Get()) { + max_log_file_size_bytes_(max_log_file_size_bytes) { history_file_writer_ = WebRtcEventLogHistoryFileWriter::Create( GetWebRtcEventLogHistoryFilePath(log_file_.path)); if (!history_file_writer_) { @@ -190,12 +195,12 @@ WebRtcEventLogUploaderImpl::~WebRtcEventLogUploaderImpl() { // 3. The upload was never started, due to an early failure (e.g. file not // found). In that case, |url_loader_| will not have been set. // 4. Chrome shutdown. - if (io_task_runner_->RunsTasksInCurrentSequence()) { // Scenarios 1-3. + if (task_runner_->RunsTasksInCurrentSequence()) { // Scenarios 1-3. DCHECK(!url_loader_); } else { // # Scenario #4 - Chrome shutdown. DCHECK_CURRENTLY_ON(content::BrowserThread::UI); bool will_delete = - io_task_runner_->DeleteSoon(FROM_HERE, url_loader_.release()); + task_runner_->DeleteSoon(FROM_HERE, url_loader_.release()); DCHECK(!will_delete) << "Task runners must have been stopped by this stage of shutdown."; } @@ -203,35 +208,33 @@ WebRtcEventLogUploaderImpl::~WebRtcEventLogUploaderImpl() { const WebRtcLogFileInfo& WebRtcEventLogUploaderImpl::GetWebRtcLogFileInfo() const { - DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); + DCHECK(task_runner_->RunsTasksInCurrentSequence()); return log_file_; } -bool WebRtcEventLogUploaderImpl::Cancel() { - DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); +void WebRtcEventLogUploaderImpl::Cancel() { + DCHECK(task_runner_->RunsTasksInCurrentSequence()); - // The upload could already have been completed, or maybe was never properly - // started (due to a file read failure, etc.). - const bool upload_was_active = (url_loader_.get() != nullptr); + if (url_loader_.get() == nullptr) { + // Either the upload has already completed, or it never properly started. + return; + } - // Note that in this case, it might still be that the last bytes hit the - // wire right as we attempt to cancel the upload. OnURLFetchComplete, however, - // will not be called. + // Stop the upload. url_loader_.reset(); - DeleteLogFile(); - DeleteHistoryFile(); - - if (upload_was_active) { - UmaRecordWebRtcEventLoggingUpload( - WebRtcEventLoggingUploadUma::kUploadCancelled); - } - - return upload_was_active; + // Note edge case - the upload might on very rare occasions already have + // finished, with the on-complete callback pending on the queue. + // In those very rare occasions, we will record the UMA for cancellation + // as well as the success/failure of the upload. Also, we'll post to replies + // back to the owner of this WebRtcEventLogUploaderImpl object. + UmaRecordWebRtcEventLoggingUpload( + WebRtcEventLoggingUploadUma::kUploadCancelled); + ReportResult(/*upload_successful=*/false, /*delete_history_file=*/true); } bool WebRtcEventLogUploaderImpl::PrepareUploadData(std::string* upload_data) { - DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); + DCHECK(task_runner_->RunsTasksInCurrentSequence()); std::string log_file_contents; if (!base::ReadFileToStringWithMaxSize(log_file_.path, &log_file_contents, @@ -271,7 +274,7 @@ bool WebRtcEventLogUploaderImpl::PrepareUploadData(std::string* upload_data) { } void WebRtcEventLogUploaderImpl::StartUpload(const std::string& upload_data) { - DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); + DCHECK(task_runner_->RunsTasksInCurrentSequence()); auto resource_request = std::make_unique<network::ResourceRequest>(); resource_request->url = GURL(kUploadURL); @@ -282,8 +285,8 @@ void WebRtcEventLogUploaderImpl::StartUpload(const std::string& upload_data) { // immediately, even though it needs to finish initialization on the UI // thread. mojo::Remote<network::mojom::URLLoaderFactory> url_loader_factory_remote; - base::PostTask( - FROM_HERE, {content::BrowserThread::UI}, + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(BindURLLoaderFactoryReceiver, url_loader_factory_remote.BindNewPipeAndPassReceiver())); @@ -293,18 +296,16 @@ void WebRtcEventLogUploaderImpl::StartUpload(const std::string& upload_data) { url_loader_->SetOnUploadProgressCallback( base::BindRepeating(OnURLLoadUploadProgress)); - // See comment in destructor for an explanation about why using - // base::Unretained(this) is safe here. url_loader_->DownloadToString( url_loader_factory_remote.get(), base::BindOnce(&WebRtcEventLogUploaderImpl::OnURLLoadComplete, - base::Unretained(this)), + weak_ptr_factory_.GetWeakPtr()), kWebRtcEventLogMaxUploadIdBytes); } void WebRtcEventLogUploaderImpl::OnURLLoadComplete( std::unique_ptr<std::string> response_body) { - DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); + DCHECK(task_runner_->RunsTasksInCurrentSequence()); DCHECK(url_loader_); if (response_body.get() != nullptr && response_body->empty()) { @@ -341,8 +342,9 @@ void WebRtcEventLogUploaderImpl::OnURLLoadComplete( ReportResult(upload_successful); } -void WebRtcEventLogUploaderImpl::ReportResult(bool result) { - DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); +void WebRtcEventLogUploaderImpl::ReportResult(bool upload_successful, + bool delete_history_file) { + DCHECK(task_runner_->RunsTasksInCurrentSequence()); // * If the upload was successful, the file is no longer needed. // * If the upload failed, we don't want to retry, because we run the risk of @@ -353,17 +355,21 @@ void WebRtcEventLogUploaderImpl::ReportResult(bool result) { // TODO(crbug.com/775415): Provide refined retrial behavior. DeleteLogFile(); + if (delete_history_file) { + DeleteHistoryFile(); + } + // Release hold of history file, allowing it to be read, moved or deleted. history_file_writer_.reset(); - io_task_runner_->PostTask( - FROM_HERE, base::BindOnce(std::move(callback_), log_file_.path, result)); + task_runner_->PostTask( + FROM_HERE, + base::BindOnce(std::move(callback_), log_file_.path, upload_successful)); } void WebRtcEventLogUploaderImpl::DeleteLogFile() { - DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); - const bool deletion_successful = - base::DeleteFile(log_file_.path, /*recursive=*/false); + DCHECK(task_runner_->RunsTasksInCurrentSequence()); + const bool deletion_successful = base::DeleteFile(log_file_.path); if (!deletion_successful) { // This is a somewhat serious (though unlikely) error, because now we'll // try to upload this file again next time Chrome launches. @@ -372,7 +378,7 @@ void WebRtcEventLogUploaderImpl::DeleteLogFile() { } void WebRtcEventLogUploaderImpl::DeleteHistoryFile() { - DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); + DCHECK(task_runner_->RunsTasksInCurrentSequence()); if (!history_file_writer_) { LOG(ERROR) << "Deletion of history file attempted after uploader " << "has relinquished ownership of it."; diff --git a/chromium/chrome/browser/media/webrtc/webrtc_event_log_uploader.h b/chromium/chrome/browser/media/webrtc/webrtc_event_log_uploader.h index f0699503180..d6d4f28abac 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_event_log_uploader.h +++ b/chromium/chrome/browser/media/webrtc/webrtc_event_log_uploader.h @@ -37,12 +37,12 @@ class WebRtcEventLogUploader { public: virtual ~Factory() = default; - // Creates uploaders. The observer is passed to each call of Create, - // rather than be memorized by the factory's constructor, because factories - // created by unit tests have no visibility into the real implementation's - // observer (WebRtcRemoteEventLogManager). + // Creates uploaders. // This takes ownership of the file. The caller must not attempt to access - // the file after invoking Create(). + // the file after invoking Create(). Even deleting the file due to + // the user clearing cache, is to be done through the uploader's Cancel, + // and not through direct access of the caller to the file. The file may + // be touched again only after |this| is destroyed. virtual std::unique_ptr<WebRtcEventLogUploader> Create( const WebRtcLogFileInfo& log_file, UploadResultCallback callback) = 0; @@ -55,10 +55,9 @@ class WebRtcEventLogUploader { virtual const WebRtcLogFileInfo& GetWebRtcLogFileInfo() const = 0; // Cancels the upload, then deletes the log file and its history file. - // Returns true if the upload was cancelled due to this call, and false if - // the upload was already completed or aborted before this call. - // (Aborted uploads are ones where the file could not be read, etc.) - virtual bool Cancel() = 0; + // (These files are deleted even if the upload has been completed by the time + // Cancel is called.) + virtual void Cancel() = 0; }; // Primary implementation of WebRtcEventLogUploader. Uploads log files to crash. @@ -67,7 +66,9 @@ class WebRtcEventLogUploaderImpl : public WebRtcEventLogUploader { public: class Factory : public WebRtcEventLogUploader::Factory { public: - ~Factory() override = default; + explicit Factory(scoped_refptr<base::SequencedTaskRunner> task_runner); + + ~Factory() override; std::unique_ptr<WebRtcEventLogUploader> Create( const WebRtcLogFileInfo& log_file, @@ -80,9 +81,13 @@ class WebRtcEventLogUploaderImpl : public WebRtcEventLogUploader { const WebRtcLogFileInfo& log_file, UploadResultCallback callback, size_t max_remote_log_file_size_bytes); + + private: + scoped_refptr<base::SequencedTaskRunner> task_runner_; }; WebRtcEventLogUploaderImpl( + scoped_refptr<base::SequencedTaskRunner> task_runner, const WebRtcLogFileInfo& log_file, UploadResultCallback callback, size_t max_remote_log_file_size_bytes); @@ -90,7 +95,7 @@ class WebRtcEventLogUploaderImpl : public WebRtcEventLogUploader { const WebRtcLogFileInfo& GetWebRtcLogFileInfo() const override; - bool Cancel() override; + void Cancel() override; private: friend class WebRtcEventLogUploaderImplTest; @@ -110,7 +115,7 @@ class WebRtcEventLogUploaderImpl : public WebRtcEventLogUploader { void OnURLLoadComplete(std::unique_ptr<std::string> response_body); // Cleanup and posting of the result callback. - void ReportResult(bool result); + void ReportResult(bool upload_successful, bool delete_history_file = false); // Remove the log file which is owned by |this|. void DeleteLogFile(); @@ -121,6 +126,9 @@ class WebRtcEventLogUploaderImpl : public WebRtcEventLogUploader { // The URL used for uploading the logs. static const char kUploadURL[]; + // The object lives on this IO-capable task runner. + scoped_refptr<base::SequencedTaskRunner> task_runner_; + // Housekeeping information about the uploaded file (path, time of last // modification, associated BrowserContext). const WebRtcLogFileInfo log_file_; @@ -139,8 +147,8 @@ class WebRtcEventLogUploaderImpl : public WebRtcEventLogUploader { // This object is in charge of the actual upload. std::unique_ptr<network::SimpleURLLoader> url_loader_; - // The object lives on this IO-capable task runner. - scoped_refptr<base::SequencedTaskRunner> io_task_runner_; + // Allows releasing |this| while a task from |url_loader_| is still pending. + base::WeakPtrFactory<WebRtcEventLogUploaderImpl> weak_ptr_factory_{this}; }; } // namespace webrtc_event_logging diff --git a/chromium/chrome/browser/media/webrtc/webrtc_event_log_uploader_impl_unittest.cc b/chromium/chrome/browser/media/webrtc/webrtc_event_log_uploader_impl_unittest.cc index 853cc085998..5d341ccdebc 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_event_log_uploader_impl_unittest.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_event_log_uploader_impl_unittest.cc @@ -28,6 +28,7 @@ namespace webrtc_event_logging { +using ::testing::_; using ::testing::StrictMock; using BrowserContextId = WebRtcEventLogPeerConnectionKey::BrowserContextId; @@ -81,7 +82,8 @@ class WebRtcEventLogUploaderImplTest : public ::testing::Test { EXPECT_TRUE(base::Time::FromString("30 Dec 1983", &kReasonableTime)); - uploader_factory_ = std::make_unique<WebRtcEventLogUploaderImpl::Factory>(); + uploader_factory_ = std::make_unique<WebRtcEventLogUploaderImpl::Factory>( + base::SequencedTaskRunnerHandle::Get()); } ~WebRtcEventLogUploaderImplTest() override { @@ -276,31 +278,34 @@ TEST_F(WebRtcEventLogUploaderImplTest, ExcessivelyLargeFilesNotUploaded) { } TEST_F(WebRtcEventLogUploaderImplTest, - CancelBeforeUploadCompletionReturnsTrue) { + CancelBeforeUploadCompletionCallsCallbackWithFalse) { const base::Time last_modified = base::Time::Now(); StartUploadThatWillNotTerminate(browser_context_id_, last_modified); - - EXPECT_TRUE(uploader_->Cancel()); + EXPECT_CALL(observer_, CompletionCallback(log_file_, false)).Times(1); + uploader_->Cancel(); } -TEST_F(WebRtcEventLogUploaderImplTest, CancelOnCancelledUploadReturnsFalse) { +TEST_F(WebRtcEventLogUploaderImplTest, SecondCallToCancelHasNoEffect) { const base::Time last_modified = base::Time::Now(); StartUploadThatWillNotTerminate(browser_context_id_, last_modified); - ASSERT_TRUE(uploader_->Cancel()); - EXPECT_FALSE(uploader_->Cancel()); + EXPECT_CALL(observer_, CompletionCallback(log_file_, _)).Times(1); + + uploader_->Cancel(); + uploader_->Cancel(); } TEST_F(WebRtcEventLogUploaderImplTest, - CancelAfterUploadCompletionReturnsFalse) { + CancelAfterUploadCompletionCallbackWasCalledHasNoEffect) { SetURLLoaderResponse(net::HTTP_OK, net::OK); EXPECT_CALL(observer_, CompletionCallback(log_file_, true)).Times(1); StartAndWaitForUpload(); - EXPECT_FALSE(uploader_->Cancel()); + EXPECT_CALL(observer_, CompletionCallback(_, _)).Times(0); + uploader_->Cancel(); } -TEST_F(WebRtcEventLogUploaderImplTest, CancelOnAbortedUploadReturnsFalse) { +TEST_F(WebRtcEventLogUploaderImplTest, CancelOnAbortedUploadHasNoEffect) { // Show the failure was independent of the URLLoaderFactory's primed return // value. SetURLLoaderResponse(net::HTTP_OK, net::OK); @@ -309,13 +314,17 @@ TEST_F(WebRtcEventLogUploaderImplTest, CancelOnAbortedUploadReturnsFalse) { EXPECT_CALL(observer_, CompletionCallback(log_file_, false)).Times(1); StartAndWaitForUpload(); - EXPECT_FALSE(uploader_->Cancel()); + EXPECT_CALL(observer_, CompletionCallback(_, _)).Times(0); + uploader_->Cancel(); } TEST_F(WebRtcEventLogUploaderImplTest, CancelOnOngoingUploadDeletesFile) { const base::Time last_modified = base::Time::Now(); StartUploadThatWillNotTerminate(browser_context_id_, last_modified); - ASSERT_TRUE(uploader_->Cancel()); + + EXPECT_CALL(observer_, CompletionCallback(log_file_, false)).Times(1); + uploader_->Cancel(); + observer_run_loop_.Run(); EXPECT_FALSE(base::PathExists(log_file_)); } @@ -331,7 +340,8 @@ TEST_F(WebRtcEventLogUploaderImplTest, EXPECT_EQ(info.last_modified, last_modified); // Test tear-down. - ASSERT_TRUE(uploader_->Cancel()); + EXPECT_CALL(observer_, CompletionCallback(log_file_, false)).Times(1); + uploader_->Cancel(); } TEST_F(WebRtcEventLogUploaderImplTest, @@ -352,7 +362,8 @@ TEST_F(WebRtcEventLogUploaderImplTest, GetWebRtcLogFileInfoReturnsCorrectInfoWhenCalledOnCancelledUpload) { const base::Time last_modified = base::Time::Now(); StartUploadThatWillNotTerminate(browser_context_id_, last_modified); - ASSERT_TRUE(uploader_->Cancel()); + EXPECT_CALL(observer_, CompletionCallback(log_file_, false)).Times(1); + uploader_->Cancel(); const WebRtcLogFileInfo info = uploader_->GetWebRtcLogFileInfo(); EXPECT_EQ(info.browser_context_id, browser_context_id_); diff --git a/chromium/chrome/browser/media/webrtc/webrtc_log_uploader_unittest.cc b/chromium/chrome/browser/media/webrtc/webrtc_log_uploader_unittest.cc index 6aa2a76c11f..f7d57c8beb4 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_log_uploader_unittest.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_log_uploader_unittest.cc @@ -203,7 +203,7 @@ TEST_F(WebRtcLogUploaderTest, AddLocallyStoredLogInfoToUploadListFile) { // Get a temporary filename. We don't want the file to exist to begin with // since that's the normal use case, hence the delete. ASSERT_TRUE(base::CreateTemporaryFile(&test_list_path_)); - EXPECT_TRUE(base::DeleteFile(test_list_path_, false)); + EXPECT_TRUE(base::DeleteFile(test_list_path_)); std::unique_ptr<WebRtcLogUploader> webrtc_log_uploader( new WebRtcLogUploader()); @@ -241,7 +241,7 @@ TEST_F(WebRtcLogUploaderTest, AddUploadedLogInfoToUploadListFile) { // Get a temporary filename. We don't want the file to exist to begin with // since that's the normal use case, hence the delete. ASSERT_TRUE(base::CreateTemporaryFile(&test_list_path_)); - EXPECT_TRUE(base::DeleteFile(test_list_path_, false)); + EXPECT_TRUE(base::DeleteFile(test_list_path_)); std::unique_ptr<WebRtcLogUploader> webrtc_log_uploader( new WebRtcLogUploader()); diff --git a/chromium/chrome/browser/media/webrtc/webrtc_logging_controller.cc b/chromium/chrome/browser/media/webrtc/webrtc_logging_controller.cc index 8caf1d7fe01..a2dc872f349 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_logging_controller.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_logging_controller.cc @@ -85,7 +85,10 @@ void WebRtcLoggingController::StartLogging( content::RenderProcessHost* host = content::RenderProcessHost::FromID(render_process_id_); - // OK for this to replace an existing logging_agent_ connection. + // OK to rebind existing |logging_agent_| and |receiver_| connections. + logging_agent_.reset(); + receiver_.reset(); + host->BindReceiver(logging_agent_.BindNewPipeAndPassReceiver()); logging_agent_.set_disconnect_handler( base::BindOnce(&WebRtcLoggingController::OnAgentDisconnected, this)); diff --git a/chromium/chrome/browser/media/webrtc/webrtc_pan_tilt_zoom_browsertest.cc b/chromium/chrome/browser/media/webrtc/webrtc_pan_tilt_zoom_browsertest.cc index b8a3e81f925..71a1920071d 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_pan_tilt_zoom_browsertest.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_pan_tilt_zoom_browsertest.cc @@ -5,6 +5,7 @@ #include <string> #include "base/strings/stringprintf.h" +#include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h" #include "chrome/browser/media/webrtc/webrtc_browsertest_base.h" #include "content/public/browser/web_contents.h" #include "content/public/common/content_switches.h" @@ -70,34 +71,262 @@ INSTANTIATE_TEST_SUITE_P( RequestPanTiltZoomPermission, WebRtcPanTiltZoomBrowserTest, testing::Values( + // no pan, tilt, zoom in audio and video constraints TestConfig{"{ video: true }", "prompt", "granted", "prompt"}, TestConfig{"{ audio: true }", "granted", "prompt", "prompt"}, TestConfig{"{ audio: true, video: true }", "granted", "granted", "prompt"}, - TestConfig{"{ video: { pan : {} } }", "prompt", "granted", "prompt"}, - TestConfig{"{ video: { tilt : {} } }", "prompt", "granted", "prompt"}, - TestConfig{"{ video: { zoom : {} } }", "prompt", "granted", "prompt"}, - TestConfig{"{ video: { advanced: [{ pan : {} }] } }", "prompt", - "granted", "prompt"}, - TestConfig{"{ video: { advanced: [{ tilt : {} }] } }", "prompt", - "granted", "prompt"}, - TestConfig{"{ video: { advanced: [{ zoom : {} }] } }", "prompt", - "granted", "prompt"}, + // pan, tilt, zoom in audio constraints + TestConfig{"{ audio: { pan : false } }", "granted", "prompt", "prompt"}, + TestConfig{"{ audio: { tilt : false } }", "granted", "prompt", + "prompt"}, + TestConfig{"{ audio: { zoom : false } }", "granted", "prompt", + "prompt"}, TestConfig{"{ audio: { pan : {} } }", "granted", "prompt", "prompt"}, TestConfig{"{ audio: { tilt : {} } }", "granted", "prompt", "prompt"}, TestConfig{"{ audio: { zoom : {} } }", "granted", "prompt", "prompt"}, + TestConfig{"{ audio: { pan : 1 } }", "granted", "prompt", "prompt"}, + TestConfig{"{ audio: { tilt : 1 } }", "granted", "prompt", "prompt"}, + TestConfig{"{ audio: { zoom : 1 } }", "granted", "prompt", "prompt"}, + TestConfig{"{ audio: { pan : true } }", "granted", "prompt", "prompt"}, + TestConfig{"{ audio: { tilt : true } }", "granted", "prompt", "prompt"}, + TestConfig{"{ audio: { zoom : true } }", "granted", "prompt", "prompt"}, + // pan, tilt, zoom in basic video constraints if no audio + TestConfig{"{ video: { pan : false } }", "prompt", "granted", "prompt"}, + TestConfig{"{ video: { tilt : false } }", "prompt", "granted", + "prompt"}, + TestConfig{"{ video: { zoom : false } }", "prompt", "granted", + "prompt"}, + TestConfig{"{ video: { pan : {} } }", "prompt", "granted", "granted"}, + TestConfig{"{ video: { tilt : {} } }", "prompt", "granted", "granted"}, + TestConfig{"{ video: { zoom : {} } }", "prompt", "granted", "granted"}, TestConfig{"{ video: { pan : 1 } }", "prompt", "granted", "granted"}, TestConfig{"{ video: { tilt : 1 } }", "prompt", "granted", "granted"}, TestConfig{"{ video: { zoom : 1 } }", "prompt", "granted", "granted"}, + TestConfig{"{ video: { pan : true } }", "prompt", "granted", "granted"}, + TestConfig{"{ video: { tilt : true } }", "prompt", "granted", + "granted"}, + TestConfig{"{ video: { zoom : true } }", "prompt", "granted", + "granted"}, + // pan, tilt, zoom in advanced video constraints if no audio + TestConfig{"{ video: { advanced: [{ pan : false }] } }", "prompt", + "granted", "prompt"}, + TestConfig{"{ video: { advanced: [{ tilt : false }] } }", "prompt", + "granted", "prompt"}, + TestConfig{"{ video: { advanced: [{ zoom : false }] } }", "prompt", + "granted", "prompt"}, + TestConfig{"{ video: { advanced: [{ pan : {} }] } }", "prompt", + "granted", "granted"}, + TestConfig{"{ video: { advanced: [{ tilt : {} }] } }", "prompt", + "granted", "granted"}, + TestConfig{"{ video: { advanced: [{ zoom : {} }] } }", "prompt", + "granted", "granted"}, TestConfig{"{ video: { advanced: [{ pan : 1 }] } }", "prompt", "granted", "granted"}, TestConfig{"{ video: { advanced: [{ tilt : 1 }] } }", "prompt", "granted", "granted"}, TestConfig{"{ video: { advanced: [{ zoom : 1 }] } }", "prompt", "granted", "granted"}, + TestConfig{"{ video: { advanced: [{ pan : true }] } }", "prompt", + "granted", "granted"}, + TestConfig{"{ video: { advanced: [{ tilt : true }] } }", "prompt", + "granted", "granted"}, + TestConfig{"{ video: { advanced: [{ zoom : true }] } }", "prompt", + "granted", "granted"}, + // pan, tilt, zoom in basic video constraints if audio + TestConfig{"{ audio: true, video: { pan : false } }", "granted", + "granted", "prompt"}, + TestConfig{"{ audio: true, video: { tilt : false } }", "granted", + "granted", "prompt"}, + TestConfig{"{ audio: true, video: { zoom : false } }", "granted", + "granted", "prompt"}, + TestConfig{"{ audio: true, video: { pan : {} } }", "granted", "granted", + "granted"}, + TestConfig{"{ audio: true, video: { tilt : {} } }", "granted", + "granted", "granted"}, + TestConfig{"{ audio: true, video: { zoom : {} } }", "granted", + "granted", "granted"}, TestConfig{"{ audio: true, video: { pan : 1 } }", "granted", "granted", "granted"}, TestConfig{"{ audio: true, video: { tilt : 1 } }", "granted", "granted", "granted"}, TestConfig{"{ audio: true, video: { zoom : 1 } }", "granted", "granted", - "granted"})); + "granted"}, + TestConfig{"{ audio: true, video: { pan : true } }", "granted", + "granted", "granted"}, + TestConfig{"{ audio: true, video: { tilt : true } }", "granted", + "granted", "granted"}, + TestConfig{"{ audio: true, video: { zoom : true } }", "granted", + "granted", "granted"}, + // pan, tilt, zoom in advanced video constraints if audio + TestConfig{"{ audio: true, video: { advanced: [{ pan : false }] } }", + "granted", "granted", "prompt"}, + TestConfig{"{ audio: true, video: { advanced: [{ tilt : false }] } }", + "granted", "granted", "prompt"}, + TestConfig{"{ audio: true, video: { advanced: [{ zoom : false }] } }", + "granted", "granted", "prompt"}, + TestConfig{"{ audio: true, video: { advanced: [{ pan : {} }] } }", + "granted", "granted", "granted"}, + TestConfig{"{ audio: true, video: { advanced: [{ tilt : {} }] } }", + "granted", "granted", "granted"}, + TestConfig{"{ audio: true, video: { advanced: [{ zoom : {} }] } }", + "granted", "granted", "granted"}, + TestConfig{"{ audio: true, video: { advanced: [{ pan : 1 }] } }", + "granted", "granted", "granted"}, + TestConfig{"{ audio: true, video: { advanced: [{ tilt : 1 }] } }", + "granted", "granted", "granted"}, + TestConfig{"{ audio: true, video: { advanced: [{ zoom : 1 }] } }", + "granted", "granted", "granted"}, + TestConfig{"{ audio: true, video: { advanced: [{ pan : true }] } }", + "granted", "granted", "granted"}, + TestConfig{"{ audio: true, video: { advanced: [{ tilt : true }] } }", + "granted", "granted", "granted"}, + TestConfig{"{ audio: true, video: { advanced: [{ zoom : true }] } }", + "granted", "granted", "granted"})); + +class WebRtcPanTiltZoomPermissionRequestBrowserTest + : public WebRtcTestBase, + public ::testing::WithParamInterface< + bool /* IsPanTiltZoomSupported() */> { + public: + void SetUpCommandLine(base::CommandLine* command_line) override { + command_line->AppendSwitchASCII( + switches::kEnableBlinkFeatures, + "MediaCapturePanTilt,PermissionsRequestRevoke"); + } + + bool IsPanTiltZoomSupported() const { return GetParam(); } + + void SetUpOnMainThread() override { + WebRtcTestBase::SetUpOnMainThread(); + + blink::MediaStreamDevices video_devices; + blink::MediaStreamDevice fake_video_device( + blink::mojom::MediaStreamType::DEVICE_VIDEO_CAPTURE, "fake_video_dev", + "Fake Video Device", media::MEDIA_VIDEO_FACING_NONE, base::nullopt, + IsPanTiltZoomSupported()); + video_devices.push_back(fake_video_device); + MediaCaptureDevicesDispatcher::GetInstance()->SetTestVideoCaptureDevices( + video_devices); + } + + void SetUpInProcessBrowserTestFixture() override { + DetectErrorsInJavaScript(); + } +}; + +IN_PROC_BROWSER_TEST_P(WebRtcPanTiltZoomPermissionRequestBrowserTest, + TestRequestPanTiltZoomPermission) { + ASSERT_TRUE(embedded_test_server()->Start()); + content::WebContents* tab = OpenTestPageInNewTab(kMainHtmlPage); + + std::string result; + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + tab->GetMainFrame(), "runRequestPanTiltZoom();", &result)); + EXPECT_EQ(result, "runRequestPanTiltZoom-success"); + + std::string camera; + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + tab->GetMainFrame(), "getCameraPermission();", &camera)); + EXPECT_EQ(camera, "granted"); + + std::string pan_tilt_zoom; + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + tab->GetMainFrame(), "getPanTiltZoomPermission();", &pan_tilt_zoom)); + EXPECT_EQ(pan_tilt_zoom, IsPanTiltZoomSupported() ? "granted" : "prompt"); +} + +INSTANTIATE_TEST_SUITE_P(RequestPanTiltZoomPermission, + WebRtcPanTiltZoomPermissionRequestBrowserTest, + ::testing::Bool() /* IsPanTiltZoomSupported() */); + +class WebRtcPanTiltZoomCameraDevicesBrowserTest : public WebRtcTestBase { + public: + void SetUpCommandLine(base::CommandLine* command_line) override { + command_line->AppendSwitchASCII( + switches::kEnableBlinkFeatures, + "MediaCapturePanTilt,PermissionsRequestRevoke"); + } + + void SetVideoCaptureDevice(bool pan_tilt_zoom_supported) { + blink::MediaStreamDevices video_devices; + blink::MediaStreamDevice fake_video_device( + blink::mojom::MediaStreamType::DEVICE_VIDEO_CAPTURE, "fake_video_dev", + "Fake Video Device", media::MEDIA_VIDEO_FACING_NONE, base::nullopt, + pan_tilt_zoom_supported); + video_devices.push_back(fake_video_device); + MediaCaptureDevicesDispatcher::GetInstance()->SetTestVideoCaptureDevices( + video_devices); + } + + void SetUpInProcessBrowserTestFixture() override { + DetectErrorsInJavaScript(); + } +}; + +IN_PROC_BROWSER_TEST_F(WebRtcPanTiltZoomCameraDevicesBrowserTest, + TestCameraPanTiltZoomPermissionIsNotGrantedAfterCamera) { + ASSERT_TRUE(embedded_test_server()->Start()); + content::WebContents* tab = OpenTestPageInNewTab(kMainHtmlPage); + + // Simulate camera device with no PTZ support and request PTZ camera + // permission. + SetVideoCaptureDevice(false /* pan_tilt_zoom_supported */); + std::string result; + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + tab->GetMainFrame(), "runRequestPanTiltZoom();", &result)); + EXPECT_EQ(result, "runRequestPanTiltZoom-success"); + + // Camera permission should be granted. + std::string camera; + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + tab->GetMainFrame(), "getCameraPermission();", &camera)); + EXPECT_EQ(camera, "granted"); + + // Camera PTZ permission should not be granted. + std::string pan_tilt_zoom; + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + tab->GetMainFrame(), "getPanTiltZoomPermission();", &pan_tilt_zoom)); + EXPECT_EQ(pan_tilt_zoom, "prompt"); + + // Simulate camera device with PTZ support. + SetVideoCaptureDevice(true /* pan_tilt_zoom_supported */); + + // Camera PTZ permission should still not be granted. + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + tab->GetMainFrame(), "getPanTiltZoomPermission();", &pan_tilt_zoom)); + EXPECT_EQ(pan_tilt_zoom, "prompt"); +} + +IN_PROC_BROWSER_TEST_F(WebRtcPanTiltZoomCameraDevicesBrowserTest, + TestCameraPanTiltZoomPermissionPersists) { + ASSERT_TRUE(embedded_test_server()->Start()); + content::WebContents* tab = OpenTestPageInNewTab(kMainHtmlPage); + + // Simulate camera device with PTZ support and request PTZ camera permission. + SetVideoCaptureDevice(true /* pan_tilt_zoom_supported */); + std::string result; + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + tab->GetMainFrame(), "runRequestPanTiltZoom();", &result)); + EXPECT_EQ(result, "runRequestPanTiltZoom-success"); + + // Camera permission should be granted. + std::string camera; + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + tab->GetMainFrame(), "getCameraPermission();", &camera)); + EXPECT_EQ(camera, "granted"); + + // Camera PTZ permission should be granted. + std::string pan_tilt_zoom; + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + tab->GetMainFrame(), "getPanTiltZoomPermission();", &pan_tilt_zoom)); + EXPECT_EQ(pan_tilt_zoom, "granted"); + + // Simulate camera device with no PTZ support. + SetVideoCaptureDevice(false /* pan_tilt_zoom_supported */); + + // Camera PTZ permission should still be granted. + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + tab->GetMainFrame(), "getPanTiltZoomPermission();", &pan_tilt_zoom)); + EXPECT_EQ(pan_tilt_zoom, "granted"); +} diff --git a/chromium/chrome/browser/media/webrtc/webrtc_rtp_dump_handler.cc b/chromium/chrome/browser/media/webrtc/webrtc_rtp_dump_handler.cc index 1bc27e2a6d9..1216bac21f6 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_rtp_dump_handler.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_rtp_dump_handler.cc @@ -64,15 +64,13 @@ WebRtcRtpDumpHandler::~WebRtcRtpDumpHandler() { if (incoming_state_ != STATE_NONE && !incoming_dump_path_.empty()) { base::ThreadPool::PostTask( FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT}, - base::BindOnce(base::IgnoreResult(&base::DeleteFile), - incoming_dump_path_, false)); + base::BindOnce(base::GetDeleteFileCallback(), incoming_dump_path_)); } if (outgoing_state_ != STATE_NONE && !outgoing_dump_path_.empty()) { base::ThreadPool::PostTask( FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT}, - base::BindOnce(base::IgnoreResult(&base::DeleteFile), - outgoing_dump_path_, false)); + base::BindOnce(base::GetDeleteFileCallback(), outgoing_dump_path_)); } } @@ -295,8 +293,7 @@ void WebRtcRtpDumpHandler::OnDumpEnded(const base::Closure& callback, if (!incoming_success) { base::ThreadPool::PostTask( FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT}, - base::BindOnce(base::IgnoreResult(&base::DeleteFile), - incoming_dump_path_, false)); + base::BindOnce(base::GetDeleteFileCallback(), incoming_dump_path_)); DVLOG(2) << "Deleted invalid incoming dump " << incoming_dump_path_.value(); @@ -311,8 +308,7 @@ void WebRtcRtpDumpHandler::OnDumpEnded(const base::Closure& callback, if (!outgoing_success) { base::ThreadPool::PostTask( FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT}, - base::BindOnce(base::IgnoreResult(&base::DeleteFile), - outgoing_dump_path_, false)); + base::BindOnce(base::GetDeleteFileCallback(), outgoing_dump_path_)); DVLOG(2) << "Deleted invalid outgoing dump " << outgoing_dump_path_.value(); diff --git a/chromium/chrome/browser/media/webrtc/webrtc_rtp_dump_handler_unittest.cc b/chromium/chrome/browser/media/webrtc/webrtc_rtp_dump_handler_unittest.cc index 01c742b0968..0be8f897037 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_rtp_dump_handler_unittest.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_rtp_dump_handler_unittest.cc @@ -14,6 +14,7 @@ #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" #include "base/location.h" +#include "base/logging.h" #include "base/run_loop.h" #include "base/sequenced_task_runner.h" #include "base/single_thread_task_runner.h" diff --git a/chromium/chrome/browser/media/webrtc/webrtc_simulcast_browsertest.cc b/chromium/chrome/browser/media/webrtc/webrtc_simulcast_browsertest.cc deleted file mode 100644 index b98d937895c..00000000000 --- a/chromium/chrome/browser/media/webrtc/webrtc_simulcast_browsertest.cc +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright 2015 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 "base/command_line.h" -#include "base/files/file_path.h" -#include "base/path_service.h" -#include "build/build_config.h" -#include "chrome/browser/media/webrtc/webrtc_browsertest_base.h" -#include "chrome/browser/media/webrtc/webrtc_browsertest_common.h" -#include "chrome/browser/ui/browser.h" -#include "chrome/browser/ui/browser_tabstrip.h" -#include "chrome/browser/ui/tabs/tab_strip_model.h" -#include "chrome/common/chrome_switches.h" -#include "chrome/test/base/in_process_browser_test.h" -#include "chrome/test/base/ui_test_utils.h" -#include "content/public/browser/notification_service.h" -#include "content/public/browser/render_process_host.h" -#include "content/public/common/content_switches.h" -#include "content/public/test/browser_test.h" -#include "content/public/test/browser_test_utils.h" -#include "media/base/media_switches.h" -#include "net/test/embedded_test_server/embedded_test_server.h" -#include "testing/perf/perf_test.h" -#include "ui/gl/gl_switches.h" - -static const char kSimulcastTestPage[] = "/webrtc/webrtc-simulcast.html"; - -// Simulcast integration test. This test ensures 'a=x-google-flag:conference' -// is working and that Chrome is capable of sending simulcast streams. -class WebRtcSimulcastBrowserTest : public WebRtcTestBase { - public: - // TODO(phoglund): Make it possible to enable DetectErrorsInJavaScript() here. - - void SetUpCommandLine(base::CommandLine* command_line) override { - // Just answer 'allow' to GetUserMedia invocations. - command_line->AppendSwitch(switches::kUseFakeUIForMediaStream); - - // The video playback will not work without a GPU, so force its use here. - command_line->AppendSwitch(switches::kUseGpuInTests); - } -}; - -// Fails/times out on Windows and Chrome OS. Flaky on Mac and Linux. -// http://crbug.com/452623 -// http://crbug.com/1004546 -// MSan reports errors. http://crbug.com/452892 -#if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_LINUX) || \ - defined(MEMORY_SANITIZER) -#define MAYBE_TestVgaReturnsTwoSimulcastStreams DISABLED_TestVgaReturnsTwoSimulcastStreams -#else -#define MAYBE_TestVgaReturnsTwoSimulcastStreams TestVgaReturnsTwoSimulcastStreams -#endif -IN_PROC_BROWSER_TEST_F(WebRtcSimulcastBrowserTest, - MAYBE_TestVgaReturnsTwoSimulcastStreams) { - ASSERT_TRUE(embedded_test_server()->Start()); - - ui_test_utils::NavigateToURL( - browser(), embedded_test_server()->GetURL(kSimulcastTestPage)); - - content::WebContents* tab_contents = - browser()->tab_strip_model()->GetActiveWebContents(); - - ASSERT_EQ("OK", ExecuteJavascript("testVgaReturnsTwoSimulcastStreams()", - tab_contents)); -} diff --git a/chromium/chrome/browser/media/webrtc/webrtc_text_log_handler.cc b/chromium/chrome/browser/media/webrtc/webrtc_text_log_handler.cc index dd27ecc5f0f..0185766cf85 100644 --- a/chromium/chrome/browser/media/webrtc/webrtc_text_log_handler.cc +++ b/chromium/chrome/browser/media/webrtc/webrtc_text_log_handler.cc @@ -19,7 +19,6 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/stringprintf.h" #include "base/system/sys_info.h" -#include "base/task/post_task.h" #include "base/time/time.h" #include "chrome/common/channel_info.h" #include "chrome/common/media/webrtc_logging.mojom.h" @@ -249,8 +248,8 @@ bool WebRtcTextLogHandler::StopLogging(const GenericDoneCallback& callback) { stop_callback_ = callback; logging_state_ = STOPPING; - base::PostTask(FROM_HERE, {content::BrowserThread::IO}, - base::BindOnce(&content::WebRtcLog::ClearLogMessageCallback, + content::GetIOThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(&content::WebRtcLog::ClearLogMessageCallback, render_process_id_)); return true; } @@ -281,8 +280,8 @@ void WebRtcTextLogHandler::StopDone() { void WebRtcTextLogHandler::ChannelClosing() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); if (logging_state_ == STARTING || logging_state_ == STARTED) { - base::PostTask(FROM_HERE, {content::BrowserThread::IO}, - base::BindOnce(&content::WebRtcLog::ClearLogMessageCallback, + content::GetIOThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(&content::WebRtcLog::ClearLogMessageCallback, render_process_id_)); } channel_is_closing_ = true; @@ -528,8 +527,8 @@ void WebRtcTextLogHandler::OnGetNetworkInterfaceList( &ForwardMessageViaTaskRunner, base::SequencedTaskRunnerHandle::Get(), base::Bind(&WebRtcTextLogHandler::LogMessage, weak_factory_.GetWeakPtr())); - base::PostTask( - FROM_HERE, {content::BrowserThread::IO}, + content::GetIOThreadTaskRunner({})->PostTask( + FROM_HERE, base::BindOnce(&content::WebRtcLog::SetLogMessageCallback, render_process_id_, std::move(log_message_callback))); } diff --git a/chromium/chrome/browser/media/webrtc/window_icon_util_x11.cc b/chromium/chrome/browser/media/webrtc/window_icon_util_x11.cc index 272d0109da6..ecc942559cd 100644 --- a/chromium/chrome/browser/media/webrtc/window_icon_util_x11.cc +++ b/chromium/chrome/browser/media/webrtc/window_icon_util_x11.cc @@ -13,29 +13,9 @@ gfx::ImageSkia GetWindowIcon(content::DesktopMediaID id) { DCHECK(id.type == content::DesktopMediaID::TYPE_WINDOW); - Display* display = gfx::GetXDisplay(); - Atom property = gfx::GetAtom("_NET_WM_ICON"); - Atom actual_type; - int actual_format; - unsigned long bytes_after; // NOLINT: type required by XGetWindowProperty - unsigned long size; - long* data; - - // The |error_tracker| essentially provides an empty X error handler for - // the call of XGetWindowProperty. The motivation is to guard against crash - // for any reason that XGetWindowProperty fails. For example, at the time that - // XGetWindowProperty is called, the window handler (a.k.a |id.id|) may - // already be invalid due to the fact that the end user has closed the - // corresponding window, etc. - std::unique_ptr<gfx::X11ErrorTracker> error_tracker( - new gfx::X11ErrorTracker()); - int status = XGetWindowProperty(display, id.id, property, 0L, ~0L, x11::False, - AnyPropertyType, &actual_type, &actual_format, - &size, &bytes_after, - reinterpret_cast<unsigned char**>(&data)); - error_tracker.reset(); - - if (status != x11::Success) { + std::vector<uint32_t> data; + if (!ui::GetArrayProperty(static_cast<x11::Window>(id.id), + gfx::GetAtom("_NET_WM_ICON"), &data)) { return gfx::ImageSkia(); } @@ -45,10 +25,10 @@ gfx::ImageSkia GetWindowIcon(content::DesktopMediaID id) { int width = 0; int height = 0; int start = 0; - int i = 0; - while (i + 1 < static_cast<int>(size)) { + size_t i = 0; + while (i + 1 < data.size()) { if ((i == 0 || static_cast<int>(data[i] * data[i + 1]) > width * height) && - (i + 1 + data[i] * data[i + 1] < static_cast<int>(size))) { + (i + 1 + data[i] * data[i + 1] < data.size())) { width = static_cast<int>(data[i]); height = static_cast<int>(data[i + 1]); start = i + 2; @@ -69,6 +49,5 @@ gfx::ImageSkia GetWindowIcon(content::DesktopMediaID id) { } } - XFree(data); return gfx::ImageSkia::CreateFrom1xBitmap(result); } |