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

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 9e2fd22e9c8c59971ce8f898b92864e220704a2e..20228c3537979cad9baad461dac9880e680cacee 100644
--- a/content/browser/presentation/presentation_service_impl.cc
+++ b/content/browser/presentation/presentation_service_impl.cc
@@ -300,6 +300,33 @@ void PresentationServiceImpl::SetDefaultPresentationURL(
   DoSetDefaultPresentationUrl(new_default_url, default_presentation_id);
 }
 
+void PresentationServiceImpl::SendStringMessage(
+    const mojo::String& presentation_url,
+    const mojo::String& presentation_id,
+    const mojo::String& message,
+    const SendMessageMojoCallback& callback) {
+  if (!delegate_) {
+    callback.Run();
+    return;
+  }
+  // Save the callback and run it after request completion by delegate_.
+  send_message_cb_ptr_.reset(new SendMessageMojoCallback(callback));

[imcheng, 2015/04/14 21:29:13]

Given that incoming SendStringMessage requests are already throttled by the mojo
flow control, can we simplify this by invoking the callback after calling the
delegate?

The other problem with this approach is that resetting a non-null ptr will drop
the previous callback, resulting in a DCHECK.

[USE s.singapati at gmail.com, 2015/04/15 16:09:08]

Acknowledged. Yes, I also had almost same question here:
https://codereview.chromium.org/1037483003/#msg19

- Should PresentationServiceImpl run callback to PresentationDispatcher
immediately after receiving message / array<>  (as main purpose of sending
array<> here is to minimize mojo calls as I understand)
OR should it wait until all requests are sent and processed by delegate?

Any ideas from other please. I will update the patch based on the comments.

[USE s.singapati at gmail.com, 2015/04/16 11:14:15]

Also, if callback is invoked immediately after calling the delegate, we can
avoid Run & resetting in Reset(). Is this OK ? Could there be any other issues? 

+  delegate_->SendStringMessage(
+      render_frame_host_->GetProcess()->GetID(),
+      render_frame_host_->GetRoutingID(),
+      presentation_url,
+      presentation_id,
+      message,
+      base::Bind(&PresentationServiceImpl::OnSendMessageCallback,
+                 weak_factory_.GetWeakPtr()));
+}
+
+void PresentationServiceImpl::OnSendMessageCallback() {
+  DCHECK(send_message_cb_ptr_);
+  send_message_cb_ptr_->Run();
+  send_message_cb_ptr_.reset();
+}
+
 void PresentationServiceImpl::CloseSession(
     const mojo::String& presentation_url,
     const mojo::String& presentation_id) {
@@ -373,6 +400,13 @@ void PresentationServiceImpl::Reset() {
     InvokeNewSessionMojoCallbackWithError(*pending_entry.second);
   }
   pending_session_cbs_.clear();
+
+  if (send_message_cb_ptr_) {
+    // Running the callback might result in the PresentationDispatcher sending
+    // the next message. So, just reset the callback since the frame is being
+    // closed anyways.
+    send_message_cb_ptr_.reset();

[imcheng, 2015/04/14 21:29:13]

It cannot simply be reset since it will DCHECK per the mojo team. Also, note
that Reset() is also called as a result of navigation not just frame deletion.

[USE s.singapati at gmail.com, 2015/04/15 16:09:08]

Acknowledged. 
- Then it must run the callback for eg. with an error and then reset it. (eg.,
callback OnSendMessageCallback(bool success) in PresentationDispatcher). I will
modify current non-arg callback.
- DidNavigateAnyFrame() does not call Reset() if it is in_page_navigation.

+  }
 }
 
 void PresentationServiceImpl::InvokeNewSessionMojoCallbackWithError(