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

content/browser/presentation/presentation_service_impl.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/browser/presentation/presentation_service_impl.h"
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "content/browser/presentation/presentation_type_converters.h"
@@ skipping 282 matching lines @@
 
   // Remove listener for old default presentation URL.
   delegate_->RemoveScreenAvailabilityListener(
       render_frame_host_->GetProcess()->GetID(),
       render_frame_host_->GetRoutingID(),
       old_it->second.get());
   availability_contexts_.erase(old_it);
   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) {
   NOTIMPLEMENTED();
 }
 
 void PresentationServiceImpl::ListenForSessionStateChange(
     const SessionStateCallback& callback) {
   NOTIMPLEMENTED();
 }
@@ skipping 53 matching lines @@
   }
   availability_contexts_.clear();
   for (auto& request_ptr : queued_start_session_requests_) {
     InvokeNewSessionMojoCallbackWithError(request_ptr->callback);
   }
   queued_start_session_requests_.clear();
   for (auto& pending_entry : pending_session_cbs_) {
     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(
     const NewSessionMojoCallback& callback) {
   callback.Run(
         presentation::PresentationSessionInfoPtr(),
         presentation::PresentationError::From(
             PresentationError(PRESENTATION_ERROR_UNKNOWN, "Internal error")));
 }
 
@@ skipping 69 matching lines @@
     : presentation_url(presentation_url),
       presentation_id(presentation_id),
       callback(callback) {
 }
 
 PresentationServiceImpl::StartSessionRequest::~StartSessionRequest() {
 }
 
 }  // namespace content