Enable transmitting Fetch Requests over Mojo

Enable transmitting Fetch Requests over Mojo

This CL implements support for sending Fetch API requests over Mojo. The
used type in Blink is WebServiceWorkerRequest, where the browser side
will use ServiceWorkerFetchRequest.

It's a lot of extra code and definitely more duplication. However, this
is something that we'll need in the near future to support Onion Soup of
the other users of these types, too. There also are opportunities to
remove duplication, notably of the enum values, by adopting the Mojo
enumerations as opposed to the ones in WebURLRequest.

The introduced type now also supersedes the native struct definition
included in service_worker_event_dispatcher.mojom.

BUG=692535
Review-Url: https://codereview.chromium.org/2762303002
Cr-Commit-Position: refs/heads/master@{#460359}
Committed: https://chromium.googlesource.com/chromium/src/+/f05540d87c83d707b4e9d7a00f29ebd423b9cfb8

[Peter Beverloo, 2017-03-21 20:06:41]

peter@chromium.org changed reviewers:
+ dcheng@chromium.org, harkness@chromium.org, horo@chromium.org,
yhirano@chromium.org

[Peter Beverloo, 2017-03-21 20:06:42]

+horo, yhirano for fetch
+dcheng for Blink/Mojo/security
+harkness FYI

This patch is not ready for review yet. At this stage I'm looking for feedback
on the approach.

Basically, the Background Fetch API takes a series of Request objects and has to
send those to the browser process, where they'll be received as
WebServiceWorkerRequest objects. This CL explores that flow in Mojo, with the
appropriate type converters on both ends.

Do you think this is reasonable? There's lots of loose ends to finish up
(Referrer typemap, Read()ing the received objects, it in fact still crashes), so
any sort of feedback is welcome.

[yhirano, 2017-03-22 13:08:08]

Description was changed from

==========
Enable transmitting Fetch Requests over Mojo

This CL implements support for sending Fetch API requests over Mojo. The
used type in Blink is WebServiceWorkerRequest, where the browser side
will use ServiceWorkerFetchRequest.

It's a lot of extra code and definitely more duplication. However, this
is something that we'll need in the near future to support Onion Soup of
the other users of these types, too. There also are opportunities to
remove duplication, notably of the enum values, by adopting the Mojo
enumerations as opposed to the ones in WebURLRequest.

The introduced type now also supersedes the native struct definition
included in service_worker_event_dispatcher.mojom.

BUG=692535
==========

to

==========
Enable transmitting Fetch Requests over Mojo

This CL implements support for sending Fetch API requests over Mojo. The
used type in Blink is WebServiceWorkerRequest, where the browser side
will use ServiceWorkerFetchRequest.

It's a lot of extra code and definitely more duplication. However, this
is something that we'll need in the near future to support Onion Soup of
the other users of these types, too. There also are opportunities to
remove duplication, notably of the enum values, by adopting the Mojo
enumerations as opposed to the ones in WebURLRequest.

The introduced type now also supersedes the native struct definition
included in service_worker_event_dispatcher.mojom.

BUG=692535
==========

[yhirano, 2017-03-22 13:10:20]

I think Request is too generic. I know that it corresponds to blink::Request,
but I think a more specific name, FetchAPIRequest for example, would be better.
What do you think?

cc: tyoshino

[falken, 2017-03-22 13:44:22]

On 2017/03/22 13:10:20, yhirano wrote:
> I think Request is too generic. I know that it corresponds to blink::Request,
> but I think a more specific name, FetchAPIRequest for example, would be
better.
> What do you think?
> 
> cc: tyoshino

Long-term, should the Fetch API be using WebServiceWorkerRequest and
ServiceWorkerFetchRequest? Fetch API is in theory an independent concept of
service worker.  It seems we could de-dupe some data structures by reusing
ResourceRequest/FetchRequest/URLRequest. Should ServiceWorker*Request even
exist?

[Peter Beverloo, 2017-03-22 14:36:10]

On 2017/03/22 13:44:22, falken wrote:
> On 2017/03/22 13:10:20, yhirano wrote:
> > I think Request is too generic. I know that it corresponds to
blink::Request,
> > but I think a more specific name, FetchAPIRequest for example, would be
> better.
> > What do you think?
> > 
> > cc: tyoshino
> 
> Long-term, should the Fetch API be using WebServiceWorkerRequest and
> ServiceWorkerFetchRequest? Fetch API is in theory an independent concept of
> service worker.  It seems we could de-dupe some data structures by reusing
> ResourceRequest/FetchRequest/URLRequest. Should ServiceWorker*Request even
> exist?

Ah, that's what you meant - generalize one step further.

I agree with separating from Service Workers, that's why I added the file in a
new `fetch` directory in the Blink API and separated out the struct traits.
However, I also think we can do that iteratively -- many of the settings and
enum values are the same, so this would provide a good base to start such work
from.

[horo, 2017-03-23 10:10:57]

We (yhirano@, falken@ and shimazu@) talked offline, and agreed that:
- The name "Request" is too generic.
- "FetchRequest" is a good name but it conflicts with
loader/fetch/FetchRequest.h.
- But the FetchRequest will be replaced with "FetchParameters". (Is it correct?
> yhirano@)
  https://groups.google.com/a/chromium.org/forum/#!topic/loading-dev/xMQ4WGn870E
- So please use FetchAPIRequest for this mojom struct at this moment.
- And we can rename FetchAPIRequest to FetchRequest in future.

[Peter Beverloo, 2017-03-23 21:02:44]

That is very helpful, thank you! I've renamed the `Request` structure to
`FetchAPIRequest`, and implemented the Read() methods on both sides of the
interface.

There will still be rough edges (and I'm sure trybots will tell me about crashes
and failing tests -- will fix), but this should be ready for an initial review
now.

[Peter Beverloo, 2017-03-23 21:02:59]

The CQ bit was checked by peter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-23 21:04:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-23 21:09:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-23 21:09:16]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[kinuko, 2017-03-24 03:37:24]

On 2017/03/23 21:02:44, Peter Beverloo wrote:
> That is very helpful, thank you! I've renamed the `Request` structure to
> `FetchAPIRequest`, and implemented the Read() methods on both sides of the
> interface.
> 
> There will still be rough edges (and I'm sure trybots will tell me about
crashes
> and failing tests -- will fix), but this should be ready for an initial review
> now.

(drive-by) just to make sure I understand the context, we're also going to
change ServiceWorkerResponse to FetchAPIResponse, is that the case?

[dcheng, 2017-03-24 05:27:48]

https://codereview.chromium.org/2762303002/diff/40001/content/common/service_...
File content/common/service_worker/service_worker_fetch_request_struct_traits.cc
(right):

https://codereview.chromium.org/2762303002/diff/40001/content/common/service_...
content/common/service_worker/service_worker_fetch_request_struct_traits.cc:398:
return std::map<std::string, std::string>();
Is this something we expect to eventually implement? Or is it always supposed to
be empty when we're sending it from (content) to Mojo?

Some background: it's slightly wasteful for getters to create temporaries, since
they're currently called twice for serializing each object.

https://codereview.chromium.org/2762303002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/mojo/FetchAPIRequestStructTraits.cpp
(right):

https://codereview.chromium.org/2762303002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/mojo/FetchAPIRequestStructTraits.cpp:380:
return request.method();
Can these return a const ref to avoid copying?

https://codereview.chromium.org/2762303002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/mojo/FetchAPIRequestStructTraits.cpp:388:
WTF::HashMap<WTF::String, WTF::String> headers;
So this is a little expensive, because we'll call it twice as well =/

The way you're supposed to workaround this limitation is to use
SetUpContext/TearDownContext:
https://www.chromium.org/developers/design-documents/mojo/type-mapping

And then do the expensive conversions one time there.

(I'm going to add this CL as an example to the bug for not calling accessors
twice)

https://codereview.chromium.org/2762303002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/mojo/FetchAPIRequestStructTraits.cpp:409:
return WTF::String();
I guess this can't return a const ref...

https://codereview.chromium.org/2762303002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/mojo/ReferrerStructTraits.h (right):

https://codereview.chromium.org/2762303002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/mojo/ReferrerStructTraits.h:30: return true;
Is this empty because we expect to never deserialize this in BLink?

[Peter Beverloo, 2017-03-24 18:48:43]

Thanks for the comments! This should now be ready for review, please take a
look.

On 2017/03/24 03:37:24, kinuko wrote:
> (drive-by) just to make sure I understand the context, we're also going to
> change ServiceWorkerResponse to FetchAPIResponse, is that the case?

I think the rename makes sense, though we may also want to move it to a new
location in //content then. Since this CL is already very large, and the rename
is mechanical, let's do it as a follow-up?

https://codereview.chromium.org/2762303002/diff/40001/content/common/service_...
File content/common/service_worker/service_worker_fetch_request_struct_traits.cc
(right):

https://codereview.chromium.org/2762303002/diff/40001/content/common/service_...
content/common/service_worker/service_worker_fetch_request_struct_traits.cc:398:
return std::map<std::string, std::string>();
On 2017/03/24 05:27:47, dcheng wrote:
> Is this something we expect to eventually implement? Or is it always supposed
to
> be empty when we're sending it from (content) to Mojo?
> 
> Some background: it's slightly wasteful for getters to create temporaries,
since
> they're currently called twice for serializing each object.

We actually need it as part of this CL, so yes. There's definitely still loose
ends in this CL, I didn't want to dive too deep before getting people to agree
on the idea. (Added now.)

https://codereview.chromium.org/2762303002/diff/40001/content/common/service_...
content/common/service_worker/service_worker_fetch_request_struct_traits.cc:398:
return std::map<std::string, std::string>();
On 2017/03/24 05:27:47, dcheng wrote:
> Is this something we expect to eventually implement? Or is it always supposed
to
> be empty when we're sending it from (content) to Mojo?
> 
> Some background: it's slightly wasteful for getters to create temporaries,
since
> they're currently called twice for serializing each object.

Regarding performance, that's really unfortunate. The `request.headers` is a map
with a custom comperator, I'm not sure if there are feasible optimization
strategies there.

https://codereview.chromium.org/2762303002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/mojo/FetchAPIRequestStructTraits.cpp
(right):

https://codereview.chromium.org/2762303002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/mojo/FetchAPIRequestStructTraits.cpp:380:
return request.method();
On 2017/03/24 05:27:48, dcheng wrote:
> Can these return a const ref to avoid copying?

Yeah, I noticed and dislike this too.

In this case there's two issues -
  1) The WebServiceWorkerRequest class returns new instances in many cases where
it doesn't have to. I've sent https://codereview.chromium.org/2767343004/ to
address this.
  2) Even after they, it returns Web{String,URL} instances, where the Blink
bindings need WTF::String or KURL.

The second issue is not as bad as it may seem, because both are backed by
StringImpl which are reference counted, so this effectively boils down to
ref/deref rather than string copy. Still unfortunate.

https://codereview.chromium.org/2762303002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/mojo/FetchAPIRequestStructTraits.cpp:388:
WTF::HashMap<WTF::String, WTF::String> headers;
On 2017/03/24 05:27:47, dcheng wrote:
> So this is a little expensive, because we'll call it twice as well =/
> 
> The way you're supposed to workaround this limitation is to use
> SetUpContext/TearDownContext:
> https://www.chromium.org/developers/design-documents/mojo/type-mapping
> 
> And then do the expensive conversions one time there.
> 
> (I'm going to add this CL as an example to the bug for not calling accessors
> twice)

Oh that's cool! Done. Do you think it'd be worth doing this for the strings too?

https://codereview.chromium.org/2762303002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/mojo/FetchAPIRequestStructTraits.cpp:409:
return WTF::String();
On 2017/03/24 05:27:47, dcheng wrote:
> I guess this can't return a const ref...

Unfortunately not, because BlobDataHandle returns isolated copies due to its
threading semantics.

https://codereview.chromium.org/2762303002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/mojo/ReferrerStructTraits.h (right):

https://codereview.chromium.org/2762303002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/mojo/ReferrerStructTraits.h:30: return true;
On 2017/03/24 05:27:48, dcheng wrote:
> Is this empty because we expect to never deserialize this in BLink?

No, because it was after 9PM and I needed to get home :). Filled in now!

[Peter Beverloo, 2017-03-24 18:49:17]

The CQ bit was checked by peter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-24 18:49:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-24 18:53:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-24 18:53:57]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Peter Beverloo, 2017-03-24 19:21:28]

The CQ bit was checked by peter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-24 19:22:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-24 20:03:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-24 20:03:25]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[horo, 2017-03-27 05:52:02]

lgtm

On 2017/03/24 18:48:43, Peter Beverloo wrote:
> Thanks for the comments! This should now be ready for review, please take a
> look.
> 
> On 2017/03/24 03:37:24, kinuko wrote:
> > (drive-by) just to make sure I understand the context, we're also going to
> > change ServiceWorkerResponse to FetchAPIResponse, is that the case?
> 
> I think the rename makes sense, though we may also want to move it to a new
> location in //content then. Since this CL is already very large, and the
rename
> is mechanical, let's do it as a follow-up?

+1!

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/modules/fetch/request.mojom (right):

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/fetch/request.mojom:79: ERROR,
This ERROR cause compile error in MSVC.
I think you should add

#ifdef ERROR
#undef ERROR
#endif

in module-shared.h.tmpl.

dcheng@
Is it OK?

[yhirano, 2017-03-27 05:58:20]

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/modules/fetch/request.mojom (right):

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/fetch/request.mojom:1: // Copyright
2017 The Chromium Authors. All rights reserved.
Is it better to rename this file?

[kinuko, 2017-03-27 10:01:02]

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

[kinuko, 2017-03-27 10:01:03]

> > On 2017/03/24 03:37:24, kinuko wrote:
> > > (drive-by) just to make sure I understand the context, we're also going to
> > > change ServiceWorkerResponse to FetchAPIResponse, is that the case?
> > 
> > I think the rename makes sense, though we may also want to move it to a new
> > location in //content then. Since this CL is already very large, and the
> rename
> > is mechanical, let's do it as a follow-up?
> 
> +1!

+1 :)

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/modules/fetch/request.mojom (right):

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/fetch/request.mojom:12: enum
FetchRequestMode {
For the enums that are also used in WebURLRequest can we define it in common
.mojom file (not under modules/fetch) and directly use it in our code base?  We
seem to have some precedence of doing that.  (We can do that in a follow-up
patch / in an incremental way too)

[Peter Beverloo, 2017-03-27 22:22:31]

dcheng - ptal

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/modules/fetch/request.mojom (right):

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/fetch/request.mojom:1: // Copyright
2017 The Chromium Authors. All rights reserved.
I don't mind too much. It defines a fair number of things.
fetch_api_request.mojom?

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/fetch/request.mojom:12: enum
FetchRequestMode {
On 2017/03/27 10:01:03, kinuko wrote:
> For the enums that are also used in WebURLRequest can we define it in common
> .mojom file (not under modules/fetch) and directly use it in our code base? 
We
> seem to have some precedence of doing that.  (We can do that in a follow-up
> patch / in an incremental way too)

Yeah, it'd be great to remove the duplication. The one downside of that is that
each file using the WebURLRequest code would have to include the mojom.h file to
get the correct enum type.

(I'll follow up with a bunch of clean-ups.)

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/fetch/request.mojom:79: ERROR,
On 2017/03/27 05:52:02, horo wrote:
> This ERROR cause compile error in MSVC.
> I think you should add
> 
> #ifdef ERROR
> #undef ERROR
> #endif
> 
> in module-shared.h.tmpl.
> 
> dcheng@
> Is it OK?

Thanks for looking into the error! ERROR_MODE would also work as a work-around.

[dcheng, 2017-03-28 06:52:24]

mojo lgtm

https://codereview.chromium.org/2762303002/diff/80001/content/common/service_...
File content/common/service_worker/service_worker_fetch_request_struct_traits.cc
(right):

https://codereview.chromium.org/2762303002/diff/80001/content/common/service_...
content/common/service_worker/service_worker_fetch_request_struct_traits.cc:8:
#include "content/public/common/referrer_struct_traits.h"
Out of curiosity, why are these includes needed?

https://codereview.chromium.org/2762303002/diff/80001/content/common/service_...
content/common/service_worker/service_worker_fetch_request_struct_traits.cc:428:
return static_cast<ServiceWorkerFetchRequestStructTraitsContext*>(context)
I'm OK with this to unblock things. Would you mind seeing if we can do something
clever with MapTraits in a future CL to make this simpler?

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/mojo/FetchAPIRequestStructTraits.cpp
(right):

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/mojo/FetchAPIRequestStructTraits.cpp:389:
context->headers.insert(pair.key, pair.value);
Ditto here with MapTraits, or possibly typemapping HTTPHeaderMap directly.

[kinuko, 2017-03-28 12:48:27]

lgtm

[yhirano, 2017-03-28 12:52:18]

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/modules/fetch/request.mojom (right):

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/fetch/request.mojom:1: // Copyright
2017 The Chromium Authors. All rights reserved.
On 2017/03/27 22:22:31, Peter Beverloo wrote:
> I don't mind too much. It defines a fair number of things.
> fetch_api_request.mojom?

sgtm

[Peter Beverloo, 2017-03-28 14:07:49]

The CQ bit was checked by peter@chromium.org to run a CQ dry run

[Peter Beverloo, 2017-03-28 14:07:53]

https://codereview.chromium.org/2762303002/diff/80001/content/common/service_...
File content/common/service_worker/service_worker_fetch_request_struct_traits.cc
(right):

https://codereview.chromium.org/2762303002/diff/80001/content/common/service_...
content/common/service_worker/service_worker_fetch_request_struct_traits.cc:8:
#include "content/public/common/referrer_struct_traits.h"
On 2017/03/28 06:52:24, dcheng wrote:
> Out of curiosity, why are these includes needed?

We're calling ReadReferrer() on the DataView on line 442, which needs knowledge
of the struct traits to convert it to the content::Referrer type.

Looking at the generated files, referrer.mojom.h includes the public referrer.h
header, but not the struct traits of the type. Dito w/ GURL.

https://codereview.chromium.org/2762303002/diff/80001/content/common/service_...
content/common/service_worker/service_worker_fetch_request_struct_traits.cc:428:
return static_cast<ServiceWorkerFetchRequestStructTraitsContext*>(context)
On 2017/03/28 06:52:24, dcheng wrote:
> I'm OK with this to unblock things. Would you mind seeing if we can do
something
> clever with MapTraits in a future CL to make this simpler?

There's some more interesting differences in these case, but we may be able to
use iterators. Filed crbug.com/705972

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/mojo/FetchAPIRequestStructTraits.cpp
(right):

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/mojo/FetchAPIRequestStructTraits.cpp:389:
context->headers.insert(pair.key, pair.value);
On 2017/03/28 06:52:24, dcheng wrote:
> Ditto here with MapTraits, or possibly typemapping HTTPHeaderMap directly.

I believe this is also covered by crbug.com/705972

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/modules/fetch/request.mojom (right):

https://codereview.chromium.org/2762303002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/fetch/request.mojom:1: // Copyright
2017 The Chromium Authors. All rights reserved.
On 2017/03/28 12:52:18, yhirano wrote:
> On 2017/03/27 22:22:31, Peter Beverloo wrote:
> > I don't mind too much. It defines a fair number of things.
> > fetch_api_request.mojom?
> 
> sgtm

Done

[commit-bot: I haz the power, 2017-03-28 14:08:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yhirano, 2017-03-28 14:40:45]

lgtm

[commit-bot: I haz the power, 2017-03-28 15:47:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-28 15:47:10]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Peter Beverloo, 2017-03-28 15:48:39]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2017-03-28 15:48:40]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
dcheng@chromium.org, horo@chromium.org
Link to the patchset: https://codereview.chromium.org/2762303002/#ps100001
(title: "Enable transmitting Fetch Requests over Mojo")

[commit-bot: I haz the power, 2017-03-28 15:49:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-28 17:35:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-28 17:35:27]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Peter Beverloo, 2017-03-28 20:47:44]

The CQ bit was checked by peter@chromium.org

[commit-bot: I haz the power, 2017-03-28 20:48:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-28 21:15:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-28 21:15:35]

CQ has no permission to schedule in bucket master.tryserver.chromium.linux

[Peter Beverloo, 2017-03-28 22:04:46]

The CQ bit was checked by peter@chromium.org

[commit-bot: I haz the power, 2017-03-28 22:05:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-29 01:04:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-29 01:04:42]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Peter Beverloo, 2017-03-29 10:40:23]

The CQ bit was checked by peter@chromium.org

[commit-bot: I haz the power, 2017-03-29 10:40:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-29 12:36:33]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1490784023527220,
"parent_rev": "fa6f34f961ffb2d4570a1c58f46b7fca6ddfad8b", "commit_rev":
"f05540d87c83d707b4e9d7a00f29ebd423b9cfb8"}

[commit-bot: I haz the power, 2017-03-29 12:37:21]

Description was changed from

==========
Enable transmitting Fetch Requests over Mojo

This CL implements support for sending Fetch API requests over Mojo. The
used type in Blink is WebServiceWorkerRequest, where the browser side
will use ServiceWorkerFetchRequest.

It's a lot of extra code and definitely more duplication. However, this
is something that we'll need in the near future to support Onion Soup of
the other users of these types, too. There also are opportunities to
remove duplication, notably of the enum values, by adopting the Mojo
enumerations as opposed to the ones in WebURLRequest.

The introduced type now also supersedes the native struct definition
included in service_worker_event_dispatcher.mojom.

BUG=692535
==========

to

==========
Enable transmitting Fetch Requests over Mojo

This CL implements support for sending Fetch API requests over Mojo. The
used type in Blink is WebServiceWorkerRequest, where the browser side
will use ServiceWorkerFetchRequest.

It's a lot of extra code and definitely more duplication. However, this
is something that we'll need in the near future to support Onion Soup of
the other users of these types, too. There also are opportunities to
remove duplication, notably of the enum values, by adopting the Mojo
enumerations as opposed to the ones in WebURLRequest.

The introduced type now also supersedes the native struct definition
included in service_worker_event_dispatcher.mojom.

BUG=692535

Review-Url: https://codereview.chromium.org/2762303002
Cr-Commit-Position: refs/heads/master@{#460359}
Committed:
https://chromium.googlesource.com/chromium/src/+/f05540d87c83d707b4e9d7a00f29...
==========

[commit-bot: I haz the power, 2017-03-29 12:37:25]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/f05540d87c83d707b4e9d7a00f29...