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

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 17ec835b5f3017cb2bf09d6e292e9a2c3798a26b..46aafdf76f93c47f7a8c8ae947e7be0810855c95 100644
--- a/content/browser/presentation/presentation_service_impl.cc
+++ b/content/browser/presentation/presentation_service_impl.cc
@@ -49,15 +49,15 @@ blink::mojom::SessionMessagePtr ToMojoSessionMessage(
     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;
     }
@@ -71,39 +71,42 @@ std::unique_ptr<PresentationSessionMessage> GetPresentationSessionMessage(
   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;
     }
   }
@@ -179,37 +182,35 @@ void PresentationServiceImpl::SetClient(
 }
 
 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;
 
@@ -218,7 +219,7 @@ void PresentationServiceImpl::StopListeningForScreenAvailability(
   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_) {
@@ -247,8 +248,8 @@ void PresentationServiceImpl::StartSession(const mojo::String& presentation_url,
 }
 
 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_) {
@@ -265,10 +266,8 @@ void PresentationServiceImpl::JoinSession(
     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,
@@ -358,18 +357,17 @@ bool PresentationServiceImpl::RunAndEraseJoinSessionMojoCallback(
 }
 
 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()));
 }
@@ -406,16 +404,16 @@ void PresentationServiceImpl::OnSendMessageCallback(bool sent) {
 }
 
 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);
@@ -466,9 +464,11 @@ void PresentationServiceImpl::OnSessionMessages(
   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),