[Presentation API] (5th) (1-UA) integrate controller and receiver side for 1-UA messaging

chrome/browser/extensions/api/tab_capture/offscreen_tab.cc

Index: chrome/browser/extensions/api/tab_capture/offscreen_tab.cc
diff --git a/chrome/browser/extensions/api/tab_capture/offscreen_tab.cc b/chrome/browser/extensions/api/tab_capture/offscreen_tab.cc
index 776dd0a32817053d059482c3f2e15d8522bef0cc..cd9fd677a92b587b70ac7482c86c980f7c12b96e 100644
--- a/chrome/browser/extensions/api/tab_capture/offscreen_tab.cc
+++ b/chrome/browser/extensions/api/tab_capture/offscreen_tab.cc
@@ -10,6 +10,7 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "chrome/browser/extensions/api/tab_capture/tab_capture_registry.h"
+#include "chrome/browser/media/router/receiver_presentation_service_delegate_impl.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/web_contents_sizer.h"
 #include "content/public/browser/render_view_host.h"
@@ -117,11 +118,12 @@ void OffscreenTab::Start(const GURL& start_url,
   // automatically unmuted, but will be captured into the MediaStream.
   offscreen_tab_web_contents_->SetAudioMuted(true);
 
-  // TODO(imcheng): If |optional_presentation_id| is not empty, register it with
-  // the PresentationRouter.  http://crbug.com/513859

[miu, 2017/02/02 21:17:34]

Please include 513859 on the BUG= line in this change description.

[zhaobin, 2017/02/03 03:22:54]

Done.

   if (!optional_presentation_id.empty()) {
-    NOTIMPLEMENTED()
-        << "Register with PresentationRouter, id=" << optional_presentation_id;
+    DVLOG(1) << " Register with ReceiverPresentationServiceDelegateImpl, "
+             << "[presentation_id]: " << optional_presentation_id;
+    // Register the offscreen tab as the receiver of the offscreen presentation.

[miu, 2017/02/02 21:17:34]

I think a more-accurate comment here would be something like:

  Create a ReceiverPSDImpl associated with the offscreen tab's WebContents. The
new instance will set up the necessary infrastructure to allow controlling peers
the ability to connect to the offscreen tab.

[zhaobin, 2017/02/03 03:22:54]

Done.

+    media_router::ReceiverPresentationServiceDelegateImpl::CreateForWebContents(

[miu, 2017/02/02 21:17:35]

I dove into the call points from here, and I wonder if there's a potential bug
in assumptions made about the Profiles (BrowserContexts):

1. The offscreen tab will be created from the tab capture extension API. The
extension that launches the tab will be using whatever Profile the MR Extension
was loaded under.

2. The offscreen tab will, of course, create its own incognito Profile based on
the extension's Profile.

3. Code downstream of here will then create an
OffscreenPresentationManagerFactory using the BrowserContextKeyedServiceFactory.
IIUC, the BCKSF will do a backwards-lookup from the offscreen tab's incognito
Profile to get/create a factory that will be associated with the Profile of the
MR Extension.

4. Therefore, will pages in other Profiles that want to start 1UA presentations
be unable to access the OffscreenPresentationManagerFactory to connect to the
offscreen tab?

I suggest you make sure to test using the presentation API 1UA functionality in
multiple-profile scenarios (e.g., browser started-up on one profile, but you can
switch to another profile and start a controlling page that starts a
presentation). A browsertest might be appropriate here.

[zhaobin, 2017/02/03 03:22:54]

reconnect() for 1-ua has not been implemented yet. Created crbug.com/688233 to
track this issue.

+        offscreen_tab_web_contents_.get(), optional_presentation_id);
 
     if (auto* render_view_host =
             offscreen_tab_web_contents_->GetRenderViewHost()) {