[Presentation API] (MR side) 1-UA: notify receiver page when receiver connection becomes available

content/browser/presentation/presentation_service_impl.cc

Index: content/browser/presentation/presentation_service_impl.cc
diff --git a/content/browser/presentation/presentation_service_impl.cc b/content/browser/presentation/presentation_service_impl.cc
index 900d186b1f50d6c7723a55a93982fcffa2fda26c..af19f9a6b5d7c8cf35474410722a2021a5470cc1 100644
--- a/content/browser/presentation/presentation_service_impl.cc
+++ b/content/browser/presentation/presentation_service_impl.cc
@@ -144,6 +144,8 @@ PresentationServiceImpl::PresentationServiceImpl(
 }
 
 PresentationServiceImpl::~PresentationServiceImpl() {
+  DVLOG(2) << "~PresentationServiceImpl: " << render_process_id_ << ", "
+           << render_frame_id_;
   if (delegate_)
     delegate_->RemoveObserver(render_process_id_, render_frame_id_);
 }
@@ -178,6 +180,12 @@ void PresentationServiceImpl::SetClient(
   DCHECK(!client_.get());
   // TODO(imcheng): Set ErrorHandler to listen for errors.
   client_ = std::move(client);
+
+  // It will invoke onReceiverConnectionAvailable callback
+  // on receiver frame (associated with ReceiverPSDImpl);
+  // It does nothing on controller frame.
+  if (delegate_)
+    delegate_->RegisterOffscreenPresentationReceiver(this);

[imcheng, 2016/09/19 23:01:17]

I have a few concerns with this approach:

1) We are calling this in a common code path, regardless of whether this PSImpl
corresponds to a controller or a receiver. Needing a comment to state that it
does nothing on the controller frame is indicative that that we are not modeling
the relationship between objects / interfaces properly.

2) It appears what the code is trying to achieve is to bind the entire
controller PSImpl to talk to the receiver PSImpl directly and vice versa. In
reality, we only want to use this special direct path for offscreen presentation
connections. For instance, if a controller page has both offscreen and
non-offscreen presentations, it's not clear to me how it would work in that
case.

3) It doesn't seem like we are actually using the special path in here - I see
we are storing |offscreen_presentation_observers_|, but they aren't used. It is
also not clear how the multiple controllers case will work.

[zhaobin, 2016/09/23 17:18:18]

Acknowledged.

 }
 
 void PresentationServiceImpl::ListenForScreenAvailability(const GURL& url) {
@@ -319,6 +327,10 @@ void PresentationServiceImpl::OnStartSessionSucceeded(
   ListenForConnectionStateChange(session_info);
   pending_start_session_cb_.reset();
   start_session_request_id_ = kInvalidRequestSessionId;
+
+  if (delegate_)
+    delegate_->RegisterOffscreenPresentationController(
+        session_info.presentation_id, this);
 }
 
 void PresentationServiceImpl::OnStartSessionError(
@@ -424,9 +436,11 @@ void PresentationServiceImpl::CloseConnection(
     const GURL& presentation_url,
     const std::string& presentation_id) {
   DVLOG(2) << "CloseConnection " << presentation_id;
-  if (delegate_)
+  if (delegate_) {
     delegate_->CloseConnection(render_process_id_, render_frame_id_,
                                presentation_id);
+    delegate_->UnregisterOffscreenPresentationController(presentation_id, this);
+  }
 }
 
 void PresentationServiceImpl::Terminate(const GURL& presentation_url,
@@ -492,6 +506,23 @@ void PresentationServiceImpl::OnSessionMessages(
       std::move(mojo_messages));
 }
 
+void PresentationServiceImpl::OnReceiverConnectionAvailable(
+    const content::PresentationSessionInfo& session_info) {
+  DVLOG(2) << "PresentationServiceImpl::OnReceiverConnectionAvailable";
+  client_->OnReceiverConnectionAvailable(
+      blink::mojom::PresentationSessionInfo::From(session_info));
+}
+
+void PresentationServiceImpl::AddOffscreenPresentationObserver(
+    OffscreenPresentationClient* client) {
+  offscreen_presentation_observers_.insert(client);
+}
+
+void PresentationServiceImpl::RemoveOffscreenPresentationObserver(
+    OffscreenPresentationClient* client) {
+  offscreen_presentation_observers_.erase(client);
+}
+
 void PresentationServiceImpl::DidNavigateAnyFrame(
     content::RenderFrameHost* render_frame_host,
     const content::LoadCommittedDetails& details,
@@ -537,8 +568,10 @@ void PresentationServiceImpl::WebContentsDestroyed() {
 
 void PresentationServiceImpl::Reset() {
   DVLOG(2) << "PresentationServiceImpl::Reset";
-  if (delegate_)
+  if (delegate_) {
     delegate_->Reset(render_process_id_, render_frame_id_);
+    delegate_->UnregisterOffscreenPresentationReceiver(this);
+  }
 
   default_presentation_urls_.clear();