mojo: Add DataURLFetcher for processing data: urls.

mojo: Add DataURLFetcher for processing data: urls.

BUG=531560
Committed: https://crrev.com/14b2f13878ac9f207e553e1d70343750acbe29c0
Cr-Commit-Position: refs/heads/master@{#349437}

[sadrul, 2015-09-17 12:53:58]

sadrul@chromium.org changed reviewers:
+ ben@chromium.org

[sadrul, 2015-09-17 12:53:59]

https://codereview.chromium.org/1356623002/diff/1/mojo/fetcher/data_fetcher.cc
File mojo/fetcher/data_fetcher.cc (right):

https://codereview.chromium.org/1356623002/diff/1/mojo/fetcher/data_fetcher.c...
mojo/fetcher/data_fetcher.cc:55: if (net::DataURL::Parse(url_, &mime_type,
&charset, &data)) {
I am not sure if it's OK to directly parse this here, or if this should talk to
NetworkService instead.

[sadrul, 2015-09-17 13:40:46]

Patchset #3 (id:40001) has been deleted

[Ben Goodger (Google), 2015-09-17 17:10:00]

lgtm

[sadrul, 2015-09-17 17:19:42]

The CQ bit was checked by sadrul@chromium.org

[commit-bot: I haz the power, 2015-09-17 17:20:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1356623002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1356623002/60001

[commit-bot: I haz the power, 2015-09-17 17:26:11]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-09-17 17:26:54]

Patchset 3 (id:??) landed as
https://crrev.com/14b2f13878ac9f207e553e1d70343750acbe29c0
Cr-Commit-Position: refs/heads/master@{#349437}

[sky, 2015-09-17 23:21:50]

sky@chromium.org changed reviewers:
+ sky@chromium.org

[sky, 2015-09-17 23:21:51]

One potential problem is that if this url originates from an html_viewer, then
we want the resulting app to inherit the security origin of the originator. In
other words, if the navigation is in a child frame, then we want the same
html_viewer instance to render the data url. Unfortunately I don't think we have
a good way to express that now:(

https://codereview.chromium.org/1356623002/diff/60001/mojo/fetcher/data_fetch...
File mojo/fetcher/data_fetcher.cc (right):

https://codereview.chromium.org/1356623002/diff/60001/mojo/fetcher/data_fetch...
mojo/fetcher/data_fetcher.cc:21: uint32_t num_bytes =
static_cast<uint32_t>(data.size());
Might it be possible for the string to be bigger than uint32_t?

[sadrul, 2015-09-18 05:08:58]

https://codereview.chromium.org/1356623002/diff/60001/mojo/fetcher/data_fetch...
File mojo/fetcher/data_fetcher.cc (right):

https://codereview.chromium.org/1356623002/diff/60001/mojo/fetcher/data_fetch...
mojo/fetcher/data_fetcher.cc:21: uint32_t num_bytes =
static_cast<uint32_t>(data.size());
On 2015/09/17 23:21:51, sky wrote:
> Might it be possible for the string to be bigger than uint32_t?

I suppose it could be. Perhaps we should check if that is the case, and abort?
For example:

diff --git a/mojo/fetcher/data_fetcher.cc b/mojo/fetcher/data_fetcher.cc
index e785be7..51c25a8 100644
--- a/mojo/fetcher/data_fetcher.cc
+++ b/mojo/fetcher/data_fetcher.cc
@@ -18,6 +18,8 @@ namespace fetcher {
 
 ScopedDataPipeConsumerHandle CreateConsumerHandleForString(
     const std::string& data) {
+  if (data.size() > std::numeric_limits<uint32_t>::max())
+    return ScopedDataPipeConsumerHandle();
   uint32_t num_bytes = static_cast<uint32_t>(data.size());
   MojoCreateDataPipeOptions options;
   options.struct_size = sizeof(MojoCreateDataPipeOptions);

[sky, 2015-09-18 16:02:38]

https://codereview.chromium.org/1356623002/diff/60001/mojo/fetcher/data_fetch...
File mojo/fetcher/data_fetcher.cc (right):

https://codereview.chromium.org/1356623002/diff/60001/mojo/fetcher/data_fetch...
mojo/fetcher/data_fetcher.cc:21: uint32_t num_bytes =
static_cast<uint32_t>(data.size());
On 2015/09/18 05:08:58, sadrul wrote:
> On 2015/09/17 23:21:51, sky wrote:
> > Might it be possible for the string to be bigger than uint32_t?
> 
> I suppose it could be. Perhaps we should check if that is the case, and abort?
> For example:
> 
> diff --git a/mojo/fetcher/data_fetcher.cc b/mojo/fetcher/data_fetcher.cc
> index e785be7..51c25a8 100644
> --- a/mojo/fetcher/data_fetcher.cc
> +++ b/mojo/fetcher/data_fetcher.cc
> @@ -18,6 +18,8 @@ namespace fetcher {
>  
>  ScopedDataPipeConsumerHandle CreateConsumerHandleForString(
>      const std::string& data) {
> +  if (data.size() > std::numeric_limits<uint32_t>::max())
> +    return ScopedDataPipeConsumerHandle();
>    uint32_t num_bytes = static_cast<uint32_t>(data.size());
>    MojoCreateDataPipeOptions options;
>    options.struct_size = sizeof(MojoCreateDataPipeOptions);

Fine by me.