[Presentation API] Convert presentation.mojom to new wrapper types.

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 <stddef.h>
 #include <stdint.h>
 #include <algorithm>
 #include <string>
@@ skipping 31 matching lines @@
 blink::mojom::SessionMessagePtr ToMojoSessionMessage(
     content::PresentationSessionMessage* input,
     bool pass_ownership) {
   DCHECK(input);
   blink::mojom::SessionMessagePtr output(blink::mojom::SessionMessage::New());
   if (input->is_binary()) {
     // binary data
     DCHECK(input->data);
     output->type = blink::mojom::PresentationMessageType::ARRAY_BUFFER;
     if (pass_ownership) {
-      output->data.Swap(input->data.get());
+      output->data = std::vector<uint8_t>(std::move(*(input->data)));

[dcheng, 2016/07/28 01:48:44]

I think this can just be std::move(*(input->data))

Also, I think I mentioned this earlier, but the conversion should really just be
taking place at a higher level: it's super awkward to plumb through the 'single
observer' bit, and the fact we need to do that means that our layering is
probably a little wrong.

[mark a. foltz, 2016/07/28 18:28:24]

Done.
 
Agree this is not ideal, will discuss with imcheng@ about how this came to be. 
But changing the layering of this API is out of scope for this patch.

     } else {
-      output->data = mojo::Array<uint8_t>::From(*input->data);
+      output->data = std::vector<uint8_t>(*(input->data));

[dcheng, 2016/07/28 01:48:44]

Ditto: I don't think we need to constructor the vector here?

[mark a. foltz, 2016/07/28 18:28:24]

Done.

     }
   } else {
     // string message
     output->type = blink::mojom::PresentationMessageType::TEXT;
     if (pass_ownership) {
-      output->message.Swap(&input->message);
+      output->message = std::move(input->message);
     } else {
       output->message = input->message;
     }
   }
   return output;
 }
 
 std::unique_ptr<PresentationSessionMessage> GetPresentationSessionMessage(
     blink::mojom::SessionMessagePtr input) {
   DCHECK(!input.is_null());
   std::unique_ptr<content::PresentationSessionMessage> output;
   switch (input->type) {
     case blink::mojom::PresentationMessageType::TEXT: {
-      DCHECK(!input->message.is_null());
-      DCHECK(input->data.is_null());
+      // TODO(mfoltz): Should these be CHECKs?

[imcheng, 2016/07/27 20:59:38]

Probably not. We shouldn't make the browser crash due to invalid (untrusted)
input from the renderer.

By the way, it's possible for PresentationServiceImpl to pass in a nullptr
message to PresentationServiceDelegate, which doesn't appear to check for
nullptr resulting in segfault. We shouldn't pass in nullptr message to delegate.

[mark a. foltz, 2016/07/28 18:28:24]

Now converted to return nullptr on invalid input.
Fixed.

+      DCHECK(!input->data);
+      DCHECK(input->message);
       // Return null PresentationSessionMessage if size exceeds.
-      if (input->message.size() > content::kMaxPresentationSessionMessageSize)
+      if (input->message->size() > content::kMaxPresentationSessionMessageSize)

[dcheng, 2016/07/28 01:48:44]

We need to handle unset values here, this code is running in the browser process
and can't trust the data it receives. The same thing applies to all the other
DCHECKs that are being performed: now that we're using base::Optional to
represent these, these derefs become undefined behavior if the optional doesn't
contain a value.

[mark a. foltz, 2016/07/28 18:28:24]

Done.

         return output;
 
       output.reset(
           new PresentationSessionMessage(PresentationMessageType::TEXT));
-      input->message.Swap(&output->message);
+      input->message->swap(output->message);

[dcheng, 2016/07/28 01:48:44]

Nit: std::move() instead here for consistency (same comment elsewhere)

[mark a. foltz, 2016/07/28 18:28:24]

Done.  No more swaps.

       return output;
     }
     case blink::mojom::PresentationMessageType::ARRAY_BUFFER: {
-      DCHECK(!input->data.is_null());
-      DCHECK(input->message.is_null());
-      if (input->data.size() > content::kMaxPresentationSessionMessageSize)
+      // TODO(mfoltz): Should these be CHECKs?
+      DCHECK(input->data);
+      DCHECK(!input->message);
+      if (input->data->size() > content::kMaxPresentationSessionMessageSize)
         return output;
 
       output.reset(new PresentationSessionMessage(
           PresentationMessageType::ARRAY_BUFFER));
       output->data.reset(new std::vector<uint8_t>);
-      input->data.Swap(output->data.get());
+      input->data->swap(*(output->data));
       return output;
     }
     case blink::mojom::PresentationMessageType::BLOB: {
-      DCHECK(!input->data.is_null());
-      DCHECK(input->message.is_null());
-      if (input->data.size() > content::kMaxPresentationSessionMessageSize)
+      // TODO(mfoltz): Should these be CHECKs?
+      DCHECK(input->data);
+      DCHECK(!input->message);
+      if (input->data->size() > content::kMaxPresentationSessionMessageSize)
         return output;
 
       output.reset(
           new PresentationSessionMessage(PresentationMessageType::BLOB));
       output->data.reset(new std::vector<uint8_t>);
-      input->data.Swap(output->data.get());
+      input->data->swap(*(output->data));
       return output;
     }
   }
 
   NOTREACHED() << "Invalid presentation message type " << input->type;
   return output;
 }
 
 void InvokeNewSessionCallbackWithError(
     const PresentationServiceImpl::NewSessionCallback& callback) {
@@ skipping 55 matching lines @@
 }
 
 void PresentationServiceImpl::SetClient(
     blink::mojom::PresentationServiceClientPtr client) {
   DCHECK(!client_.get());
   // TODO(imcheng): Set ErrorHandler to listen for errors.
   client_ = std::move(client);
 }
 
 void PresentationServiceImpl::ListenForScreenAvailability(
-      const mojo::String& url) {
+    const std::string& url) {
   DVLOG(2) << "ListenForScreenAvailability " << url;
   if (!delegate_) {
     client_->OnScreenAvailabilityUpdated(url, false);
     return;
   }
 
-  const std::string& availability_url = url.get();
-  if (screen_availability_listeners_.count(availability_url))
+  if (screen_availability_listeners_.count(url))
     return;
 
   std::unique_ptr<ScreenAvailabilityListenerImpl> listener(
-      new ScreenAvailabilityListenerImpl(availability_url, this));
+      new ScreenAvailabilityListenerImpl(url, this));
   if (delegate_->AddScreenAvailabilityListener(
       render_process_id_,
       render_frame_id_,
       listener.get())) {
-    screen_availability_listeners_[availability_url] = std::move(listener);
+    screen_availability_listeners_[url] = std::move(listener);
   } else {
     DVLOG(1) << "AddScreenAvailabilityListener failed. Ignoring request.";
   }
 }
 
 void PresentationServiceImpl::StopListeningForScreenAvailability(
-    const mojo::String& url) {
+    const std::string& url) {
   DVLOG(2) << "StopListeningForScreenAvailability " << url;
   if (!delegate_)
     return;
 
-  const std::string& availability_url = url.get();
-  auto listener_it = screen_availability_listeners_.find(availability_url);
+  auto listener_it = screen_availability_listeners_.find(url);
   if (listener_it == screen_availability_listeners_.end())
     return;
 
   delegate_->RemoveScreenAvailabilityListener(
       render_process_id_, render_frame_id_, listener_it->second.get());
   screen_availability_listeners_.erase(listener_it);
 }
 
-void PresentationServiceImpl::StartSession(const mojo::String& presentation_url,
+void PresentationServiceImpl::StartSession(const std::string& presentation_url,
                                            const NewSessionCallback& callback) {
   DVLOG(2) << "StartSession";
   if (!delegate_) {
     callback.Run(
         blink::mojom::PresentationSessionInfoPtr(),
         blink::mojom::PresentationError::From(PresentationError(
             PRESENTATION_ERROR_NO_AVAILABLE_SCREENS, "No screens found.")));
     return;
   }
 
   // There is a StartSession request in progress. To avoid queueing up
   // requests, the incoming request is rejected.
   if (start_session_request_id_ != kInvalidRequestSessionId) {
     InvokeNewSessionCallbackWithError(callback);
     return;
   }
 
   start_session_request_id_ = GetNextRequestSessionId();
   pending_start_session_cb_.reset(new NewSessionCallbackWrapper(callback));
   delegate_->StartSession(
       render_process_id_, render_frame_id_, presentation_url,
       base::Bind(&PresentationServiceImpl::OnStartSessionSucceeded,
                  weak_factory_.GetWeakPtr(), start_session_request_id_),
       base::Bind(&PresentationServiceImpl::OnStartSessionError,
                  weak_factory_.GetWeakPtr(), start_session_request_id_));
 }
 
 void PresentationServiceImpl::JoinSession(
-    const mojo::String& presentation_url,
-    const mojo::String& presentation_id,
+    const std::string& presentation_url,
+    const base::Optional<std::string>& presentation_id,
     const NewSessionCallback& callback) {
   DVLOG(2) << "JoinSession";
   if (!delegate_) {
     callback.Run(blink::mojom::PresentationSessionInfoPtr(),
                  blink::mojom::PresentationError::From(PresentationError(
                      PRESENTATION_ERROR_NO_PRESENTATION_FOUND,
                      "Error joining route: No matching route")));
     return;
   }
 
   int request_session_id = RegisterJoinSessionCallback(callback);
   if (request_session_id == kInvalidRequestSessionId) {
     InvokeNewSessionCallbackWithError(callback);
     return;
   }
   delegate_->JoinSession(
-      render_process_id_,
-      render_frame_id_,
-      presentation_url,
-      presentation_id,
+      render_process_id_, render_frame_id_, presentation_url,
+      presentation_id.value_or(std::string()),
       base::Bind(&PresentationServiceImpl::OnJoinSessionSucceeded,
                  weak_factory_.GetWeakPtr(), request_session_id),
       base::Bind(&PresentationServiceImpl::OnJoinSessionError,
                  weak_factory_.GetWeakPtr(), request_session_id));
 }
 
 int PresentationServiceImpl::RegisterJoinSessionCallback(
     const NewSessionCallback& callback) {
   if (pending_join_session_cbs_.size() >= kMaxNumQueuedSessionRequests)
     return kInvalidRequestSessionId;
@@ skipping 69 matching lines @@
   if (it == pending_join_session_cbs_.end())
     return false;
 
   DCHECK(it->second.get());
   it->second->Run(std::move(session), std::move(error));
   pending_join_session_cbs_.erase(it);
   return true;
 }
 
 void PresentationServiceImpl::SetDefaultPresentationURL(
-    const mojo::String& url) {
+    const std::string& url) {
   DVLOG(2) << "SetDefaultPresentationURL";
   if (!delegate_)
     return;
 
-  const std::string& new_default_url = url.get();
-  if (default_presentation_url_ == new_default_url)
+  if (default_presentation_url_ == url)
     return;
 
-  default_presentation_url_ = new_default_url;
+  default_presentation_url_ = url;
   delegate_->SetDefaultPresentationUrl(
-      render_process_id_, render_frame_id_, new_default_url,
+      render_process_id_, render_frame_id_, url,
       base::Bind(&PresentationServiceImpl::OnDefaultPresentationStarted,
                  weak_factory_.GetWeakPtr()));
 }
 
 void PresentationServiceImpl::SendSessionMessage(
     blink::mojom::PresentationSessionInfoPtr session,
     blink::mojom::SessionMessagePtr session_message,
     const SendSessionMessageCallback& callback) {
   DVLOG(2) << "SendSessionMessage";
   DCHECK(!session_message.is_null());
@@ skipping 16 matching lines @@
 void PresentationServiceImpl::OnSendMessageCallback(bool sent) {
   // It is possible that Reset() is invoked before receiving this callback.
   // So, always check send_message_callback_ for non-null.
   if (send_message_callback_) {
     send_message_callback_->Run(sent);
     send_message_callback_.reset();
   }
 }
 
 void PresentationServiceImpl::CloseConnection(
-    const mojo::String& presentation_url,
-    const mojo::String& presentation_id) {
+    const std::string& presentation_url,
+    const std::string& presentation_id) {
   DVLOG(2) << "CloseConnection " << presentation_id;
   if (delegate_)
     delegate_->CloseConnection(render_process_id_, render_frame_id_,
                                presentation_id);
 }
 
-void PresentationServiceImpl::Terminate(const mojo::String& presentation_url,
-                                        const mojo::String& presentation_id) {
+void PresentationServiceImpl::Terminate(const std::string& presentation_url,
+                                        const std::string& presentation_id) {
   DVLOG(2) << "Terminate " << presentation_id;
   if (delegate_)
     delegate_->Terminate(render_process_id_, render_frame_id_, presentation_id);
 }
 
 void PresentationServiceImpl::OnConnectionStateChanged(
     const PresentationSessionInfo& connection,
     const PresentationConnectionStateChangeInfo& info) {
   DCHECK(client_.get());
   if (info.state == PRESENTATION_CONNECTION_STATE_CLOSED) {
@@ skipping 30 matching lines @@
                  weak_factory_.GetWeakPtr(), session_info));
 }
 
 void PresentationServiceImpl::OnSessionMessages(
     const PresentationSessionInfo& session,
     const ScopedVector<PresentationSessionMessage>& messages,
     bool pass_ownership) {
   DCHECK(client_);
 
   DVLOG(2) << "OnSessionMessages";
-  mojo::Array<blink::mojom::SessionMessagePtr> mojoMessages(messages.size());
-  for (size_t i = 0; i < messages.size(); ++i)
-    mojoMessages[i] = ToMojoSessionMessage(messages[i], pass_ownership);
+  std::vector<blink::mojom::SessionMessagePtr> mojoMessages(messages.size());
+  std::transform(messages.begin(), messages.end(), mojoMessages.begin(),
+                 [pass_ownership](PresentationSessionMessage* message) {
+                   return ToMojoSessionMessage(message, pass_ownership);
+                 });
 
   client_->OnSessionMessagesReceived(
       blink::mojom::PresentationSessionInfo::From(session),
       std::move(mojoMessages));
 }
 
 void PresentationServiceImpl::DidNavigateAnyFrame(
     content::RenderFrameHost* render_frame_host,
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
@@ skipping 120 matching lines @@
 
 void PresentationServiceImpl::NewSessionCallbackWrapper::Run(
     blink::mojom::PresentationSessionInfoPtr session,
     blink::mojom::PresentationErrorPtr error) {
   DCHECK(!callback_.is_null());
   callback_.Run(std::move(session), std::move(error));
   callback_.Reset();
 }
 
 }  // namespace content