ABANDONED CL: LoadURLParams: Add |method| + removing LOAD_TYPE_BROWSER_INITIATED_HTTP_POST.

ABANDONED CL: This CL has been abandoned in favor of just renaming LOAD_TYPE_BROWSER_INITIATED_HTTP_POST into LOAD_TYPE_HTTP_POST.  We don't want to introduce an open-ended |std::string method| above //content layer (because we only support GET and POST for page/frame navigations).

========================================================

LoadURLParams: Add |method| + remove LOAD_TYPE_BROWSER_INITIATED_HTTP_POST.

This CL prepares for plumbing arbitrary HTTP method and data (i.e. POST
body) through OpenURL code path by adding a |std::string method| field
to content::NavigationController::LoadURLParams.

This CL also removes LOAD_TYPE_BROWSER_INITIATED_HTTP_POST enum value,
which was reduntant and overlapping with other fields, as explained
below:

- POST part was redundant with 1) the newly added |method| field, and
  also 2) presence of the already existing |browser_initiated_post_data|
  field.

- BROWSER_INITIATED part was reduntant with already existing
  |is_renderer_initiated| field.

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

[Łukasz Anforowicz, 2016-05-27 21:38:51]

Description was changed from

==========
LoadURLParams: Add |method| + removing LOAD_TYPE_BROWSER_INITIATED_HTTP_POST.

This CL prepares for plumbing arbitrary HTTP method and data (i.e. POST
body) through OpenURL code path by adding a |std::string method| field
to content::NavigationController::LoadURLParams.

This CL also removes LOAD_TYPE_BROWSER_INITIATED_HTTP_POST enum value,
which was reduntant and overlapping with other fields, as explained
below:

- POST part was redundant with 1) the newly added |method| field, and
  also 2) presence of the already existing |browser_initiated_post_data|
  field.

- BROWSER_INITIATED part was reduntant with already existing
  |is_renderer_initiated| field.

BUG=344348
==========

to

==========
LoadURLParams: Add |method| + removing LOAD_TYPE_BROWSER_INITIATED_HTTP_POST.

This CL prepares for plumbing arbitrary HTTP method and data (i.e. POST
body) through OpenURL code path by adding a |std::string method| field
to content::NavigationController::LoadURLParams.

This CL also removes LOAD_TYPE_BROWSER_INITIATED_HTTP_POST enum value,
which was reduntant and overlapping with other fields, as explained
below:

- POST part was redundant with 1) the newly added |method| field, and
  also 2) presence of the already existing |browser_initiated_post_data|
  field.

- BROWSER_INITIATED part was reduntant with already existing
  |is_renderer_initiated| field.

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

[Łukasz Anforowicz, 2016-05-27 23:52:08]

Description was changed from

==========
LoadURLParams: Add |method| + removing LOAD_TYPE_BROWSER_INITIATED_HTTP_POST.

This CL prepares for plumbing arbitrary HTTP method and data (i.e. POST
body) through OpenURL code path by adding a |std::string method| field
to content::NavigationController::LoadURLParams.

This CL also removes LOAD_TYPE_BROWSER_INITIATED_HTTP_POST enum value,
which was reduntant and overlapping with other fields, as explained
below:

- POST part was redundant with 1) the newly added |method| field, and
  also 2) presence of the already existing |browser_initiated_post_data|
  field.

- BROWSER_INITIATED part was reduntant with already existing
  |is_renderer_initiated| field.

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

to

==========
LoadURLParams: Add |method| + remove LOAD_TYPE_BROWSER_INITIATED_HTTP_POST.

This CL prepares for plumbing arbitrary HTTP method and data (i.e. POST
body) through OpenURL code path by adding a |std::string method| field
to content::NavigationController::LoadURLParams.

This CL also removes LOAD_TYPE_BROWSER_INITIATED_HTTP_POST enum value,
which was reduntant and overlapping with other fields, as explained
below:

- POST part was redundant with 1) the newly added |method| field, and
  also 2) presence of the already existing |browser_initiated_post_data|
  field.

- BROWSER_INITIATED part was reduntant with already existing
  |is_renderer_initiated| field.

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

[Łukasz Anforowicz, 2016-06-01 00:07:39]

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

[Łukasz Anforowicz, 2016-06-01 00:07:40]

clamy@ + creis@, could you please comment on the overall direction of this CL?

Approach taken by this CL:
- Add |std::string method| field to content::NavigationController::LoadURLParams
- Remove LOAD_TYPE_BROWSER_INITIATED_HTTP_POST enum value

I've recently started to consider (after learning that browsers only support GET
and POST for page/frame navigations [and that other HTTP methods are only
supported in other, corner-case scenarios like XHR]) an alternative approach:
- Simply rename:
    LOAD_TYPE_BROWSER_INITIATED_HTTP_POST -> LOAD_TYPE_HTTP_POST

WDYT?

[clamy, 2016-06-01 13:53:31]

Having a method parameter makes the LoadURlParams closer to the various *Params
we use in content/, which looks better from a code unification standpoint. But
it also opens up the possibility for embedders (in particular other than
Chromium) to start doing browser-initiated navigations with methods that are not
POST or GET. Since it's far from certain that we actually support that in
content/ (I bet we don't), I'm not sure we want to make it look like we do. To
me, just renaming the load type seems like the safest option.

[Łukasz Anforowicz, 2016-06-01 16:28:31]

On 2016/06/01 13:53:31, clamy wrote:
> Having a method parameter makes the LoadURlParams closer to the various
*Params
> we use in content/, which looks better from a code unification standpoint. But
> it also opens up the possibility for embedders (in particular other than
> Chromium) to start doing browser-initiated navigations with methods that are
not
> POST or GET. Since it's far from certain that we actually support that in
> content/ (I bet we don't), I'm not sure we want to make it look like we do. To
> me, just renaming the load type seems like the safest option.

Thanks for the feedback and confirming that my initial approach (introducing an
open ended |std::string method| above //content layer) might not be appropriate
here.  I'll abandon the current CL (and fold the
LOAD_TYPE_BROWSER_INITIATED_HTTP_POST -> LOAD_TYPE_HTTP_POST renaming into
another CL working toward OpenURL/POST support -
https://codereview.chromium.org/2030473002/).

I wonder if we might also want to gravitate toward |bool uses_post| in the
//content layer internals as well.  I also added a comment about //content
public API in
https://codereview.chromium.org/1956383003/diff/420001/content/public/browser....

[Łukasz Anforowicz, 2016-06-01 16:30:00]

Description was changed from

==========
LoadURLParams: Add |method| + remove LOAD_TYPE_BROWSER_INITIATED_HTTP_POST.

This CL prepares for plumbing arbitrary HTTP method and data (i.e. POST
body) through OpenURL code path by adding a |std::string method| field
to content::NavigationController::LoadURLParams.

This CL also removes LOAD_TYPE_BROWSER_INITIATED_HTTP_POST enum value,
which was reduntant and overlapping with other fields, as explained
below:

- POST part was redundant with 1) the newly added |method| field, and
  also 2) presence of the already existing |browser_initiated_post_data|
  field.

- BROWSER_INITIATED part was reduntant with already existing
  |is_renderer_initiated| field.

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

to

==========
ABANDONED CL: This CL has been abandoned in favor of just renaming
LOAD_TYPE_BROWSER_INITIATED_HTTP_POST into LOAD_TYPE_HTTP_POST.  We don't want
to introduce an open-ended |std::string method| above //content layer (because
we only support GET and POST for page/frame navigations).

========================================================

LoadURLParams: Add |method| + remove LOAD_TYPE_BROWSER_INITIATED_HTTP_POST.

This CL prepares for plumbing arbitrary HTTP method and data (i.e. POST
body) through OpenURL code path by adding a |std::string method| field
to content::NavigationController::LoadURLParams.

This CL also removes LOAD_TYPE_BROWSER_INITIATED_HTTP_POST enum value,
which was reduntant and overlapping with other fields, as explained
below:

- POST part was redundant with 1) the newly added |method| field, and
  also 2) presence of the already existing |browser_initiated_post_data|
  field.

- BROWSER_INITIATED part was reduntant with already existing
  |is_renderer_initiated| field.

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