[Presentation API] Plumbing of |onstatechange| from the Mojo service to the public/platform

content/renderer/presentation/presentation_dispatcher.cc

Index: content/renderer/presentation/presentation_dispatcher.cc
diff --git a/content/renderer/presentation/presentation_dispatcher.cc b/content/renderer/presentation/presentation_dispatcher.cc
index eec933b874dba7d3ebdf510d6dc3833d3e3a74e5..e816e6059d9ca30caa2808ca807a2dd7c5224f8a 100644
--- a/content/renderer/presentation/presentation_dispatcher.cc
+++ b/content/renderer/presentation/presentation_dispatcher.cc
@@ -33,6 +33,19 @@ blink::WebPresentationError::ErrorType GetWebPresentationErrorTypeFromMojo(
   }
 }
 
+blink::WebPresentationSessionClient::SessionState
+    GetWebPresentationSessionStateFromMojo(
+        presentation::PresentationSessionState mojoSessionState) {
+  switch (mojoSessionState) {
+    case presentation::PRESENTATION_SESSION_STATE_CONNECTED:
+      return blink::WebPresentationSessionClient::SessionStateConnected;
+    case presentation::PRESENTATION_SESSION_STATE_DISCONNECTED:
+      return blink::WebPresentationSessionClient::SessionStateDisconnected;
+    default:

[Peter Beverloo, 2015/03/20 19:13:39]

Are there more than two options in the generated enum?

If there aren't, please remove the default: case so that the compiler verifies
that all states are being handled.

[whywhat, 2015/03/24 19:20:57]

Done.

+      return blink::WebPresentationSessionClient::SessionStateUnknown;
+  }
+}
+
 GURL GetPresentationURLFromFrame(content::RenderFrame* frame) {
   DCHECK(frame);
 
@@ -161,11 +174,8 @@ void PresentationDispatcher::OnDefaultSessionStarted(
       base::Unretained(this)));
 
   DCHECK(!session_info.is_null());
-  PresentationSessionDispatcher* session_dispatcher =
-      new PresentationSessionDispatcher(session_info.Pass());
-  presentation_session_dispatchers_.push_back(session_dispatcher);
   controller_->didStartDefaultSession(
-      new PresentationSessionClient(session_dispatcher));
+      new PresentationSessionClient(session_info.Pass()));
 }
 
 void PresentationDispatcher::OnSessionCreated(
@@ -182,10 +192,16 @@ void PresentationDispatcher::OnSessionCreated(
   }
 
   DCHECK(!session_info.is_null());
-  PresentationSessionDispatcher* session_dispatcher =
-      new PresentationSessionDispatcher(session_info.Pass());
-  presentation_session_dispatchers_.push_back(session_dispatcher);
-  callback->onSuccess(new PresentationSessionClient(session_dispatcher));
+  callback->onSuccess(new PresentationSessionClient(session_info.Pass()));
+}
+
+void PresentationDispatcher::OnSessionStateChange(
+    presentation::PresentationSessionInfoPtr session_info,
+    presentation::PresentationSessionState session_state) {
+  DCHECK(!session_info.is_null());
+  controller_->didChangeSessionState(

[imcheng, 2015/03/20 23:01:46]

Does controller_ hold references to multiple PresentationSessionClients? How
does it handle multiple sessions in the same frame?

[whywhat, 2015/03/24 19:20:58]

Presentation has a set of PresentationSession objects (one per each JS object
returned to the frame). The PresentationSessionClient is just a way to pass the
message from content/ to blink/ and is destroyed as soon as PresentationSession
object is constructed.

+      new PresentationSessionClient(session_info.Pass()),

[Peter Beverloo, 2015/03/20 19:13:39]

It would be nice if you could document in the
WebPresentationController::didChangeSessionState API that it takes ownership of
the PresentationSessionClient object.

[whywhat, 2015/03/24 19:20:57]

It would be nice to read this comment before submitting the Blink change.
Fingers crossed for it to fail to land :D

+      GetWebPresentationSessionStateFromMojo(session_state));
 }
 
 void PresentationDispatcher::ConnectToPresentationServiceIfNeeded() {
@@ -197,6 +213,9 @@ void PresentationDispatcher::ConnectToPresentationServiceIfNeeded() {
   presentation_service_->ListenForDefaultSessionStart(base::Bind(
       &PresentationDispatcher::OnDefaultSessionStarted,
       base::Unretained(this)));
+  presentation_service_->ListenForSessionStateChange(base::Bind(
+      &PresentationDispatcher::OnSessionStateChange,

[imcheng, 2015/03/20 23:01:46]

How does PresentationServiceImpl know which session to listen to state changes
for?

[whywhat, 2015/03/24 19:20:58]

PresentationServiceImpl should call the callback for any state change for any
session open from the frame. I need to call ListenForSessionStateChange again in
OnSessionStateChange though.

+      base::Unretained(this)));
 }
 
 }  // namespace content