[Presentation API] (alternative) 1-UA: send message between controller and receiver page

chrome/browser/media/router/offscreen_presentation_manager.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/media/router/offscreen_presentation_manager.h"
+
+#include <utility>
+
+#include "base/memory/ptr_util.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/web_contents.h"
+
+namespace media_router {
+
+// OffscreenPresentationManager implementation. ////////////////////////////////

[mark a. foltz, 2016/10/06 03:13:54]

I don't think the trailing slashes are needed.

[zhaobin, 2016/10/07 01:07:03]

Done.

+OffscreenPresentationManager::OffscreenPresentationManager() {}
+
+OffscreenPresentationManager::~OffscreenPresentationManager() {}
+
+void OffscreenPresentationManager::RegisterOffscreenPresentationController(
+    const std::string& presentation_id,
+    const std::string& presentation_url,
+    const RenderFrameHostId& render_frame_host_id,
+    content::PresentationConnectionPtr controller) {
+  DVLOG(2) << __FUNCTION__ << " [presentation_id]: " << presentation_id
+           << ", [render_frame_host_id]: " << render_frame_host_id.second;
+
+  auto it = offscreen_presentations_.find(presentation_id);
+  // Create a new presentation.

[mark a. foltz, 2016/10/06 03:13:54]

It's not clear to me why registering an offscreen controller would create an
offscreen presentation.

[zhaobin, 2016/10/07 01:07:03]

RegisterController() and RegisterReceiver() are async. We do not know which one
is invoked first. Need to create an entry in offscreen_presentations_ map if
presentation_id is not found.

+  if (it == offscreen_presentations_.end()) {
+    it = offscreen_presentations_
+             .insert(std::make_pair(presentation_id,
+                                    base::MakeUnique<OffscreenPresentation>(
+                                        presentation_id, presentation_url)))
+             .first;
+  }

[mark a. foltz, 2016/10/06 03:13:54]

This logic is duplicated here and in RegisterOffscreenPresentationReceiver.

Would it make sense to have a separate function
RegisterOffscreenPresentation(id, url) that must be called by the controller's
PSImpl before registering a controller's PresentationConnection (in step 3)?

[zhaobin, 2016/10/07 01:07:03]

Extract a RegisterOffscreenPresentation(id, url) function. RegisterController()
and RegisterReceiver() are async. Order is not guaranteed, so we need to call
RegisterOffscreenPresentation() in both controller and receiver. Would it be
better if we keep the creation logic in OffscreenPresentationManager?

+  it->second->RegisterController(render_frame_host_id, std::move(controller));
+}
+
+void OffscreenPresentationManager::UnregisterOffscreenPresentationController(
+    const std::string& presentation_id,
+    const RenderFrameHostId& render_frame_host_id) {
+  DVLOG(2) << __FUNCTION__ << " [presentation_id]: " << presentation_id
+           << ", [render_frame_host_id]: " << render_frame_host_id.second;
+
+  auto it = offscreen_presentations_.find(presentation_id);
+  if (it == offscreen_presentations_.end())
+    return;
+
+  // Remove presentation if no controller and receiver.
+  it->second->UnregisterController(render_frame_host_id);
+  if (it->second->pending_controllers_.size() == 0 &&

[mark a. foltz, 2016/10/06 03:13:54]

Even if the last controller connection is closed, we don't terminate the
receiver, so I don't think we should erase the OffscreenPresentation entry here.

[zhaobin, 2016/10/07 01:07:03]

We only remove presentation if no there no controller and no receiver (e.g. fail
to start offscreen tab).

+      it->second->receiver_callback_.is_null())
+    offscreen_presentations_.erase(presentation_id);
+}
+
+void OffscreenPresentationManager::RegisterOffscreenPresentationReceiver(
+    const std::string& presentation_id,
+    const std::string& presentation_url,
+    const content::ReceiverConnectionAvailableCallback& receiver_callback) {
+  DVLOG(2) << __FUNCTION__ << " [presentation_id]: " << presentation_id;
+
+  auto it = offscreen_presentations_.find(presentation_id);
+  if (it == offscreen_presentations_.end()) {
+    it = offscreen_presentations_
+             .insert(std::make_pair(presentation_id,
+                                    base::MakeUnique<OffscreenPresentation>(
+                                        presentation_id, presentation_url)))
+             .first;
+  }
+  it->second->RegisterReceiver(receiver_callback);
+}
+
+void OffscreenPresentationManager::UnregisterOffscreenPresentationReceiver(
+    const std::string& presentation_id) {
+  DVLOG(2) << __FUNCTION__ << " [presentation_id]: " << presentation_id;
+  offscreen_presentations_.erase(presentation_id);
+}
+
+// OffscreenPresentation implementation. ///////////////////////////////////////

[mark a. foltz, 2016/10/06 03:13:54]

Omit trailing slashes

[zhaobin, 2016/10/07 01:07:03]

Done.

+OffscreenPresentationManager::OffscreenPresentation::OffscreenPresentation(
+    const std::string& presentation_id,
+    const std::string& presentation_url)
+    : presentation_id_(presentation_id),
+      presentation_url_(presentation_url),
+      receiver_callback_(nullptr) {}
+
+OffscreenPresentationManager::OffscreenPresentation::~OffscreenPresentation() {}
+
+void OffscreenPresentationManager::OffscreenPresentation::RegisterController(
+    const RenderFrameHostId& render_frame_host_id,
+    content::PresentationConnectionPtr controller) {
+  // Connect controller PSImpl and receiver PSImpl.

[mark a. foltz, 2016/10/06 03:13:54]

It may be difficult for those unfamiliar with our code to know what "PSImpl"
means.
I would say "PresentationServiceImpl" or just "PresentationService" (the name of
the Mojo interface that is implemented).

[zhaobin, 2016/10/07 01:07:03]

Outdated comments. Updated.

+  if (receiver_callback_) {
+    receiver_callback_.Run(
+        content::PresentationSessionInfo(presentation_url_, presentation_id_),
+        std::move(controller));
+  } else {
+    pending_controllers_.insert(
+        std::make_pair(render_frame_host_id, std::move(controller)));
+  }
+}
+
+void OffscreenPresentationManager::OffscreenPresentation::UnregisterController(
+    const RenderFrameHostId& render_frame_host_id) {
+  pending_controllers_.erase(render_frame_host_id);
+}
+
+void OffscreenPresentationManager::OffscreenPresentation::RegisterReceiver(
+    const content::ReceiverConnectionAvailableCallback& receiver_callback) {
+  for (auto& controller : pending_controllers_) {

[mark a. foltz, 2016/10/06 03:13:54]

This must be called only once per receiver, correct? If so,
DCHECK(!receiver_callback_)

[zhaobin, 2016/10/07 01:07:03]

Done.

+    receiver_callback.Run(
+        content::PresentationSessionInfo(presentation_url_, presentation_id_),
+        std::move(controller.second));
+  }
+  receiver_callback_ = receiver_callback;
+  pending_controllers_.clear();
+}
+
+}  // namespace media_router