[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 295 matching lines @@
 
   // Remove listener for old default presentation URL.
   delegate_->RemoveScreenAvailabilityListener(
       render_process_id_,
       render_frame_id_,
       old_it->second.get());
   availability_contexts_.erase(old_it);
   DoSetDefaultPresentationUrl(new_default_url, default_presentation_id);
 }
 
+void PresentationServiceImpl::SendMessage(
+    presentation::SessionMessagePtr message_request,
+    const SendMessageMojoCallback& callback) {
+  DVLOG(2) << "SendMessage";
+  DCHECK(!message_request.is_null());
+  if (!delegate_) {
+    callback.Run(true);

[whywhat, 2015/04/30 15:50:58]

nit: did you want to Run(false) here?

[USE s.singapati at gmail.com, 2015/05/01 10:30:25]

Yes.

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

Done.

+    return;
+  }
+
+  // send_message_cb_ptr_ should be null by now.
+  DCHECK(!send_message_cb_ptr_);

[imcheng (use chromium acct), 2015/04/30 20:11:29]

instead of DCHECKing and/or dropping the old callback, should the new callback
be invoked with false in this scenario?

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

Done.

+  send_message_cb_ptr_.reset(new SendMessageMojoCallback(callback));
+
+  delegate_->SendMessage(
+      render_process_id_,
+      render_frame_id_,
+      new PresentationMessageRequest(
+          message_request->presentation_url,
+          message_request->presentation_id,
+          static_cast<PresentationMessageType>(message_request->type),

[whywhat, 2015/04/30 15:50:58]

nit: it'd be safer to have a static function that converts one type to another

[imcheng (use chromium acct), 2015/04/30 20:11:29]

To add to what Anton said, I think it would be good to have a helper function
that converts a presentation::SessionMessagePtr to a PresentationMessageRequest.
There should be some browser-side validation on the message as well, such as
size check and message/data matches the type.

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

Done for PresentationMessageType conversion.

Need to add helper function for converting SessionMessagePtr to
PresentationMessageRequest. This needs to consider passing
scoped_ptr<PresentationMessageRequest>. I am working on this.

[USE s.singapati at gmail.com, 2015/05/05 14:26:36]

Done.

+          message_request->message,
+          message_request->data),
+      base::Bind(&PresentationServiceImpl::OnSendMessageCallback,
+                 weak_factory_.GetWeakPtr()));
+}
+
+void PresentationServiceImpl::OnSendMessageCallback() {
+  DCHECK(send_message_cb_ptr_);

[imcheng (use chromium acct), 2015/04/30 20:11:29]

Hmm, Reset() could have occurred between SendMessage() and
OnSendMessageCallback(). So this DCHECK isn't safe.

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

Done.

+  send_message_cb_ptr_->Run(true);
+  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 50 matching lines @@
   DVLOG(2) << "PresentationServiceImpl::Reset";
   if (delegate_)
     delegate_->Reset(render_process_id_, render_frame_id_);
 
   default_presentation_url_.clear();
   default_presentation_id_.clear();
   availability_contexts_.clear();
   queued_start_session_requests_.clear();
   FlushNewSessionCallbacks();
   default_session_start_context_.reset();
+  if (send_message_cb_ptr_) {
+    // Run the callback with false, indicating the renderer to stop sending
+    // the requests and invalidate all pending requests.
+    send_message_cb_ptr_->Run(false);
+    send_message_cb_ptr_.reset();
+  }
 }
 
 // static
 void PresentationServiceImpl::InvokeNewSessionMojoCallbackWithError(
     const NewSessionMojoCallback& callback) {
   callback.Run(
         presentation::PresentationSessionInfoPtr(),
         presentation::PresentationError::From(
             PresentationError(PRESENTATION_ERROR_UNKNOWN, "Internal error")));
 }
@@ skipping 129 matching lines @@
 void PresentationServiceImpl::DefaultSessionStartContext::Reset() {
   ScopedVector<DefaultSessionMojoCallback> callbacks;
   callbacks.swap(callbacks_);
   for (const auto& callback : callbacks)
     callback->Run(presentation::PresentationSessionInfoPtr());
   session_.reset();
 }
 
 }  // namespace content