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

third_party/WebKit/public/platform/modules/presentation/presentation.mojom

Index: third_party/WebKit/public/platform/modules/presentation/presentation.mojom
diff --git a/third_party/WebKit/public/platform/modules/presentation/presentation.mojom b/third_party/WebKit/public/platform/modules/presentation/presentation.mojom
index ec0f087bcb93904d144e91c6bc7da79d8b9e011e..d5f6dcdc43facd8857fe331432cd22173323606e 100644
--- a/third_party/WebKit/public/platform/modules/presentation/presentation.mojom
+++ b/third_party/WebKit/public/platform/modules/presentation/presentation.mojom
@@ -52,10 +52,24 @@ struct SessionMessage {
   array<uint8>? data;
 };
 
+interface PresentationConnection {
+   // TODO(zhaobin): migrate SendSessionMessage from PresentationService =>
+   // PresentationConnection.Send(). http://crbug.com/658474
+
+   // Called to set the PresentationConnection that is the destination for
+   // messages sent and the source of messages received by this connection.
+   SetTargetConnection(PresentationConnection connection);
+
+   // Called when a message is sent by the target connection.
+   OnMessage(SessionMessage message);
+};
+
 interface PresentationService {
   // Sets the PresentationServiceClient.
   SetClient(PresentationServiceClient client);
 
+  ///////////// Functions here are for the controller part of the API. /////////
+
   // Called when the frame sets or changes the default presentation URLs.
   // When the default presentation is started on this frame,
   // PresentationServiceClient::OnDefaultSessionStarted will be invoked.
@@ -90,6 +104,14 @@ interface PresentationService {
   JoinSession(array<url.mojom.Url> presentation_urls, string? presentation_id)
       => (PresentationSessionInfo? sessionInfo, PresentationError? error);
 
+  // Called in StartSession's callback function for offscreen presentation only.
+  // It passes in controlling frame's PresentationConnection to
+  // PresentationService.
+  SetPresentationConnection(PresentationSessionInfo sessionInfo,
+                            PresentationConnection connection);

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

As far as I can tell, the goal is a 1:1 relationship of PresentationConnection
objects: we only need to associate a PresentationConnection in the renderer
starting the presentation with the PresentationConnection object in the
offscreen presentation. So my only big suggestion is to try to remove
SetTargetConnection() to simplify the exposed API in Mojo. Right now, this
method can be called at anytime, it's harder to understand how different
PresentationConnections can be related.

Is it possible to pass a PresentationConnection + PresentationConnection& pair
here? In C++, this would translate to a PresentationConnectionPtr and a
PresentationConnectionRequest.

In C++, in the renderer starting the presentation, we would create the proxy
connection:
  auto proxy = base::MakeUnique<PresentationConnectionProxy>();

then we would create PresentationConnectionRequest:
  auto remote_connection_request = proxy->MakeRemoteRequest();

and then we pass both the interface pointer to the created proxy, as well as the
interface request that the remote end will connect:

  service->SetPresentationConnection(info, proxy->Bind(),
      std::move(remote_connection_request));

On the browser side, the |remote_connection_request| is passed to the offscreen
presentation, which binds it to the PresentationConnection impl it creates,
completing the connection.

The MakeRemoteRequest() helper implementation would look like this:
PresentationConnectionRequest
PresentationConnectionProxy::MakeRemoteRequest() {
  DCHECK(!target_connection_)
      << "target_connection_ should only be bound once.";
  return mojo::MakeRequest(&target_connection_);
}

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

Done.

[mark a. foltz, 2017/01/10 23:05:33]

I agree it would be slightly better to have an API where it's clear that
connection passing happens once.  However it comes at the expense of adding a
connection request struct to the API, which is really more general than we need
to implement offscreen presentations.

I think there are other ways to solve this.  For example PresentationSessionInfo
could have an optional PresentationConnectionPtr for the target connection
(which could be set on the callback to StartSession, instead of through a
separate method).  However, I don't think additional bike shedding is going to
help much, and effort is better spent on documenting the current solution.

[dcheng, 2017/01/11 09:22:18]

I'm not familiar enough with how the presentation mojo interfaces are setup.
This suggestion does sound better to me, but I'm OK with this CL as-is as well.
If you don't mind CCing me in followups, I'm a bit curious how this will end up
looking.

+
+  //////////////////////////////////////////////////////////////////////////////
+
   // Called when send() is called by the frame. The true in the
   // result callback notifies that the service is ready for next message.
   // The false in the result callback notifies the renderer to stop sending