[Presentation API] (3rd) (1-UA) Split PresentationServiceDelegateImpl(PSDImpl)

chrome/browser/chrome_content_browser_client.cc

Index: chrome/browser/chrome_content_browser_client.cc
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index bfb73d1f35ab718d9f2fbf1f7285c0b5bd2f05ca..63888177024817539a548c410413ba731d5e5ec2 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -344,6 +344,7 @@
 #if defined(ENABLE_MEDIA_ROUTER)
 #include "chrome/browser/media/router/media_router_feature.h"
 #include "chrome/browser/media/router/presentation_service_delegate_impl.h"
+#include "chrome/browser/media/router/receiver_presentation_service_delegate_impl.h"
 #endif  // defined(ENABLE_MEDIA_ROUTER)
 
 #if BUILDFLAG(ENABLE_MEDIA_REMOTING) && defined(ENABLE_MEDIA_ROUTER)
@@ -3133,9 +3134,9 @@ void ChromeContentBrowserClient::OpenURL(
 #endif
 }
 
-content::PresentationServiceDelegate*
-ChromeContentBrowserClient::GetPresentationServiceDelegate(
-      content::WebContents* web_contents) {
+content::ControllerPresentationServiceDelegate*
+ChromeContentBrowserClient::GetControllerPresentationServiceDelegate(
+    content::WebContents* web_contents) {
 #if defined(ENABLE_MEDIA_ROUTER)
   if (media_router::MediaRouterEnabled(web_contents->GetBrowserContext())) {
     return media_router::PresentationServiceDelegateImpl::
@@ -3145,6 +3146,24 @@ ChromeContentBrowserClient::GetPresentationServiceDelegate(
   return nullptr;
 }
 
+content::ReceiverPresentationServiceDelegate*
+ChromeContentBrowserClient::GetReceiverPresentationServiceDelegate(
+    content::WebContents* web_contents) {
+#if defined(ENABLE_MEDIA_ROUTER)
+  if (media_router::MediaRouterEnabled(web_contents->GetBrowserContext())) {
+    // ReceiverPresentationServiceDelegateImpl exists only for WebContents
+    // created for offscreen presentations. The WebContents must belong to
+    // an incognito profile.
+    auto* impl =
+        media_router::ReceiverPresentationServiceDelegateImpl::FromWebContents(
+            web_contents);
+    if (impl && web_contents->GetBrowserContext()->IsOffTheRecord())

[dcheng, 2017/01/06 11:54:22]

I would expect the IsOffTheRecord() bit to a be a DCHECK(): we shouldn't be
creating it for a  non-OTR browser context at all, should we?

Which means this can just be simplified to:

auto* impl =
media_router::ReceiverPresentationServiceDelegateImpl::FromWebContents(web_contents);
DCHECK(web_contents->GetBrowserContext()->IsOffTheRecord());
return impl;

[zhaobin, 2017/01/10 01:39:17]

Mark suggests not using DCHECK in patch set 1:

mark a. foltz 2016/11/08 23:40:51
API design opinion:

This API is part of content/browser and it's a little weird to have a method
that will DCHECK for some WebContents.  If the caller passes a WebContents* that
cannot be a receiver, I would just return nullptr instead of DCHECK(), and the
caller can check the return value if it expects it to be valid.

[dcheng, 2017/01/10 06:46:58]

It's not really that weird, IMO. If we shouldn't have to handle "impossible
cases" (i.e. the impossible case is developer error), then it's better to DCHECK
rather than fail silently.

As an example, consider base::StringValue. base::StringValue expects to only
hold Unicode strings. If a non-UTF8 std::string is passed, we don't provide a
way to gracefully fail -- we just DCHECK, because the developer did something
wrong.

Unless there's a good reason to do otherwise, I think we should stick to that
pattern here.

[zhaobin, 2017/01/10 21:49:54]

Done.

+      return impl;
+  }
+#endif  // defined(ENABLE_MEDIA_ROUTER)
+  return nullptr;
+}
+
 void ChromeContentBrowserClient::RecordURLMetric(const std::string& metric,
                                                  const GURL& url) {
   if (url.is_valid()) {