diff options
29 files changed, 358 insertions, 166 deletions
diff --git a/chrome/browser/media/android/router/media_router_android.cc b/chrome/browser/media/android/router/media_router_android.cc index bca53b1..6065528 100644 --- a/chrome/browser/media/android/router/media_router_android.cc +++ b/chrome/browser/media/android/router/media_router_android.cc @@ -4,7 +4,9 @@ #include "chrome/browser/media/android/router/media_router_android.h" +#include <string> #include <utility> +#include <vector> #include "base/android/context_utils.h" #include "base/android/jni_android.h" @@ -73,7 +75,8 @@ void MediaRouterAndroid::CreateRoute( const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) { + base::TimeDelta timeout, + bool off_the_record) { // TODO(avayvod): Implement timeouts (crbug.com/583036). if (!origin.is_valid()) { scoped_ptr<RouteRequestResult> result = RouteRequestResult::FromError( @@ -110,6 +113,8 @@ void MediaRouterAndroid::CreateRoute( ScopedJavaLocalRef<jstring> jorigin = base::android::ConvertUTF8ToJavaString(env, origin.spec()); + // TODO(avayvod): Pass the off_the_record flag to Android. + // https://bugs.chromium.org/p/chromium/issues/detail?id=588239 Java_ChromeMediaRouter_createRoute( env, java_media_router_.obj(), @@ -137,7 +142,8 @@ void MediaRouterAndroid::JoinRoute( const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) { + base::TimeDelta timeout, + bool off_the_record) { // TODO(avayvod): Implement timeouts (crbug.com/583036). if (!origin.is_valid()) { scoped_ptr<RouteRequestResult> result = RouteRequestResult::FromError( diff --git a/chrome/browser/media/android/router/media_router_android.h b/chrome/browser/media/android/router/media_router_android.h index bae085b..2258fa6 100644 --- a/chrome/browser/media/android/router/media_router_android.h +++ b/chrome/browser/media/android/router/media_router_android.h @@ -37,13 +37,15 @@ class MediaRouterAndroid : public MediaRouterBase { const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) override; + base::TimeDelta timeout, + bool off_the_record) override; void JoinRoute(const MediaSource::Id& source, const std::string& presentation_id, const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) override; + base::TimeDelta timeout, + bool off_the_record) override; void ConnectRouteByRouteId( const MediaSource::Id& source, const MediaRoute::Id& route_id, diff --git a/chrome/browser/media/android/router/media_router_dialog_controller_android.cc b/chrome/browser/media/android/router/media_router_dialog_controller_android.cc index 399c609..bacba80 100644 --- a/chrome/browser/media/android/router/media_router_dialog_controller_android.cc +++ b/chrome/browser/media/android/router/media_router_dialog_controller_android.cc @@ -12,6 +12,7 @@ #include "chrome/browser/media/router/media_router_factory.h" #include "chrome/browser/media/router/media_source.h" #include "chrome/browser/media/router/presentation_request.h" +#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_delegate.h" @@ -51,10 +52,12 @@ void MediaRouterDialogControllerAndroid::OnSinkSelected( base::Bind(&CreatePresentationConnectionRequest::HandleRouteResponse, base::Passed(&create_connection_request))); + content::BrowserContext* browser_context = initiator()->GetBrowserContext(); MediaRouter* router = MediaRouterFactory::GetApiForBrowserContext( - initiator()->GetBrowserContext()); + browser_context); router->CreateRoute(source_id, ConvertJavaStringToUTF8(env, jsink_id), origin, - initiator(), route_response_callbacks, base::TimeDelta()); + initiator(), route_response_callbacks, base::TimeDelta(), + browser_context->IsOffTheRecord()); } void MediaRouterDialogControllerAndroid::OnRouteClosed( diff --git a/chrome/browser/media/router/media_route.cc b/chrome/browser/media/router/media_route.cc index fb2d65e..c998d02 100644 --- a/chrome/browser/media/router/media_route.cc +++ b/chrome/browser/media/router/media_route.cc @@ -22,7 +22,8 @@ MediaRoute::MediaRoute(const MediaRoute::Id& media_route_id, description_(description), is_local_(is_local), custom_controller_path_(custom_controller_path), - for_display_(for_display) {} + for_display_(for_display), + off_the_record_(false) {} MediaRoute::~MediaRoute() { } diff --git a/chrome/browser/media/router/media_route.h b/chrome/browser/media/router/media_route.h index b368751..b9481ba 100644 --- a/chrome/browser/media/router/media_route.h +++ b/chrome/browser/media/router/media_route.h @@ -21,20 +21,22 @@ namespace media_router { // operation. The fields are immutable and reflect the route status // only at the time of object creation. Updated route statuses must // be retrieved as new MediaRoute objects from the Media Router. +// +// TODO(mfoltz): Convert to a simple struct and remove uncommon parameters from +// the ctor. class MediaRoute { public: using Id = std::string; - // |media_route_id|: ID of the route. New route IDs should be created - // by the RouteIdManager class. + // |media_route_id|: ID of the route. // |media_source|: Description of source of the route. // |media_sink|: The sink that is receiving the media. // |description|: Description of the route to be displayed. // |is_local|: true if the route was created from this browser. // |custom_controller_path|: custom controller path if it is given by route - // provider. empty otherwise. + // provider. empty otherwise. // |for_display|: Set to true if this route should be displayed for - // |media_sink_id| in UI. + // |media_sink_id| in UI. MediaRoute(const MediaRoute::Id& media_route_id, const MediaSource& media_source, const MediaSink::Id& media_sink_id, @@ -71,6 +73,14 @@ class MediaRoute { bool for_display() const { return for_display_; } + // Sets off_the_record. Set this to true when the route was created by an off + // the record (incognito) profile. + void set_off_the_record(bool off_the_record) { + off_the_record_ = off_the_record; + } + + bool off_the_record() const { return off_the_record_; } + bool Equals(const MediaRoute& other) const; private: @@ -81,6 +91,7 @@ class MediaRoute { bool is_local_; std::string custom_controller_path_; bool for_display_; + bool off_the_record_; }; } // namespace media_router diff --git a/chrome/browser/media/router/media_route_unittest.cc b/chrome/browser/media/router/media_route_unittest.cc index 8388a86..7f5ba83 100644 --- a/chrome/browser/media/router/media_route_unittest.cc +++ b/chrome/browser/media/router/media_route_unittest.cc @@ -40,6 +40,12 @@ TEST(MediaRouteTest, Equals) { MediaRoute route5(kRouteId2, MediaSourceForCastApp("DialApp"), "sinkId", "Description", false, "", false); EXPECT_FALSE(route1.Equals(route5)); + + // Same as route1 with different off_the_record. + MediaRoute route6(kRouteId1, MediaSourceForCastApp("DialApp"), "sinkId", + "Description", true, "", false); + route6.set_off_the_record(true); + EXPECT_TRUE(route1.Equals(route6)); } } // namespace media_router diff --git a/chrome/browser/media/router/media_router.h b/chrome/browser/media/router/media_router.h index 82a17aa..f8d6f28 100644 --- a/chrome/browser/media/router/media_router.h +++ b/chrome/browser/media/router/media_router.h @@ -72,13 +72,16 @@ class MediaRouter : public KeyedService { // success or failure, in the order they are listed. // If |timeout| is positive, then any un-invoked |callbacks| will be invoked // with a timeout error after the timeout expires. + // If |off_the_record| is true, the request was made by an off the record + // (incognito) profile. virtual void CreateRoute( const MediaSource::Id& source_id, const MediaSink::Id& sink_id, const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) = 0; + base::TimeDelta timeout, + bool off_the_record) = 0; // Creates a route and connects it to an existing route identified by // |route_id|. |route_id| must refer to a non-local route, unnassociated with @@ -110,13 +113,16 @@ class MediaRouter : public KeyedService { // success or failure, in the order they are listed. // If |timeout| is positive, then any un-invoked |callbacks| will be invoked // with a timeout error after the timeout expires. + // If |off_the_record| is true, the request was made by an off the record + // (incognito) profile. virtual void JoinRoute( const MediaSource::Id& source, const std::string& presentation_id, const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) = 0; + base::TimeDelta timeout, + bool off_the_record) = 0; // Terminates the media route specified by |route_id|. virtual void TerminateRoute(const MediaRoute::Id& route_id) = 0; diff --git a/chrome/browser/media/router/media_router.mojom b/chrome/browser/media/router/media_router.mojom index a7acc5aa..9a2f5e7 100644 --- a/chrome/browser/media/router/media_router.mojom +++ b/chrome/browser/media/router/media_router.mojom @@ -46,6 +46,9 @@ struct MediaRoute { string? custom_controller_path; // Set to true if this route should be displayed for |media_sink_id| in UI. bool for_display; + // Set to true if this route was created by an off the record (incognito) + // profile. + bool off_the_record; }; // Notifications or an actionable events to be shown to the user. @@ -132,6 +135,8 @@ interface MediaRouteProvider { // it is not applicable. // If |timeout_millis| is positive, it will be used in place of the default // timeout defined by Media Route Provider Manager. + // If |off_the_record| is true, the request was made by an off the record + // (incognito) profile. // If the operation was successful, |route| will be defined and // |error_text| will be null. // If the operation failed, |route| will be null and |error_text| @@ -143,15 +148,19 @@ interface MediaRouteProvider { string original_presentation_id, string origin, int32 tab_id, - int64 timeout_millis) => (MediaRoute? route, - string? error_text, - RouteRequestResultCode result_code); + int64 timeout_millis, + bool off_the_record) => + (MediaRoute? route, + string? error_text, + RouteRequestResultCode result_code); // Joins an established route given by |presentation_id| // with |media_source|. // |origin| and |tab_id| are used for validating same-origin/tab scopes. // (See CreateRoute for additional documentation) // If |timeout_millis| is positive, it will be used in place of the default + // If the route request was created by an off the record (incognito) profile, + // |off_the_record| must be true. // timeout defined by Media Route Provider Manager. // If the operation was successful, |route| will be defined and // |error_text| will be null. @@ -163,9 +172,11 @@ interface MediaRouteProvider { string presentation_id, string origin, int32 tab_id, - int64 timeout_millis) => (MediaRoute? route, - string? error_text, - RouteRequestResultCode result_code); + int64 timeout_millis, + bool off_the_record) => + (MediaRoute? route, + string? error_text, + RouteRequestResultCode result_code); // Connects an established route given by |route_id| with // |media_source|. The presentation ID of the route created will be diff --git a/chrome/browser/media/router/media_router_base.cc b/chrome/browser/media/router/media_router_base.cc index 0e8a236..b95f5a4c 100644 --- a/chrome/browser/media/router/media_router_base.cc +++ b/chrome/browser/media/router/media_router_base.cc @@ -6,6 +6,11 @@ #include "base/bind.h" +// TODO(mfoltz): Enforce/verify incognito policies: +// - CreateRoute/JoinRoute with OTR = true/false returns a route with +// OTR = true/false +// - Destroying an incognito profile terminates all OTR routes + namespace media_router { MediaRouterBase::MediaRouterBase() = default; diff --git a/chrome/browser/media/router/media_router_factory.cc b/chrome/browser/media/router/media_router_factory.cc index 245a5ce..8cc918d 100644 --- a/chrome/browser/media/router/media_router_factory.cc +++ b/chrome/browser/media/router/media_router_factory.cc @@ -5,6 +5,7 @@ #include "chrome/browser/media/router/media_router_factory.h" #include "build/build_config.h" +#include "chrome/browser/profiles/incognito_helpers.h" #include "components/keyed_service/content/browser_context_dependency_manager.h" #if defined(OS_ANDROID) @@ -49,6 +50,11 @@ MediaRouterFactory::MediaRouterFactory() MediaRouterFactory::~MediaRouterFactory() { } +content::BrowserContext* MediaRouterFactory::GetBrowserContextToUse( + content::BrowserContext* context) const { + return chrome::GetBrowserContextRedirectedInIncognito(context); +} + KeyedService* MediaRouterFactory::BuildServiceInstanceFor( BrowserContext* context) const { #if defined(OS_ANDROID) diff --git a/chrome/browser/media/router/media_router_factory.h b/chrome/browser/media/router/media_router_factory.h index 7d9081e..627eb87 100644 --- a/chrome/browser/media/router/media_router_factory.h +++ b/chrome/browser/media/router/media_router_factory.h @@ -30,6 +30,9 @@ class MediaRouterFactory : public BrowserContextKeyedServiceFactory { ~MediaRouterFactory() override; // BrowserContextKeyedServiceFactory interface. + content::BrowserContext* GetBrowserContextToUse( + content::BrowserContext* context) const override; + KeyedService* BuildServiceInstanceFor( content::BrowserContext* context) const override; diff --git a/chrome/browser/media/router/media_router_mojo_impl.cc b/chrome/browser/media/router/media_router_mojo_impl.cc index 454a140..787dbdb 100644 --- a/chrome/browser/media/router/media_router_mojo_impl.cc +++ b/chrome/browser/media/router/media_router_mojo_impl.cc @@ -251,7 +251,8 @@ void MediaRouterMojoImpl::CreateRoute( const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) { + base::TimeDelta timeout, + bool off_the_record) { DCHECK(thread_checker_.CalledOnValidThread()); if (!origin.is_valid()) { @@ -268,7 +269,7 @@ void MediaRouterMojoImpl::CreateRoute( RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoCreateRoute, base::Unretained(this), source_id, sink_id, origin.is_empty() ? "" : origin.spec(), tab_id, - callbacks, timeout)); + callbacks, timeout, off_the_record)); } void MediaRouterMojoImpl::JoinRoute( @@ -277,7 +278,8 @@ void MediaRouterMojoImpl::JoinRoute( const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) { + base::TimeDelta timeout, + bool off_the_record) { DCHECK(thread_checker_.CalledOnValidThread()); if (!origin.is_valid()) { @@ -294,7 +296,7 @@ void MediaRouterMojoImpl::JoinRoute( RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoJoinRoute, base::Unretained(this), source_id, presentation_id, origin.is_empty() ? "" : origin.spec(), tab_id, - callbacks, timeout)); + callbacks, timeout, off_the_record)); } void MediaRouterMojoImpl::ConnectRouteByRouteId( @@ -534,7 +536,8 @@ void MediaRouterMojoImpl::DoCreateRoute( const std::string& origin, int tab_id, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) { + base::TimeDelta timeout, + bool off_the_record) { std::string presentation_id("mr_"); presentation_id += base::GenerateGUID(); DVLOG_WITH_INSTANCE(1) << "DoCreateRoute " << source_id << "=>" << sink_id @@ -543,6 +546,7 @@ void MediaRouterMojoImpl::DoCreateRoute( media_route_provider_->CreateRoute( source_id, sink_id, presentation_id, origin, tab_id, timeout > base::TimeDelta() ? timeout.InMilliseconds() : 0, + off_the_record, base::Bind(&MediaRouterMojoImpl::RouteResponseReceived, base::Unretained(this), presentation_id, callbacks)); } @@ -553,13 +557,15 @@ void MediaRouterMojoImpl::DoJoinRoute( const std::string& origin, int tab_id, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) { + base::TimeDelta timeout, + bool off_the_record) { DVLOG_WITH_INSTANCE(1) << "DoJoinRoute " << source_id << ", presentation ID: " << presentation_id; media_route_provider_->JoinRoute( source_id, presentation_id, origin, tab_id, timeout > base::TimeDelta() ? timeout.InMilliseconds() : 0, + off_the_record, base::Bind(&MediaRouterMojoImpl::RouteResponseReceived, base::Unretained(this), presentation_id, callbacks)); } diff --git a/chrome/browser/media/router/media_router_mojo_impl.h b/chrome/browser/media/router/media_router_mojo_impl.h index aaca9eb..bfe616a 100644 --- a/chrome/browser/media/router/media_router_mojo_impl.h +++ b/chrome/browser/media/router/media_router_mojo_impl.h @@ -69,13 +69,15 @@ class MediaRouterMojoImpl : public MediaRouterBase, const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) override; + base::TimeDelta timeout, + bool off_the_record) override; void JoinRoute(const MediaSource::Id& source_id, const std::string& presentation_id, const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) override; + base::TimeDelta timeout, + bool off_the_record) override; void ConnectRouteByRouteId( const MediaSource::Id& source, const MediaRoute::Id& route_id, @@ -153,6 +155,8 @@ class MediaRouterMojoImpl : public MediaRouterBase, // Empty otherwise. std::vector<MediaSink> cached_sink_list; base::ObserverList<MediaSinksObserver> observers; + + private: DISALLOW_COPY_AND_ASSIGN(MediaSinksQuery); }; @@ -165,6 +169,7 @@ class MediaRouterMojoImpl : public MediaRouterBase, bool is_active = false; base::ObserverList<MediaRoutesObserver> observers; + private: DISALLOW_COPY_AND_ASSIGN(MediaRoutesQuery); }; @@ -212,13 +217,15 @@ class MediaRouterMojoImpl : public MediaRouterBase, const std::string& origin, int tab_id, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout); + base::TimeDelta timeout, + bool off_the_record); void DoJoinRoute(const MediaSource::Id& source_id, const std::string& presentation_id, const std::string& origin, int tab_id, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout); + base::TimeDelta timeout, + bool off_the_record); void DoConnectRouteByRouteId( const MediaSource::Id& source_id, const MediaRoute::Id& route_id, diff --git a/chrome/browser/media/router/media_router_mojo_impl_unittest.cc b/chrome/browser/media/router/media_router_mojo_impl_unittest.cc index 7b585a7..c9cfaac 100644 --- a/chrome/browser/media/router/media_router_mojo_impl_unittest.cc +++ b/chrome/browser/media/router/media_router_mojo_impl_unittest.cc @@ -188,19 +188,19 @@ class RegisterMediaRouteProviderHandler { TEST_F(MediaRouterMojoImplTest, CreateRoute) { MediaSource media_source(kSource); - MediaRoute expected_route(kRouteId, media_source, - kSinkId, "", false, "", false); + MediaRoute expected_route(kRouteId, media_source, kSinkId, "", false, "", + false); // Use a lambda function as an invocation target here to work around // a limitation with GMock::Invoke that prevents it from using move-only types // in runnable parameter lists. EXPECT_CALL(mock_media_route_provider_, CreateRoute(mojo::String(kSource), mojo::String(kSinkId), _, - mojo::String(kOrigin), kInvalidTabId, _, _)) + mojo::String(kOrigin), kInvalidTabId, _, _, _)) .WillOnce(Invoke( [](const mojo::String& source, const mojo::String& sink, const mojo::String& presentation_id, const mojo::String& origin, - int tab_id, int64_t timeout_millis, + int tab_id, int64_t timeout_millis, bool off_the_record, const interfaces::MediaRouteProvider::CreateRouteCallback& cb) { interfaces::MediaRoutePtr route = interfaces::MediaRoute::New(); route->media_source = kSource; @@ -209,6 +209,7 @@ TEST_F(MediaRouterMojoImplTest, CreateRoute) { route->description = kDescription; route->is_local = true; route->for_display = true; + route->off_the_record = false; cb.Run(std::move(route), mojo::String(), interfaces::RouteRequestResultCode::OK); })); @@ -221,9 +222,9 @@ TEST_F(MediaRouterMojoImplTest, CreateRoute) { std::vector<MediaRouteResponseCallback> route_response_callbacks; route_response_callbacks.push_back(base::Bind( &RouteResponseCallbackHandler::Invoke, base::Unretained(&handler))); - router()->CreateRoute(kSource, kSinkId, GURL(kOrigin), nullptr, - route_response_callbacks, - base::TimeDelta::FromMilliseconds(kTimeoutMillis)); + router()->CreateRoute( + kSource, kSinkId, GURL(kOrigin), nullptr, route_response_callbacks, + base::TimeDelta::FromMilliseconds(kTimeoutMillis), false); run_loop.Run(); } @@ -231,11 +232,11 @@ TEST_F(MediaRouterMojoImplTest, CreateRouteFails) { EXPECT_CALL( mock_media_route_provider_, CreateRoute(mojo::String(kSource), mojo::String(kSinkId), _, - mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, _)) + mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, _, _)) .WillOnce(Invoke( [](const mojo::String& source, const mojo::String& sink, const mojo::String& presentation_id, const mojo::String& origin, - int tab_id, int64_t timeout_millis, + int tab_id, int64_t timeout_millis, bool off_the_record, const interfaces::MediaRouteProvider::CreateRouteCallback& cb) { cb.Run(interfaces::MediaRoutePtr(), mojo::String(kError), interfaces::RouteRequestResultCode::TIMED_OUT); @@ -249,16 +250,16 @@ TEST_F(MediaRouterMojoImplTest, CreateRouteFails) { std::vector<MediaRouteResponseCallback> route_response_callbacks; route_response_callbacks.push_back(base::Bind( &RouteResponseCallbackHandler::Invoke, base::Unretained(&handler))); - router()->CreateRoute(kSource, kSinkId, GURL(kOrigin), nullptr, - route_response_callbacks, - base::TimeDelta::FromMilliseconds(kTimeoutMillis)); + router()->CreateRoute( + kSource, kSinkId, GURL(kOrigin), nullptr, route_response_callbacks, + base::TimeDelta::FromMilliseconds(kTimeoutMillis), false); run_loop.Run(); } TEST_F(MediaRouterMojoImplTest, JoinRoute) { MediaSource media_source(kSource); - MediaRoute expected_route(kRouteId, media_source, - kSinkId, "", false, "", false); + MediaRoute expected_route(kRouteId, media_source, kSinkId, "", false, "", + false); interfaces::MediaRoutePtr route = interfaces::MediaRoute::New(); route->media_source = kSource; route->media_sink_id = kSinkId; @@ -266,6 +267,7 @@ TEST_F(MediaRouterMojoImplTest, JoinRoute) { route->description = kDescription; route->is_local = true; route->for_display = true; + route->off_the_record = false; // Use a lambda function as an invocation target here to work around // a limitation with GMock::Invoke that prevents it from using move-only types @@ -273,10 +275,11 @@ TEST_F(MediaRouterMojoImplTest, JoinRoute) { EXPECT_CALL( mock_media_route_provider_, JoinRoute(mojo::String(kSource), mojo::String(kPresentationId), - mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, _)) + mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, _, _)) .WillOnce(Invoke([&route]( const mojo::String& source, const mojo::String& presentation_id, const mojo::String& origin, int tab_id, int64_t timeout_millis, + bool off_the_record, const interfaces::MediaRouteProvider::JoinRouteCallback& cb) { cb.Run(std::move(route), mojo::String(), interfaces::RouteRequestResultCode::OK); @@ -292,7 +295,7 @@ TEST_F(MediaRouterMojoImplTest, JoinRoute) { &RouteResponseCallbackHandler::Invoke, base::Unretained(&handler))); router()->JoinRoute(kSource, kPresentationId, GURL(kOrigin), nullptr, route_response_callbacks, - base::TimeDelta::FromMilliseconds(kTimeoutMillis)); + base::TimeDelta::FromMilliseconds(kTimeoutMillis), false); run_loop.Run(); } @@ -300,10 +303,11 @@ TEST_F(MediaRouterMojoImplTest, JoinRouteFails) { EXPECT_CALL( mock_media_route_provider_, JoinRoute(mojo::String(kSource), mojo::String(kPresentationId), - mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, _)) + mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, _, _)) .WillOnce(Invoke( [](const mojo::String& source, const mojo::String& presentation_id, const mojo::String& origin, int tab_id, int64_t timeout_millis, + bool off_the_record, const interfaces::MediaRouteProvider::JoinRouteCallback& cb) { cb.Run(interfaces::MediaRoutePtr(), mojo::String(kError), interfaces::RouteRequestResultCode::TIMED_OUT); @@ -319,14 +323,15 @@ TEST_F(MediaRouterMojoImplTest, JoinRouteFails) { &RouteResponseCallbackHandler::Invoke, base::Unretained(&handler))); router()->JoinRoute(kSource, kPresentationId, GURL(kOrigin), nullptr, route_response_callbacks, - base::TimeDelta::FromMilliseconds(kTimeoutMillis)); + base::TimeDelta::FromMilliseconds(kTimeoutMillis), false); run_loop.Run(); } TEST_F(MediaRouterMojoImplTest, ConnectRouteByRouteId) { MediaSource media_source(kSource); - MediaRoute expected_route(kRouteId, media_source, - kSinkId, "", false, "", false); + MediaRoute expected_route(kRouteId, media_source, kSinkId, "", false, "", + false); + expected_route.set_off_the_record(true); interfaces::MediaRoutePtr route = interfaces::MediaRoute::New(); route->media_source = kSource; route->media_sink_id = kSinkId; @@ -334,6 +339,7 @@ TEST_F(MediaRouterMojoImplTest, ConnectRouteByRouteId) { route->description = kDescription; route->is_local = true; route->for_display = true; + route->off_the_record = true; // Use a lambda function as an invocation target here to work around // a limitation with GMock::Invoke that prevents it from using move-only types @@ -645,8 +651,10 @@ TEST_F(MediaRouterMojoImplTest, RegisterAndUnregisterMediaRoutesObserver) { std::vector<MediaRoute> expected_routes; expected_routes.push_back(MediaRoute(kRouteId, media_source, kSinkId, kDescription, false, "", false)); - expected_routes.push_back(MediaRoute(kRouteId2, media_source, kSinkId, - kDescription, false, "", false)); + MediaRoute incognito_expected_route(kRouteId2, media_source, kSinkId, + kDescription, false, "", false); + incognito_expected_route.set_off_the_record(true); + expected_routes.push_back(incognito_expected_route); std::vector<MediaRoute::Id> expected_joinable_route_ids; expected_joinable_route_ids.push_back(kJoinableRouteId); expected_joinable_route_ids.push_back(kJoinableRouteId2); @@ -662,12 +670,14 @@ TEST_F(MediaRouterMojoImplTest, RegisterAndUnregisterMediaRoutesObserver) { mojo_routes[0]->media_sink_id = kSinkId; mojo_routes[0]->description = kDescription; mojo_routes[0]->is_local = false; + mojo_routes[0]->off_the_record = false; mojo_routes[1] = interfaces::MediaRoute::New(); mojo_routes[1]->media_route_id = kRouteId2; mojo_routes[1]->media_source = kSource; mojo_routes[1]->media_sink_id = kSinkId; mojo_routes[1]->description = kDescription; mojo_routes[1]->is_local = false; + mojo_routes[1]->off_the_record = true; EXPECT_CALL(routes_observer, OnRoutesUpdated(SequenceEquals(expected_routes), diff --git a/chrome/browser/media/router/media_router_type_converters.cc b/chrome/browser/media/router/media_router_type_converters.cc index 2c4ebc8..edbf4b2 100644 --- a/chrome/browser/media/router/media_router_type_converters.cc +++ b/chrome/browser/media/router/media_router_type_converters.cc @@ -74,20 +74,24 @@ TypeConverter<media_router::MediaSink, MediaSinkPtr>::Convert( media_router::MediaRoute TypeConverter<media_router::MediaRoute, MediaRoutePtr>::Convert( const MediaRoutePtr& input) { - return media_router::MediaRoute( + media_router::MediaRoute media_route( input->media_route_id, media_router::MediaSource(input->media_source), input->media_sink_id, input->description, input->is_local, input->custom_controller_path, input->for_display); + media_route.set_off_the_record(input->off_the_record); + return media_route; } // static scoped_ptr<media_router::MediaRoute> TypeConverter<scoped_ptr<media_router::MediaRoute>, MediaRoutePtr>::Convert( const MediaRoutePtr& input) { - return make_scoped_ptr(new media_router::MediaRoute( + scoped_ptr<media_router::MediaRoute> media_route(new media_router::MediaRoute( input->media_route_id, media_router::MediaSource(input->media_source), input->media_sink_id, input->description, input->is_local, input->custom_controller_path, input->for_display)); + media_route->set_off_the_record(input->off_the_record); + return media_route; } media_router::Issue::Severity IssueSeverityFromMojo( diff --git a/chrome/browser/media/router/media_router_type_converters_unittest.cc b/chrome/browser/media/router/media_router_type_converters_unittest.cc index 0f29c65..b10ac47 100644 --- a/chrome/browser/media/router/media_router_type_converters_unittest.cc +++ b/chrome/browser/media/router/media_router_type_converters_unittest.cc @@ -76,6 +76,7 @@ TEST(MediaRouterTypeConvertersTest, ConvertMediaRoute) { MediaSource expected_source(MediaSourceForTab(123)); MediaRoute expected_media_route("routeId1", expected_source, "sinkId", "Description", false, "cast_view.html", true); + expected_media_route.set_off_the_record(true); interfaces::MediaRoutePtr mojo_route(interfaces::MediaRoute::New()); mojo_route->media_route_id = "routeId1"; mojo_route->media_source = expected_source.id(); @@ -84,6 +85,7 @@ TEST(MediaRouterTypeConvertersTest, ConvertMediaRoute) { mojo_route->is_local = false; mojo_route->custom_controller_path = "cast_view.html"; mojo_route->for_display = true; + mojo_route->off_the_record = true; MediaRoute media_route = mojo_route.To<MediaRoute>(); EXPECT_TRUE(expected_media_route.Equals(media_route)); @@ -97,6 +99,8 @@ TEST(MediaRouterTypeConvertersTest, ConvertMediaRoute) { EXPECT_EQ(expected_media_route.custom_controller_path(), media_route.custom_controller_path()); EXPECT_EQ(expected_media_route.for_display(), media_route.for_display()); + EXPECT_EQ(expected_media_route.off_the_record(), + media_route.off_the_record()); } TEST(MediaRouterTypeConvertersTest, ConvertMediaRouteWithoutOptionalFields) { @@ -109,6 +113,7 @@ TEST(MediaRouterTypeConvertersTest, ConvertMediaRouteWithoutOptionalFields) { mojo_route->description = "Description"; mojo_route->is_local = false; mojo_route->for_display = false; + mojo_route->off_the_record = false; MediaRoute media_route = mojo_route.To<MediaRoute>(); EXPECT_TRUE(expected_media_route.Equals(media_route)); diff --git a/chrome/browser/media/router/mock_media_router.h b/chrome/browser/media/router/mock_media_router.h index 53fc584..193cba3 100644 --- a/chrome/browser/media/router/mock_media_router.h +++ b/chrome/browser/media/router/mock_media_router.h @@ -25,20 +25,22 @@ class MockMediaRouter : public MediaRouter { MockMediaRouter(); ~MockMediaRouter() override; - MOCK_METHOD6(CreateRoute, + MOCK_METHOD7(CreateRoute, void(const MediaSource::Id& source, const MediaSink::Id& sink_id, const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout)); - MOCK_METHOD6(JoinRoute, + base::TimeDelta timeout, + bool off_the_record)); + MOCK_METHOD7(JoinRoute, void(const MediaSource::Id& source, const std::string& presentation_id, const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout)); + base::TimeDelta timeout, + bool off_the_record)); MOCK_METHOD6(ConnectRouteByRouteId, void(const MediaSource::Id& source, const MediaRoute::Id& route_id, diff --git a/chrome/browser/media/router/presentation_service_delegate_impl.cc b/chrome/browser/media/router/presentation_service_delegate_impl.cc index c0a85e5..5a5ffd6 100644 --- a/chrome/browser/media/router/presentation_service_delegate_impl.cc +++ b/chrome/browser/media/router/presentation_service_delegate_impl.cc @@ -6,6 +6,7 @@ #include <string> #include <utility> +#include <vector> #include "base/containers/scoped_ptr_hash_map.h" #include "base/containers/small_map.h" @@ -23,6 +24,7 @@ #include "chrome/browser/media/router/presentation_session_messages_observer.h" #include "chrome/browser/media/router/route_request_result.h" #include "chrome/browser/sessions/session_tab_helper.h" +#include "content/public/browser/browser_context.h" #include "content/public/browser/presentation_screen_availability_listener.h" #include "content/public/browser/presentation_session.h" #include "content/public/browser/render_frame_host.h" @@ -733,18 +735,20 @@ void PresentationServiceDelegateImpl::JoinSession( const std::string& presentation_id, const content::PresentationSessionStartedCallback& success_cb, const content::PresentationSessionErrorCallback& error_cb) { + bool off_the_record = web_contents_->GetBrowserContext()->IsOffTheRecord(); std::vector<MediaRouteResponseCallback> route_response_callbacks; route_response_callbacks.push_back(base::Bind( &PresentationServiceDelegateImpl::OnJoinRouteResponse, weak_factory_.GetWeakPtr(), render_process_id, render_frame_id, content::PresentationSessionInfo(presentation_url, presentation_id), success_cb, error_cb)); - router_->JoinRoute( - MediaSourceForPresentationUrl(presentation_url).id(), presentation_id, - GetLastCommittedURLForFrame( - RenderFrameHostId(render_process_id, render_frame_id)) - .GetOrigin(), - web_contents_, route_response_callbacks, base::TimeDelta()); + router_->JoinRoute(MediaSourceForPresentationUrl(presentation_url).id(), + presentation_id, + GetLastCommittedURLForFrame( + RenderFrameHostId(render_process_id, render_frame_id)) + .GetOrigin(), + web_contents_, route_response_callbacks, base::TimeDelta(), + off_the_record); } void PresentationServiceDelegateImpl::CloseConnection( diff --git a/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc b/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc index f3f2084..defefaa 100644 --- a/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc +++ b/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc @@ -12,8 +12,10 @@ #include "chrome/browser/media/router/presentation_service_delegate_impl.h" #include "chrome/browser/media/router/route_request_result.h" #include "chrome/browser/media/router/test_helper.h" +#include "chrome/browser/profiles/profile.h" #include "chrome/grit/generated_resources.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h" +#include "chrome/test/base/testing_profile.h" #include "content/public/browser/presentation_screen_availability_listener.h" #include "content/public/browser/presentation_session.h" #include "content/public/browser/render_process_host.h" @@ -57,9 +59,11 @@ class MockCreatePresentationConnnectionCallbacks { class PresentationServiceDelegateImplTest : public ChromeRenderViewHostTestHarness { public: + PresentationServiceDelegateImplTest() : delegate_impl_(nullptr) {} + void SetUp() override { ChromeRenderViewHostTestHarness::SetUp(); - content::WebContents* wc = web_contents(); + content::WebContents* wc = GetWebContents(); ASSERT_TRUE(wc); PresentationServiceDelegateImpl::CreateForWebContents(wc); delegate_impl_ = PresentationServiceDelegateImpl::FromWebContents(wc); @@ -69,13 +73,90 @@ class PresentationServiceDelegateImplTest MOCK_METHOD1(OnDefaultPresentationStarted, void(const content::PresentationSessionInfo& session_info)); + protected: + virtual content::WebContents* GetWebContents() { return web_contents(); } + + void RunDefaultPresentationUrlCallbackTest(bool off_the_record) { + content::RenderFrameHost* main_frame = GetWebContents()->GetMainFrame(); + ASSERT_TRUE(main_frame); + int render_process_id = main_frame->GetProcess()->GetID(); + int routing_id = main_frame->GetRoutingID(); + + auto callback = base::Bind( + &PresentationServiceDelegateImplTest::OnDefaultPresentationStarted, + base::Unretained(this)); + std::string presentation_url1("http://foo.fakeUrl"); + delegate_impl_->SetDefaultPresentationUrl(render_process_id, routing_id, + presentation_url1, callback); + + ASSERT_TRUE(delegate_impl_->HasDefaultPresentationRequest()); + PresentationRequest request = + delegate_impl_->GetDefaultPresentationRequest(); + + // Should not trigger callback since route response is error. + scoped_ptr<RouteRequestResult> result = RouteRequestResult::FromError( + "Error", RouteRequestResult::UNKNOWN_ERROR); + delegate_impl_->OnRouteResponse(request, *result); + EXPECT_TRUE(Mock::VerifyAndClearExpectations(this)); + + // Should not trigger callback since request doesn't match. + std::string presentation_url2("http://bar.fakeUrl"); + PresentationRequest different_request( + RenderFrameHostId(100, 200), presentation_url2, + GURL("http://anotherFrameUrl.fakeUrl")); + MediaRoute* media_route = new MediaRoute( + "differentRouteId", MediaSourceForPresentationUrl(presentation_url2), + "mediaSinkId", "", true, "", true); + media_route->set_off_the_record(off_the_record); + result = RouteRequestResult::FromSuccess(make_scoped_ptr(media_route), + "differentPresentationId"); + delegate_impl_->OnRouteResponse(different_request, *result); + EXPECT_TRUE(Mock::VerifyAndClearExpectations(this)); + + // Should trigger callback since request matches. + EXPECT_CALL(*this, OnDefaultPresentationStarted(_)).Times(1); + MediaRoute* media_route2 = new MediaRoute( + "routeId", MediaSourceForPresentationUrl(presentation_url1), + "mediaSinkId", "", true, "", true); + media_route2->set_off_the_record(off_the_record); + result = RouteRequestResult::FromSuccess(make_scoped_ptr(media_route2), + "presentationId"); + delegate_impl_->OnRouteResponse(request, *result); + } + PresentationServiceDelegateImpl* delegate_impl_; MockMediaRouter router_; }; -TEST_F(PresentationServiceDelegateImplTest, AddScreenAvailabilityListener) { - ON_CALL(router_, RegisterMediaSinksObserver(_)).WillByDefault(Return(true)); +class PresentationServiceDelegateImplIncognitoTest + : public PresentationServiceDelegateImplTest { + public: + PresentationServiceDelegateImplIncognitoTest() : + incognito_web_contents_(nullptr) {} + + protected: + content::WebContents* GetWebContents() override { + if (!incognito_web_contents_) { + Profile* incognito_profile = profile()->GetOffTheRecordProfile(); + incognito_web_contents_ = + content::WebContentsTester::CreateTestWebContents(incognito_profile, + nullptr); + } + return incognito_web_contents_; + } + + void TearDown() override { + // We must delete the incognito WC first, as that triggers observers which + // require RenderViewHost, etc., that in turn are deleted by + // RenderViewHostTestHarness::TearDown(). + delete incognito_web_contents_; + PresentationServiceDelegateImplTest::TearDown(); + } + + content::WebContents* incognito_web_contents_; +}; +TEST_F(PresentationServiceDelegateImplTest, AddScreenAvailabilityListener) { std::string presentation_url1("http://url1.fakeUrl"); std::string presentation_url2("http://url2.fakeUrl"); MediaSource source1 = MediaSourceForPresentationUrl(presentation_url1); @@ -86,7 +167,9 @@ TEST_F(PresentationServiceDelegateImplTest, AddScreenAvailabilityListener) { int render_frame_id1 = 1; int render_frame_id2 = 2; - EXPECT_CALL(router_, RegisterMediaSinksObserver(_)).Times(2); + EXPECT_CALL(router_, RegisterMediaSinksObserver(_)) + .Times(2) + .WillRepeatedly(Return(true)); EXPECT_TRUE(delegate_impl_->AddScreenAvailabilityListener( render_process_id, render_frame_id1, &listener1)); EXPECT_TRUE(delegate_impl_->AddScreenAvailabilityListener( @@ -110,15 +193,13 @@ TEST_F(PresentationServiceDelegateImplTest, AddScreenAvailabilityListener) { } TEST_F(PresentationServiceDelegateImplTest, AddSameListenerTwice) { - ON_CALL(router_, RegisterMediaSinksObserver(_)).WillByDefault(Return(true)); - std::string presentation_url1("http://url1.fakeUrl"); MediaSource source1(MediaSourceForPresentationUrl(presentation_url1)); MockScreenAvailabilityListener listener1(presentation_url1); int render_process_id = 1; int render_frame_id = 0; - EXPECT_CALL(router_, RegisterMediaSinksObserver(_)).Times(1); + EXPECT_CALL(router_, RegisterMediaSinksObserver(_)).WillOnce(Return(true)); EXPECT_TRUE(delegate_impl_->AddScreenAvailabilityListener( render_process_id, render_frame_id, &listener1)); EXPECT_FALSE(delegate_impl_->AddScreenAvailabilityListener( @@ -139,8 +220,9 @@ TEST_F(PresentationServiceDelegateImplTest, SetDefaultPresentationUrl) { EXPECT_FALSE(delegate_impl_->HasDefaultPresentationRequest()); GURL frame_url("http://www.google.com"); - content::WebContentsTester::For(web_contents())->NavigateAndCommit(frame_url); - content::RenderFrameHost* main_frame = web_contents()->GetMainFrame(); + content::WebContentsTester::For(GetWebContents()) + ->NavigateAndCommit(frame_url); + content::RenderFrameHost* main_frame = GetWebContents()->GetMainFrame(); ASSERT_TRUE(main_frame); int render_process_id = main_frame->GetProcess()->GetID(); int routing_id = main_frame->GetRoutingID(); @@ -178,48 +260,12 @@ TEST_F(PresentationServiceDelegateImplTest, SetDefaultPresentationUrl) { } TEST_F(PresentationServiceDelegateImplTest, DefaultPresentationUrlCallback) { - content::RenderFrameHost* main_frame = web_contents()->GetMainFrame(); - ASSERT_TRUE(main_frame); - int render_process_id = main_frame->GetProcess()->GetID(); - int routing_id = main_frame->GetRoutingID(); - - auto callback = base::Bind( - &PresentationServiceDelegateImplTest::OnDefaultPresentationStarted, - base::Unretained(this)); - std::string presentation_url1("http://foo.fakeUrl"); - delegate_impl_->SetDefaultPresentationUrl(render_process_id, routing_id, - presentation_url1, callback); + RunDefaultPresentationUrlCallbackTest(false); +} - ASSERT_TRUE(delegate_impl_->HasDefaultPresentationRequest()); - PresentationRequest request = delegate_impl_->GetDefaultPresentationRequest(); - - // Should not trigger callback since route response is error. - scoped_ptr<RouteRequestResult> result = - RouteRequestResult::FromError("Error", RouteRequestResult::UNKNOWN_ERROR); - delegate_impl_->OnRouteResponse(request, *result); - EXPECT_TRUE(Mock::VerifyAndClearExpectations(this)); - - // Should not trigger callback since request doesn't match. - std::string presentation_url2("http://bar.fakeUrl"); - PresentationRequest different_request(RenderFrameHostId(100, 200), - presentation_url2, - GURL("http://anotherFrameUrl.fakeUrl")); - result = RouteRequestResult::FromSuccess( - make_scoped_ptr(new MediaRoute( - "differentRouteId", MediaSourceForPresentationUrl(presentation_url2), - "mediaSinkId", "", true, "", true)), - "differentPresentationId"); - delegate_impl_->OnRouteResponse(different_request, *result); - EXPECT_TRUE(Mock::VerifyAndClearExpectations(this)); - - // Should trigger callback since request matches. - EXPECT_CALL(*this, OnDefaultPresentationStarted(_)).Times(1); - result = RouteRequestResult::FromSuccess( - make_scoped_ptr(new MediaRoute( - "routeId", MediaSourceForPresentationUrl(presentation_url1), - "mediaSinkId", "", true, "", true)), - "presentationId"); - delegate_impl_->OnRouteResponse(request, *result); +TEST_F(PresentationServiceDelegateImplIncognitoTest, + DefaultPresentationUrlCallback) { + RunDefaultPresentationUrlCallbackTest(true); } TEST_F(PresentationServiceDelegateImplTest, @@ -232,8 +278,9 @@ TEST_F(PresentationServiceDelegateImplTest, delegate_impl_->AddDefaultPresentationRequestObserver(&observer); GURL frame_url("http://www.google.com"); - content::WebContentsTester::For(web_contents())->NavigateAndCommit(frame_url); - content::RenderFrameHost* main_frame = web_contents()->GetMainFrame(); + content::WebContentsTester::For(GetWebContents()) + ->NavigateAndCommit(frame_url); + content::RenderFrameHost* main_frame = GetWebContents()->GetMainFrame(); ASSERT_TRUE(main_frame); int render_process_id = main_frame->GetProcess()->GetID(); int routing_id = main_frame->GetRoutingID(); @@ -275,15 +322,16 @@ TEST_F(PresentationServiceDelegateImplTest, TEST_F(PresentationServiceDelegateImplTest, ListenForConnnectionStateChange) { GURL frame_url("http://www.google.com"); - content::WebContentsTester::For(web_contents())->NavigateAndCommit(frame_url); - content::RenderFrameHost* main_frame = web_contents()->GetMainFrame(); + content::WebContentsTester::For(GetWebContents()) + ->NavigateAndCommit(frame_url); + content::RenderFrameHost* main_frame = GetWebContents()->GetMainFrame(); ASSERT_TRUE(main_frame); int render_process_id = main_frame->GetProcess()->GetID(); int routing_id = main_frame->GetRoutingID(); // Set up a PresentationConnection so we can listen to it. std::vector<MediaRouteResponseCallback> route_response_callbacks; - EXPECT_CALL(router_, JoinRoute(_, _, _, _, _, _)) + EXPECT_CALL(router_, JoinRoute(_, _, _, _, _, _, false)) .WillOnce(SaveArg<4>(&route_response_callbacks)); const std::string kPresentationUrl("http://url1.fakeUrl"); @@ -321,7 +369,8 @@ TEST_F(PresentationServiceDelegateImplTest, ListenForConnnectionStateChange) { } TEST_F(PresentationServiceDelegateImplTest, Reset) { - ON_CALL(router_, RegisterMediaSinksObserver(_)).WillByDefault(Return(true)); + EXPECT_CALL(router_, RegisterMediaSinksObserver(_)) + .WillRepeatedly(Return(true)); std::string presentation_url1("http://url1.fakeUrl"); MediaSource source = MediaSourceForPresentationUrl(presentation_url1); @@ -341,7 +390,7 @@ TEST_F(PresentationServiceDelegateImplTest, Reset) { TEST_F(PresentationServiceDelegateImplTest, DelegateObservers) { scoped_ptr<PresentationServiceDelegateImpl> manager( - new PresentationServiceDelegateImpl(web_contents())); + new PresentationServiceDelegateImpl(GetWebContents())); manager->SetMediaRouterForTest(&router_); StrictMock<MockDelegateObserver> delegate_observer1; @@ -358,14 +407,13 @@ TEST_F(PresentationServiceDelegateImplTest, DelegateObservers) { } TEST_F(PresentationServiceDelegateImplTest, SinksObserverCantRegister) { - ON_CALL(router_, RegisterMediaSinksObserver(_)).WillByDefault(Return(false)); - const std::string presentation_url("http://url1.fakeUrl"); MockScreenAvailabilityListener listener(presentation_url); const int render_process_id = 10; const int render_frame_id = 1; - EXPECT_CALL(router_, RegisterMediaSinksObserver(_)).Times(1); + EXPECT_CALL(router_, RegisterMediaSinksObserver(_)).WillOnce(Return(false)); + EXPECT_CALL(listener, OnScreenAvailabilityNotSupported()); EXPECT_FALSE(delegate_impl_->AddScreenAvailabilityListener( render_process_id, render_frame_id, &listener)); } diff --git a/chrome/browser/media/router/test_helper.h b/chrome/browser/media/router/test_helper.h index 2406c6a..c3369e1 100644 --- a/chrome/browser/media/router/test_helper.h +++ b/chrome/browser/media/router/test_helper.h @@ -93,20 +93,22 @@ class MockMediaRouteProvider : public interfaces::MediaRouteProvider { MockMediaRouteProvider(); ~MockMediaRouteProvider() override; - MOCK_METHOD7(CreateRoute, + MOCK_METHOD8(CreateRoute, void(const mojo::String& source_urn, const mojo::String& sink_id, const mojo::String& presentation_id, const mojo::String& origin, int tab_id, int64_t timeout_secs, + bool off_the_record, const CreateRouteCallback& callback)); - MOCK_METHOD6(JoinRoute, + MOCK_METHOD7(JoinRoute, void(const mojo::String& source_urn, const mojo::String& presentation_id, const mojo::String& origin, int tab_id, int64_t timeout_secs, + bool off_the_record, const JoinRouteCallback& callback)); MOCK_METHOD7(ConnectRouteByRouteId, void(const mojo::String& source_urn, diff --git a/chrome/browser/ui/ash/cast_config_delegate_media_router.cc b/chrome/browser/ui/ash/cast_config_delegate_media_router.cc index 2fe6a87..8b62fc9 100644 --- a/chrome/browser/ui/ash/cast_config_delegate_media_router.cc +++ b/chrome/browser/ui/ash/cast_config_delegate_media_router.cc @@ -4,6 +4,9 @@ #include "chrome/browser/ui/ash/cast_config_delegate_media_router.h" +#include <string> +#include <vector> + #include "base/macros.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" @@ -197,7 +200,7 @@ void CastConfigDelegateMediaRouter::CastToReceiver( media_router::MediaSourceForDesktop().id(), receiver_id, GURL("http://cros-cast-origin/"), nullptr, std::vector<media_router::MediaRouteResponseCallback>(), - base::TimeDelta()); + base::TimeDelta(), false); } void CastConfigDelegateMediaRouter::StopCasting(const std::string& route_id) { diff --git a/chrome/browser/ui/webui/media_router/media_router_ui.cc b/chrome/browser/ui/webui/media_router/media_router_ui.cc index 2d2d522..0069f87 100644 --- a/chrome/browser/ui/webui/media_router/media_router_ui.cc +++ b/chrome/browser/ui/webui/media_router/media_router_ui.cc @@ -407,7 +407,8 @@ bool MediaRouterUI::CreateOrConnectRoute(const MediaSink::Id& sink_id, base::TimeDelta timeout = GetRouteRequestTimeout(cast_mode); if (route_id.empty()) { router_->CreateRoute(source.id(), sink_id, origin, initiator_, - route_response_callbacks, timeout); + route_response_callbacks, timeout, + Profile::FromWebUI(web_ui())->IsOffTheRecord()); } else { router_->ConnectRouteByRouteId(source.id(), route_id, origin, initiator_, route_response_callbacks, timeout); diff --git a/chrome/browser/ui/webui/media_router/media_router_ui.h b/chrome/browser/ui/webui/media_router/media_router_ui.h index 017417c..3daf9ea 100644 --- a/chrome/browser/ui/webui/media_router/media_router_ui.h +++ b/chrome/browser/ui/webui/media_router/media_router_ui.h @@ -157,6 +157,7 @@ class MediaRouterUI : public ConstrainedWebDialogUI, GetExtensionNameEmptyWhenNotExtensionURL); FRIEND_TEST_ALL_PREFIXES(MediaRouterUITest, RouteCreationTimeoutForPresentation); + FRIEND_TEST_ALL_PREFIXES(MediaRouterUITest, RouteRequestFromIncognito); class UIIssuesObserver; diff --git a/chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc b/chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc index 2a92d49..54efa24 100644 --- a/chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc +++ b/chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc @@ -35,11 +35,18 @@ class MockRoutesUpdatedCallback { class MediaRouterUITest : public ::testing::Test { public: - MediaRouterUITest() { + ~MediaRouterUITest() override { + EXPECT_CALL(mock_router_, UnregisterMediaSinksObserver(_)) + .Times(AnyNumber()); + EXPECT_CALL(mock_router_, UnregisterMediaRoutesObserver(_)) + .Times(AnyNumber()); + } + + void CreateMediaRouterUI(Profile* profile) { initiator_.reset(content::WebContents::Create( - content::WebContents::CreateParams(&profile_))); + content::WebContents::CreateParams(profile))); web_contents_.reset(content::WebContents::Create( - content::WebContents::CreateParams(&profile_))); + content::WebContents::CreateParams(profile))); web_ui_.set_web_contents(web_contents_.get()); media_router_ui_.reset(new MediaRouterUI(&web_ui_)); message_handler_.reset( @@ -53,13 +60,6 @@ class MediaRouterUITest : public ::testing::Test { message_handler_->SetWebUIForTest(&web_ui_); } - ~MediaRouterUITest() override { - EXPECT_CALL(mock_router_, UnregisterMediaSinksObserver(_)) - .Times(AnyNumber()); - EXPECT_CALL(mock_router_, UnregisterMediaRoutesObserver(_)) - .Times(AnyNumber()); - } - protected: MockMediaRouter mock_router_; content::TestBrowserThreadBundle thread_bundle_; @@ -72,8 +72,9 @@ class MediaRouterUITest : public ::testing::Test { }; TEST_F(MediaRouterUITest, RouteRequestTimedOut) { + CreateMediaRouterUI(&profile_); std::vector<MediaRouteResponseCallback> callbacks; - EXPECT_CALL(mock_router_, CreateRoute(_, _, _, _, _, _)) + EXPECT_CALL(mock_router_, CreateRoute(_, _, _, _, _, _, _)) .WillOnce(SaveArg<4>(&callbacks)); media_router_ui_->CreateRoute("sinkId", MediaCastMode::TAB_MIRROR); @@ -85,28 +86,48 @@ TEST_F(MediaRouterUITest, RouteRequestTimedOut) { } TEST_F(MediaRouterUITest, RouteCreationTimeoutForTab) { - EXPECT_CALL(mock_router_, - CreateRoute(_, _, _, _, _, base::TimeDelta::FromSeconds(60))); + CreateMediaRouterUI(&profile_); + EXPECT_CALL( + mock_router_, + CreateRoute(_, _, _, _, _, base::TimeDelta::FromSeconds(60), false)); media_router_ui_->CreateRoute("sinkId", MediaCastMode::TAB_MIRROR); } TEST_F(MediaRouterUITest, RouteCreationTimeoutForDesktop) { - EXPECT_CALL(mock_router_, - CreateRoute(_, _, _, _, _, base::TimeDelta::FromSeconds(120))); + CreateMediaRouterUI(&profile_); + EXPECT_CALL( + mock_router_, + CreateRoute(_, _, _, _, _, base::TimeDelta::FromSeconds(120), false)); media_router_ui_->CreateRoute("sinkId", MediaCastMode::DESKTOP_MIRROR); } TEST_F(MediaRouterUITest, RouteCreationTimeoutForPresentation) { + CreateMediaRouterUI(&profile_); + PresentationRequest presentation_request( + RenderFrameHostId(0, 0), "https://fooUrl", GURL("https://frameUrl")); + media_router_ui_->OnDefaultPresentationChanged(presentation_request); + + EXPECT_CALL( + mock_router_, + CreateRoute(_, _, _, _, _, base::TimeDelta::FromSeconds(20), false)); + media_router_ui_->CreateRoute("sinkId", MediaCastMode::DEFAULT); +} + +TEST_F(MediaRouterUITest, RouteRequestFromIncognito) { + CreateMediaRouterUI(profile_.GetOffTheRecordProfile()); + PresentationRequest presentation_request( RenderFrameHostId(0, 0), "https://fooUrl", GURL("https://frameUrl")); media_router_ui_->OnDefaultPresentationChanged(presentation_request); - EXPECT_CALL(mock_router_, - CreateRoute(_, _, _, _, _, base::TimeDelta::FromSeconds(20))); + EXPECT_CALL( + mock_router_, + CreateRoute(_, _, _, _, _, base::TimeDelta::FromSeconds(20), true)); media_router_ui_->CreateRoute("sinkId", MediaCastMode::DEFAULT); } TEST_F(MediaRouterUITest, SortedSinks) { + CreateMediaRouterUI(&profile_); std::vector<MediaSinkWithCastModes> unsorted_sinks; std::string sink_id1("sink3"); std::string sink_name1("B sink"); @@ -144,12 +165,12 @@ TEST_F(MediaRouterUITest, UIMediaRoutesObserverFiltersNonDisplayRoutes) { base::Bind(&MockRoutesUpdatedCallback::OnRoutesUpdated, base::Unretained(&mock_callback)))); - MediaRoute display_route_1("routeId1", media_source, "sinkId1", - "desc 1", true, "", true); - MediaRoute non_display_route_1("routeId2", media_source, - "sinkId2", "desc 2", true, "", false); - MediaRoute display_route_2("routeId3", media_source, "sinkId2", - "desc 2", true, "", true); + MediaRoute display_route_1("routeId1", media_source, "sinkId1", "desc 1", + true, "", true); + MediaRoute non_display_route_1("routeId2", media_source, "sinkId2", "desc 2", + true, "", false); + MediaRoute display_route_2("routeId3", media_source, "sinkId2", "desc 2", + true, "", true); std::vector<MediaRoute> routes; routes.push_back(display_route_1); routes.push_back(non_display_route_1); @@ -182,12 +203,12 @@ TEST_F(MediaRouterUITest, base::Bind(&MockRoutesUpdatedCallback::OnRoutesUpdated, base::Unretained(&mock_callback)))); - MediaRoute display_route_1("routeId1", media_source, "sinkId1", - "desc 1", true, "", true); - MediaRoute non_display_route_1("routeId2", media_source, - "sinkId2", "desc 2", true, "", false); - MediaRoute display_route_2("routeId3", media_source, "sinkId2", - "desc 2", true, "", true); + MediaRoute display_route_1("routeId1", media_source, "sinkId1", "desc 1", + true, "", true); + MediaRoute non_display_route_1("routeId2", media_source, "sinkId2", "desc 2", + true, "", false); + MediaRoute display_route_2("routeId3", media_source, "sinkId2", "desc 2", + true, "", true); std::vector<MediaRoute> routes; routes.push_back(display_route_1); routes.push_back(non_display_route_1); diff --git a/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc b/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc index 336dbe1..dbdaa9c 100644 --- a/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc +++ b/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc @@ -136,6 +136,7 @@ scoped_ptr<base::DictionaryValue> RouteToValue( dictionary->SetString("description", route.description()); dictionary->SetBoolean("isLocal", route.is_local()); dictionary->SetBoolean("canJoin", canJoin); + dictionary->SetBoolean("isOffTheRecord", route.off_the_record()); const std::string& custom_path = route.custom_controller_path(); if (!custom_path.empty()) { diff --git a/chrome/browser/ui/webui/media_router/media_router_webui_message_handler_unittest.cc b/chrome/browser/ui/webui/media_router/media_router_webui_message_handler_unittest.cc index f0066f1..0c0686a 100644 --- a/chrome/browser/ui/webui/media_router/media_router_webui_message_handler_unittest.cc +++ b/chrome/browser/ui/webui/media_router/media_router_webui_message_handler_unittest.cc @@ -322,9 +322,8 @@ TEST_F(MediaRouterWebUIMessageHandlerTest, UpdateRoutes) { std::string description("This is a route"); bool is_local = true; std::vector<MediaRoute> routes; - routes.push_back(MediaRoute(route_id, media_source, sink_id, - description, is_local, kControllerPathForTesting, - true)); + routes.push_back(MediaRoute(route_id, media_source, sink_id, description, + is_local, kControllerPathForTesting, true)); std::vector<MediaRoute::Id> joinable_route_ids; joinable_route_ids.push_back(route_id); @@ -372,8 +371,10 @@ TEST_F(MediaRouterWebUIMessageHandlerTest, OnCreateRouteResponseReceived) { std::string description("This is a route"); bool is_local = true; bool is_for_display = true; + bool off_the_record = true; MediaRoute route(route_id, MediaSource("mediaSource"), sink_id, description, is_local, "", is_for_display); + route.set_off_the_record(off_the_record); EXPECT_CALL(*mock_media_router_ui_, GetRouteProviderExtensionId()).WillOnce( ReturnRef(provider_extension_id())); @@ -402,6 +403,11 @@ TEST_F(MediaRouterWebUIMessageHandlerTest, OnCreateRouteResponseReceived) { EXPECT_TRUE(route_value->GetBoolean("isLocal", &actual_is_local)); EXPECT_EQ(is_local, actual_is_local); + bool actual_is_off_the_record = false; + EXPECT_TRUE( + route_value->GetBoolean("isOffTheRecord", &actual_is_off_the_record)); + EXPECT_EQ(off_the_record, actual_is_off_the_record); + const base::Value* arg3 = call_data.arg3(); bool route_for_display = false; ASSERT_TRUE(arg3->GetAsBoolean(&route_for_display)); diff --git a/chrome/test/media_router/media_router_base_browsertest.h b/chrome/test/media_router/media_router_base_browsertest.h index 454960a..91b092d 100644 --- a/chrome/test/media_router/media_router_base_browsertest.h +++ b/chrome/test/media_router/media_router_base_browsertest.h @@ -68,6 +68,8 @@ class MediaRouterBaseBrowserTest : public ExtensionBrowserTest, bool is_extension_host_created() const { return extension_host_created_; } + bool is_off_the_record() { return profile()->IsOffTheRecord(); } + // These values are initialized via flags. base::FilePath extension_crx_; base::FilePath extension_unpacked_; diff --git a/chrome/test/media_router/media_router_e2e_browsertest.cc b/chrome/test/media_router/media_router_e2e_browsertest.cc index 5f0d671..6b232d6 100644 --- a/chrome/test/media_router/media_router_e2e_browsertest.cc +++ b/chrome/test/media_router/media_router_e2e_browsertest.cc @@ -102,7 +102,8 @@ void MediaRouterE2EBrowserTest::CreateMediaRoute( base::Bind(&MediaRouterE2EBrowserTest::OnRouteResponseReceived, base::Unretained(this))); media_router_->CreateRoute(source.id(), sink.id(), origin, web_contents, - route_response_callbacks, base::TimeDelta()); + route_response_callbacks, base::TimeDelta(), + is_off_the_record()); // Wait for the route request to be fulfilled (and route to be started). ASSERT_TRUE(ConditionalWait( diff --git a/extensions/renderer/resources/media_router_bindings.js b/extensions/renderer/resources/media_router_bindings.js index ebb668c..b6252c5 100644 --- a/extensions/renderer/resources/media_router_bindings.js +++ b/extensions/renderer/resources/media_router_bindings.js @@ -75,8 +75,10 @@ define('media_router_bindings', [ 'icon_url': route.iconUrl, 'is_local': route.isLocal, 'custom_controller_path': route.customControllerPath, - // TODO(imcheng): Remove logic when extension always sets the field. - 'for_display': route.forDisplay == undefined ? true : route.forDisplay + // Begin newly added properties, followed by the milestone they were + // added. The guard should be safe to remove N+2 milestones later. + 'for_display': route.forDisplay, // M47 + 'off_the_record': !!route.offTheRecord // M50 }); } @@ -556,15 +558,18 @@ define('media_router_bindings', [ * @param {!number} timeoutMillis If positive, the timeout duration for the * request, measured in seconds. Otherwise, the default duration will be * used. + * @param {!boolean} offTheRecord If true, the route is being requested by + * an off the record (incognito) profile. * @return {!Promise.<!Object>} A Promise resolving to an object describing * the newly created media route, or rejecting with an error message on * failure. */ MediaRouteProvider.prototype.createRoute = function(sourceUrn, sinkId, presentationId, origin, tabId, - timeoutMillis) { + timeoutMillis, offTheRecord) { return this.handlers_.createRoute( - sourceUrn, sinkId, presentationId, origin, tabId, timeoutMillis) + sourceUrn, sinkId, presentationId, origin, tabId, timeoutMillis, + offTheRecord) .then(function(route) { return toSuccessRouteResponse_(route); }, @@ -584,14 +589,17 @@ define('media_router_bindings', [ * @param {!number} timeoutMillis If positive, the timeout duration for the * request, measured in seconds. Otherwise, the default duration will be * used. + * @param {!boolean} offTheRecord If true, the route is being requested by + * an off the record (incognito) profile. * @return {!Promise.<!Object>} A Promise resolving to an object describing * the newly created media route, or rejecting with an error message on * failure. */ MediaRouteProvider.prototype.joinRoute = - function(sourceUrn, presentationId, origin, tabId, timeoutMillis) { + function(sourceUrn, presentationId, origin, tabId, timeoutMillis, + offTheRecord) { return this.handlers_.joinRoute( - sourceUrn, presentationId, origin, tabId, timeoutMillis) + sourceUrn, presentationId, origin, tabId, timeoutMillis, offTheRecord) .then(function(route) { return toSuccessRouteResponse_(route); }, |