[PresentationAPI] Implementing send() from WebPresentationClient to the PresentationService.

content/renderer/presentation/presentation_dispatcher.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 "content/renderer/presentation/presentation_dispatcher.h"
 
 #include "base/logging.h"
 #include "content/common/presentation/presentation_service.mojom.h"
 #include "content/public/common/service_registry.h"
 #include "content/public/renderer/render_frame.h"
@@ skipping 114 matching lines @@
   // OnSessionCreated() is called. |callback| needs to be alive and also needs
   // to be destroyed so we transfer its ownership to the mojo callback.
   presentation_service_->JoinSession(
       presentationUrl.utf8(),
       presentationId.utf8(),
       base::Bind(&PresentationDispatcher::OnSessionCreated,
           base::Unretained(this),
           base::Owned(callback)));
 }
 
+void PresentationDispatcher::send(
+    const blink::WebString& presentationUrl,
+    const blink::WebString& presentationId,
+    const blink::WebString& message) {
+  message_request_queue_.push(make_linked_ptr(
+      new MessageRequest(
+          presentationUrl.utf8(),
+          presentationId.utf8(),
+          message.utf8())));
+  // Start processing request if only one in the queue.
+  if (message_request_queue_.size() == 1)
+    DoSendStringMessage(presentationUrl.utf8(),
+                        presentationId.utf8(),
+                        message.utf8());
+}
+
+void PresentationDispatcher::send(
+    const blink::WebString& presentationUrl,
+    const blink::WebString& presentationId,
+    const char* data,
+    size_t length) {
+  // TODO(): Handle ArrayBuffer data.

[mark a. foltz, 2015/04/08 23:43:45]

Include login.

[USE s.singapati at gmail.com, 2015/04/10 16:26:05]

Done.

+}
+
+void PresentationDispatcher::DoSendStringMessage(
+    const std::string& presentation_url,
+    const std::string& presentation_id,
+    const std::string& message) {
+  ConnectToPresentationServiceIfNeeded();
+  // Send one string message over mojo channel.
+  presentation_service_->SendStringMessage(
+      presentation_url,
+      presentation_id,
+      message,
+      base::Bind(&PresentationDispatcher::OnSendMessageCallback,
+                 base::Unretained(this)));

[mark a. foltz, 2015/04/08 23:43:45]

The pattern for other callbacks in this file is to transfer ownership to the
Mojo service so they are destroyed when it is.

[USE s.singapati at gmail.com, 2015/04/10 16:26:06]

Done. Now callback is stored in PresentationServiceImpl::send_message_cb_ptr.
callback is run and deleted appropriately.

+}
+
+void PresentationDispatcher::HandleSendMessageRequests() {
+  DCHECK(!message_request_queue_.empty());

[imcheng, 2015/04/09 00:29:57]

Is the following scenario possible?

1. presentation_service_->SendStringMessage is called
2. FrameWillClose() was called (or navigation occurred; see below comment),
which cleared message_request_queue_
3. The callback from (1) is invoked, and blows up here and/or pops the wrong
request from the queue

+  message_request_queue_.pop();

[mark a. foltz, 2015/04/08 23:43:45]

I don't quite follow the logic of the queue.  It seems like if you pop before
the callback is invoked, you won't know whether to send the next message
immediately or queue it on send().

What about:
- Peeking at the head of the queue here and sending
- Dequeuing in the callback
- send() enqueues and calls DoSendStringMessage if the queue was previously
empty

The invariant is that the head of the queue is always being sent.

The code you have may be equivalent, just a bit harder for me to follow?

[USE s.singapati at gmail.com, 2015/04/10 16:26:05]

Now, flow goes like this:
1. send() enques requests and 
2. if queue was empty previously, then call DoSendStringMessage() and wait for
callback.
3. In callback HandleSendMessageRequests(), pop the request and continue
DoSendStringMessage() if queue is not empty.
(Special case is to check for empty queue before pop to handle
FrameWillClose()).

+  if (!message_request_queue_.empty()) {
+    const linked_ptr<MessageRequest>& request = message_request_queue_.front();
+    DoSendStringMessage(request->presentation_url,
+                        request->presentation_id,
+                        request->message);
+  }
+}
+
+void PresentationDispatcher::RemoveAllMessageRequests() {

[imcheng, 2015/04/09 00:29:57]

Do we also need to invalidate all pending send message requests if a frame
navigated away?

[USE s.singapati at gmail.com, 2015/04/10 16:26:05]

Not sure. But, Isn't this enough to invalidate requests in FrameWillClose()? as
it comments "This is the last opportunity to send messages to the host (e.g.,
for clean-up, shutdown, etc.)"

+  while (!message_request_queue_.empty())
+    message_request_queue_.pop();

[mark a. foltz, 2015/04/08 23:43:45]

If there is a pending Mojo callback it needs to be invoked, otherwise Mojo will
disconnect the service from the frame.  Maybe not a big deal since the frame is
closing anyway?

[USE s.singapati at gmail.com, 2015/04/10 16:26:05]

Done. in PresentationServiceImpl, callback send_message_cb_ptr_ is invoked and
deleted in this case.

+}
+
 void PresentationDispatcher::closeSession(
     const blink::WebString& presentationUrl,
     const blink::WebString& presentationId) {
   ConnectToPresentationServiceIfNeeded();
 
   presentation_service_->CloseSession(
       presentationUrl.utf8(),
       presentationId.utf8());
 }
 
 void PresentationDispatcher::DidChangeDefaultPresentation() {
   GURL presentation_url(GetPresentationURLFromFrame(render_frame()));
 
   ConnectToPresentationServiceIfNeeded();
   presentation_service_->SetDefaultPresentationURL(
       presentation_url.spec(), mojo::String());
 }
 
+void PresentationDispatcher::FrameWillClose() {
+  // Remove all pending send message requests.
+  RemoveAllMessageRequests();
+}
+
 void PresentationDispatcher::OnScreenAvailabilityChanged(
     const std::string& presentation_url, bool available) {
   if (!controller_)
     return;
 
   // Reset the callback to get the next event.
   DoUpdateAvailableChangeWatched(presentation_url,
                                  controller_->isAvailableChangeWatched());
 
   controller_->didChangeAvailability(available);
@@ skipping 40 matching lines @@
   presentation_service_->ListenForSessionStateChange(base::Bind(
       &PresentationDispatcher::OnSessionStateChange,
       base::Unretained(this)));
 
   DCHECK(!session_info.is_null());
   controller_->didChangeSessionState(
       new PresentationSessionClient(session_info.Pass()),
       GetWebPresentationSessionStateFromMojo(session_state));
 }
 
+void PresentationDispatcher::OnSendMessageCallback() {

[imcheng, 2015/04/09 00:29:57]

This function looks redundant -- looks like you can base::Bind
HandleSendMessageRequests above instead and get rid of this function.

[USE s.singapati at gmail.com, 2015/04/10 16:26:06]

Done.

+  HandleSendMessageRequests();
+}
+
 void PresentationDispatcher::ConnectToPresentationServiceIfNeeded() {
   if (presentation_service_.get())
     return;
 
   render_frame()->GetServiceRegistry()->ConnectToRemoteService(
       &presentation_service_);
   // TODO(imcheng): Uncomment these once they are implemented on the browser
   // side. (crbug.com/459006)
   /*
   presentation_service_->ListenForDefaultSessionStart(base::Bind(
       &PresentationDispatcher::OnDefaultSessionStarted,
       base::Unretained(this)));
   presentation_service_->ListenForSessionStateChange(base::Bind(
       &PresentationDispatcher::OnSessionStateChange,
       base::Unretained(this)));
   */
 }
 
+PresentationDispatcher::MessageRequest::MessageRequest(
+    const std::string& presentation_url,
+    const std::string& presentation_id,
+    const std::string& message)
+    : presentation_url(presentation_url),
+      presentation_id(presentation_id),
+      message(message) {
+}
+
+PresentationDispatcher::MessageRequest::~MessageRequest() {
+}
+
 }  // namespace content