Making ResourceRequestBody part of //content's public API.

Making ResourceRequestBody part of //content's public API.

This CL adds support for passing ResourceRequestBody instances outside
of //content layer.  This will be utilized by a later CL
(https://crrev.com/2004653002) that needs to add ResourceRequestBody to
OpenURLParams (already part of //content's public API) in order to
support passing POST body from a form that targets a remote frame (aka
an OOPIF).

For now, ResourceRequestBody instances can only be created within
//content layer, but this will change in a later CL
(https://crrev.com/2038233002) which will enable //content embedder to
create ResourceRequestBody from a byte array (supporting existing
scenarios, like the public WebView.postUrl API).

BUG=344348
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/de80237a90298f7c94686618c27d88cbf2316028
Cr-Commit-Position: refs/heads/master@{#400182}

[Łukasz Anforowicz, 2016-06-04 00:08:30]

lukasza@chromium.org changed reviewers:
+ creis@chromium.org

[Łukasz Anforowicz, 2016-06-04 00:08:31]

Charlie, could you please take a look?

This is a pretty big, but mostly mechanical CL - it:
- renames content/common/resource_request_body
     into content/common/resource_request_body_impl
- adds content/public/common/resource_request_body

The next CL will start using ResourceRequestBody outside of //content -
https://codereview.chromium.org/2038233002.

[Łukasz Anforowicz, 2016-06-06 20:34:18]

Description was changed from

==========
Making ResourceRequestBody part of //content's public API.

This CL adds support for passing ResourceRequestBody instances outside
of //content layer.  This will be utilized by a later CL that needs to
add ResourceRequestBody to OpenURLParams (already part of //content's
public API) in order to support passing POST body from a form that
targets a remote frame (aka an OOPIF).

For now, ResourceRequestBody instances can only be created within
//content layer, but this will change in a later CL, that will change
|std::vector<char> browser_initiated_post_data| into
|scoped_refptr<ResourceRequestBody> post_data|.

BUG=585284
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Making ResourceRequestBody part of //content's public API.

This CL adds support for passing ResourceRequestBody instances outside
of //content layer.  This will be utilized by a later CL
(https://crrev.com/2004653002) that needs to add ResourceRequestBody to
OpenURLParams (already part of //content's public API) in order to
support passing POST body from a form that targets a remote frame (aka
an OOPIF).

For now, ResourceRequestBody instances can only be created within
//content layer, but this will change in a later CL
(https://crrev.com/2038233002) which will enable //content embedder to
create ResourceRequestBody from a byte array (supporting existing
scenarios, like the public WebView.postUrl API).

BUG=585284
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[Charlie Reis, 2016-06-07 22:21:00]

LGTM, but let's wait to land this until the next one has approvals as well, so
that we can land them at the same time.  That way we aren't adding unused APIs
to content/public.

https://codereview.chromium.org/2038813003/diff/1/content/browser/loader/reso...
File content/browser/loader/resource_dispatcher_host_impl.h (right):

https://codereview.chromium.org/2038813003/diff/1/content/browser/loader/reso...
content/browser/loader/resource_dispatcher_host_impl.h:35: #include
"content/common/resource_request_body_impl.h"
Maybe this can be moved to the .cc file?  Looks like there's no occurrences
below.

https://codereview.chromium.org/2038813003/diff/1/content/common/frame_messag...
File content/common/frame_messages.h (right):

https://codereview.chromium.org/2038813003/diff/1/content/common/frame_messag...
content/common/frame_messages.h:26: #include
"content/common/resource_request_body_impl.h"
Is this stale?

https://codereview.chromium.org/2038813003/diff/1/content/public/common/resou...
File content/public/common/resource_request_body.h (right):

https://codereview.chromium.org/2038813003/diff/1/content/public/common/resou...
content/public/common/resource_request_body.h:18: // ResourceRequestBody
represents body (i.e. upload data) of a HTTP request.
nit: the body
Maybe also mention that this is intentionally opaque and will be used via
OpenURL?

[Łukasz Anforowicz, 2016-06-08 16:51:56]

Thanks for reviewing.

On 2016/06/07 22:21:00, Charlie Reis wrote:
> LGTM, but let's wait to land this until the next one has approvals as well, so
> that we can land them at the same time.  That way we aren't adding unused APIs
> to content/public.

Ack.

https://codereview.chromium.org/2038813003/diff/1/content/browser/loader/reso...
File content/browser/loader/resource_dispatcher_host_impl.h (right):

https://codereview.chromium.org/2038813003/diff/1/content/browser/loader/reso...
content/browser/loader/resource_dispatcher_host_impl.h:35: #include
"content/common/resource_request_body_impl.h"
On 2016/06/07 22:21:01, Charlie Reis wrote:
> Maybe this can be moved to the .cc file?  Looks like there's no occurrences
> below.

Good catch.  Done.

https://codereview.chromium.org/2038813003/diff/1/content/common/frame_messag...
File content/common/frame_messages.h (right):

https://codereview.chromium.org/2038813003/diff/1/content/common/frame_messag...
content/common/frame_messages.h:26: #include
"content/common/resource_request_body_impl.h"
On 2016/06/07 22:21:01, Charlie Reis wrote:
> Is this stale?

Indeed.  I should have removed that in my recent CL [1] which stopped referring
to ResourceRequestBody in IPC messages defined below.  Done.

[1] https://codereview.chromium.org/1999943002/patch/20001/30016 (2 weeks, 4
days ago: Moving HTTP POST body from StartNavigationParams to
CommonNavigationParams)

https://codereview.chromium.org/2038813003/diff/1/content/public/common/resou...
File content/public/common/resource_request_body.h (right):

https://codereview.chromium.org/2038813003/diff/1/content/public/common/resou...
content/public/common/resource_request_body.h:18: // ResourceRequestBody
represents body (i.e. upload data) of a HTTP request.
On 2016/06/07 22:21:01, Charlie Reis wrote:
> nit: the body
> Maybe also mention that this is intentionally opaque and will be used via
> OpenURL?

Done?  You're right - we should explicitly document that opaqueness is
intentional.  OTOH, I am never sure how much comments is too much (i.e. at what
point the benefits of the comment outweight concerns that the comments will at
some point become stale) - let me know if you think I went a little bit
overboard.

https://codereview.chromium.org/2038813003/diff/20001/content/public/common/r...
File content/public/common/resource_request_body.h (right):

https://codereview.chromium.org/2038813003/diff/20001/content/public/common/r...
content/public/common/resource_request_body.h:26: //   //content embedder via
content::OpenURLParams)
This will have to be amended in the next CL to say that //content embedder can
also create body from a byte vector (to support Android's WebView::postUrl API
[1] and to support test code).

[1]
https://developer.android.com/reference/android/webkit/WebView.html#postUrl(j...,
byte[])

[Łukasz Anforowicz, 2016-06-08 16:52:44]

lukasza@chromium.org changed reviewers:
+ nasko@chromium.org

[Łukasz Anforowicz, 2016-06-08 16:52:45]

Nasko, can you look at IPC changes?:
    content/common/frame_messages.h
    content/common/resource_messages.cc
    content/common/resource_messages.h

[nasko, 2016-06-08 16:57:39]

IPC LGTM

[Charlie Reis, 2016-06-08 22:43:20]

Still LGTM with nits.

https://codereview.chromium.org/2038813003/diff/1/content/public/common/resou...
File content/public/common/resource_request_body.h (right):

https://codereview.chromium.org/2038813003/diff/1/content/public/common/resou...
content/public/common/resource_request_body.h:18: // ResourceRequestBody
represents body (i.e. upload data) of a HTTP request.
On 2016/06/08 16:51:56, Łukasz Anforowicz wrote:
> On 2016/06/07 22:21:01, Charlie Reis wrote:
> > nit: the body
> > Maybe also mention that this is intentionally opaque and will be used via
> > OpenURL?
> 
> Done?  You're right - we should explicitly document that opaqueness is
> intentional.  OTOH, I am never sure how much comments is too much (i.e. at
what
> point the benefits of the comment outweight concerns that the comments will at
> some point become stale) - let me know if you think I went a little bit
> overboard.

Looks great-- those are useful to know for understanding why this is in the
public interface.

https://codereview.chromium.org/2038813003/diff/20001/content/public/common/r...
File content/public/common/resource_request_body.h (right):

https://codereview.chromium.org/2038813003/diff/20001/content/public/common/r...
content/public/common/resource_request_body.h:21: // - //content embedders
cannot inspect payload of ResourceRequestBody (i.e.
nit: Embedders
(No need to mention //content here.  Same below.)

nit: the payload

nit: Split the paren phrase out into a separate sentence; no parens needed.

https://codereview.chromium.org/2038813003/diff/20001/content/public/common/r...
content/public/common/resource_request_body.h:23: //   to file ranges, byte
vectors, blob uris, etc.)
nit: Add a period after close paren.  Same below.

https://codereview.chromium.org/2038813003/diff/20001/content/public/common/r...
content/public/common/resource_request_body.h:26: //   //content embedder via
content::OpenURLParams)
On 2016/06/08 16:51:56, Łukasz Anforowicz wrote:
> This will have to be amended in the next CL to say that //content embedder can
> also create body from a byte vector (to support Android's WebView::postUrl API
> [1] and to support test code).
> 
> [1]
>
https://developer.android.com/reference/android/webkit/WebView.html#postUrl(j...,
> byte[])

Acknowledged.

[Łukasz Anforowicz, 2016-06-08 23:20:57]

Description was changed from

==========
Making ResourceRequestBody part of //content's public API.

This CL adds support for passing ResourceRequestBody instances outside
of //content layer.  This will be utilized by a later CL
(https://crrev.com/2004653002) that needs to add ResourceRequestBody to
OpenURLParams (already part of //content's public API) in order to
support passing POST body from a form that targets a remote frame (aka
an OOPIF).

For now, ResourceRequestBody instances can only be created within
//content layer, but this will change in a later CL
(https://crrev.com/2038233002) which will enable //content embedder to
create ResourceRequestBody from a byte array (supporting existing
scenarios, like the public WebView.postUrl API).

BUG=585284
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Making ResourceRequestBody part of //content's public API.

This CL adds support for passing ResourceRequestBody instances outside
of //content layer.  This will be utilized by a later CL
(https://crrev.com/2004653002) that needs to add ResourceRequestBody to
OpenURLParams (already part of //content's public API) in order to
support passing POST body from a form that targets a remote frame (aka
an OOPIF).

For now, ResourceRequestBody instances can only be created within
//content layer, but this will change in a later CL
(https://crrev.com/2038233002) which will enable //content embedder to
create ResourceRequestBody from a byte array (supporting existing
scenarios, like the public WebView.postUrl API).

BUG=344348
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[Łukasz Anforowicz, 2016-06-08 23:22:38]

Thanks.

https://codereview.chromium.org/2038813003/diff/20001/content/public/common/r...
File content/public/common/resource_request_body.h (right):

https://codereview.chromium.org/2038813003/diff/20001/content/public/common/r...
content/public/common/resource_request_body.h:21: // - //content embedders
cannot inspect payload of ResourceRequestBody (i.e.
On 2016/06/08 22:43:20, Charlie Reis wrote:
> nit: Embedders
> (No need to mention //content here.  Same below.)

Done.
> 
> nit: the payload

Done.
> 
> nit: Split the paren phrase out into a separate sentence; no parens needed.

Done.

https://codereview.chromium.org/2038813003/diff/20001/content/public/common/r...
content/public/common/resource_request_body.h:23: //   to file ranges, byte
vectors, blob uris, etc.)
On 2016/06/08 22:43:20, Charlie Reis wrote:
> nit: Add a period after close paren.  Same below.

Done.

[Łukasz Anforowicz, 2016-06-16 15:27:25]

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

[commit-bot: I haz the power, 2016-06-16 15:28:16]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2038813003/80001

[commit-bot: I haz the power, 2016-06-16 17:09:13]

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

[commit-bot: I haz the power, 2016-06-16 17:09:14]

Dry run: This issue passed the CQ dry run.

[Łukasz Anforowicz, 2016-06-16 17:09:33]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2016-06-16 17:09:34]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org,
nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2038813003/#ps80001
(title: "Rebasing...")

[commit-bot: I haz the power, 2016-06-16 17:10:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2038813003/80001

[commit-bot: I haz the power, 2016-06-16 17:17:35]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-16 17:18:01]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-16 17:19:10]

Description was changed from

==========
Making ResourceRequestBody part of //content's public API.

This CL adds support for passing ResourceRequestBody instances outside
of //content layer.  This will be utilized by a later CL
(https://crrev.com/2004653002) that needs to add ResourceRequestBody to
OpenURLParams (already part of //content's public API) in order to
support passing POST body from a form that targets a remote frame (aka
an OOPIF).

For now, ResourceRequestBody instances can only be created within
//content layer, but this will change in a later CL
(https://crrev.com/2038233002) which will enable //content embedder to
create ResourceRequestBody from a byte array (supporting existing
scenarios, like the public WebView.postUrl API).

BUG=344348
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Making ResourceRequestBody part of //content's public API.

This CL adds support for passing ResourceRequestBody instances outside
of //content layer.  This will be utilized by a later CL
(https://crrev.com/2004653002) that needs to add ResourceRequestBody to
OpenURLParams (already part of //content's public API) in order to
support passing POST body from a form that targets a remote frame (aka
an OOPIF).

For now, ResourceRequestBody instances can only be created within
//content layer, but this will change in a later CL
(https://crrev.com/2038233002) which will enable //content embedder to
create ResourceRequestBody from a byte array (supporting existing
scenarios, like the public WebView.postUrl API).

BUG=344348
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/de80237a90298f7c94686618c27d88cbf2316028
Cr-Commit-Position: refs/heads/master@{#400182}
==========

[commit-bot: I haz the power, 2016-06-16 17:19:12]

Patchset 5 (id:??) landed as
https://crrev.com/de80237a90298f7c94686618c27d88cbf2316028
Cr-Commit-Position: refs/heads/master@{#400182}