[MR] Cancel auto-switching from tab mirroring to Cast SDK at page navigation based on user pref

chrome/browser/media/router/presentation_service_delegate_impl.cc

Index: chrome/browser/media/router/presentation_service_delegate_impl.cc
diff --git a/chrome/browser/media/router/presentation_service_delegate_impl.cc b/chrome/browser/media/router/presentation_service_delegate_impl.cc
index d0c574cfd30e5b0d9beee443660035d56d5e9618..d6d25fee60ad8246842029b7de5f7192e7dca9bb 100644
--- a/chrome/browser/media/router/presentation_service_delegate_impl.cc
+++ b/chrome/browser/media/router/presentation_service_delegate_impl.cc
@@ -25,7 +25,10 @@
 #include "chrome/browser/media/router/route_message.h"
 #include "chrome/browser/media/router/route_message_observer.h"
 #include "chrome/browser/media/router/route_request_result.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
+#include "chrome/common/pref_names.h"
+#include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/presentation_screen_availability_listener.h"
 #include "content/public/browser/presentation_session.h"
@@ -41,6 +44,10 @@ namespace media_router {
 
 namespace {
 
+// This value must be the same as |chrome.cast.AUTO_JOIN_PRESENTATION_ID| in the
+// component extension.
+const char kAutoJoinPresentationId[] = "auto-join";

[mark a. foltz, 2016/11/22 22:23:21]

Slight preference for moving the logic for detecting auto-join into
media_source_helpers.h

[takumif, 2016/12/03 00:46:48]

Done.

+
 using DelegateObserver = content::PresentationServiceDelegate::Observer;
 
 // Returns the unique identifier for the supplied RenderFrameHost.
@@ -818,6 +825,19 @@ void PresentationServiceDelegateImpl::JoinSession(
     error_cb.Run(content::PresentationError(
         content::PRESENTATION_ERROR_NO_PRESENTATION_FOUND,
         "Invalid presentation arguments."));
+    return;
+  }
+
+  GURL origin = GetLastCommittedURLForFrame(

[mark a. foltz, 2016/11/22 22:23:21]

We should use url::Origin for origins going forward instead of GURL.

[takumif, 2016/12/03 00:46:48]

Done.

+                    RenderFrameHostId(render_process_id, render_frame_id))
+                    .GetOrigin();
+
+  if (presentation_id == kAutoJoinPresentationId &&
+      ShouldOverrideAutoJoinForOrigin(origin)) {
+    error_cb.Run(content::PresentationError(
+        content::PRESENTATION_ERROR_SESSION_REQUEST_CANCELLED,
+        "Auto-join request overridden by user preferences."));
+    return;
   }
 
   // TODO(crbug.com/627655): Handle multiple URLs.
@@ -829,12 +849,9 @@ void PresentationServiceDelegateImpl::JoinSession(
       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(), incognito);
+  router_->JoinRoute(MediaSourceForPresentationUrl(presentation_url).id(),
+                     presentation_id, origin, web_contents_,
+                     route_response_callbacks, base::TimeDelta(), incognito);
 }
 
 void PresentationServiceDelegateImpl::CloseConnection(
@@ -967,4 +984,14 @@ bool PresentationServiceDelegateImpl::HasScreenAvailabilityListenerForTest(
       render_frame_host_id, source_id);
 }
 
+bool PresentationServiceDelegateImpl::ShouldOverrideAutoJoinForOrigin(
+    const GURL& origin) const {
+  const base::ListValue* hostnames =

[mark a. foltz, 2016/11/22 22:23:21]

What if hostnames is not set?

[takumif, 2016/12/03 00:46:48]

Adding a check for that.

+      Profile::FromBrowserContext(web_contents_->GetBrowserContext())

[mark a. foltz, 2016/11/22 22:23:21]

What if this is an incognito window?

[takumif, 2016/12/03 00:46:48]

I think we should ignore the preferences (and don't modify them) in
guest/incognito modes. Changing.

+          ->GetPrefs()
+          ->GetList(prefs::kMediaRouterTabMirroringSources);

[mark a. foltz, 2016/11/22 22:23:21]

How expensive is looking up this pref?  Can it change over the lifetime of
PSDImpl? 

[takumif, 2016/11/29 23:56:45]

I'm not sure how expensive this is. It's a disk read, right?
If we want to reduce pref lookups I think we can move this functionality to
MRMojoImpl, since that has a longer lifespan than PSDImpl, and make it cache the
pref value (and become a pref observer and update the cached value when it
changes). What do you think?

[mark a. foltz, 2016/12/01 00:04:34]

It appears unlikely to cause a disk read.  But it is hard to tell because there
are many implementations of the preference store.

My advice: If the pref value can't change outside of this class, I would cache
it.  If it can, then keep the code as-is.

[takumif, 2016/12/03 00:46:48]

Got it. Keeping it as is, as MediaRouterUI changes the pref.

+  return hostnames->Find(base::StringValue(origin.spec())) !=
+         hostnames->end();
+}
+
 }  // namespace media_router