PlzNavigate: Add parameters to navigation structures for ServiceWorker.

PlzNavigate: Add parameters to navigation structures for ServiceWorker.

This is a spin-off of https://codereview.chromium.org/1294243004/

BUG=440463
Committed: https://crrev.com/69788e1aa9aa74ac643fb40f7a39a05a258f344d
Cr-Commit-Position: refs/heads/master@{#352608}

[Fabrice (no longer in Chrome), 2015-09-15 10:53:59]

fdegans@chromium.org changed reviewers:
+ carlosk@chromium.org, clamy@chromium.org, horo@chromium.org,
kinuko@chromium.org, michaeln@chromium.org, nasko@chromium.org

[Fabrice (no longer in Chrome), 2015-09-15 10:53:59]

nasko@, clamy@: PTAL
others: mostly FYI

This is another spin-off of https://codereview.chromium.org/1294243004/,
"simply" adding the extra parameters for navigation structures. We have one
remaining issue with this patch I think.
clamy@ wanted to set the browser-initiated request context type to
REQUEST_CONTEXT_TYPE_HYPERLINK while nasko@ wanted it set to
REQUEST_CONTEXT_TYPE_LOCATION.

[clamy, 2015-09-15 16:42:31]

Thanks! One comment, see below. For the request context type, I think we should
check what the ResourceDispatcherHost currently receives on a browser-initiated
navigation and use that.

https://codereview.chromium.org/1315023004/diff/50001/content/browser/service...
File content/browser/service_worker/service_worker_provider_host.cc (right):

https://codereview.chromium.org/1315023004/diff/50001/content/browser/service...
content/browser/service_worker/service_worker_provider_host.cc:69: static
base::StaticAtomicSequenceNumber sequence;
The usage over the codebase is to declare a global g_sequence_number in an
anonymous namespace instead of using a static variable in a function.

[Fabrice (no longer in Chrome), 2015-10-06 12:08:09]

I re-extracted the navigation params changed from the latest version of the
patch.
PTAL.

https://codereview.chromium.org/1315023004/diff/50001/content/browser/service...
File content/browser/service_worker/service_worker_provider_host.cc (right):

https://codereview.chromium.org/1315023004/diff/50001/content/browser/service...
content/browser/service_worker/service_worker_provider_host.cc:69: static
base::StaticAtomicSequenceNumber sequence;
On 2015/09/15 16:42:31, clamy wrote:
> The usage over the codebase is to declare a global g_sequence_number in an
> anonymous namespace instead of using a static variable in a function.

Acknowledged.

[Fabrice (no longer in Chrome), 2015-10-06 12:09:58]

I was going through the comments on the other patch and realized I miss this.

https://codereview.chromium.org/1315023004/diff/70001/content/browser/frame_h...
File content/browser/frame_host/navigation_request.cc (right):

https://codereview.chromium.org/1315023004/diff/70001/content/browser/frame_h...
content/browser/frame_host/navigation_request.cc:95:
REQUEST_CONTEXT_TYPE_HYPERLINK),
nasko@ had a comment in the other about whether we should use
REQUEST_CONTEXT_TYPE_HYPERLINK or REQUEST_CONTEXT_TYPE_LOCATION for
browser-initiated navigations.
I'm going to take a look at what we do in current navigation.

[Fabrice (no longer in Chrome), 2015-10-06 13:40:56]

https://codereview.chromium.org/1315023004/diff/70001/content/browser/frame_h...
File content/browser/frame_host/navigation_request.cc (right):

https://codereview.chromium.org/1315023004/diff/70001/content/browser/frame_h...
content/browser/frame_host/navigation_request.cc:95:
REQUEST_CONTEXT_TYPE_HYPERLINK),
On 2015/10/06 12:09:58, Fabrice wrote:
> nasko@ had a comment in the other about whether we should use
> REQUEST_CONTEXT_TYPE_HYPERLINK or REQUEST_CONTEXT_TYPE_LOCATION for
> browser-initiated navigations.
> I'm going to take a look at what we do in current navigation.

We're currently using LOCATION, changing it.

[clamy, 2015-10-06 14:03:30]

Thanks! Lgtm with two nits.

https://codereview.chromium.org/1315023004/diff/90001/content/common/navigati...
File content/common/navigation_params.cc (right):

https://codereview.chromium.org/1315023004/diff/90001/content/common/navigati...
content/common/navigation_params.cc:65:
request_context_type(REQUEST_CONTEXT_TYPE_HYPERLINK) {}
nit: I thought the request context type should be REQUEST_CONTEXT_TYPE_LOCATION?

https://codereview.chromium.org/1315023004/diff/90001/content/test/test_rende...
File content/test/test_render_frame_host.cc (right):

https://codereview.chromium.org/1315023004/diff/90001/content/test/test_rende...
content/test/test_render_frame_host.cc:341: REQUEST_CONTEXT_TYPE_HYPERLINK);
Should we be using REQUEST_CONTEXT_TYPE_LOCATION here?

[Fabrice (no longer in Chrome), 2015-10-06 14:13:13]

Thanks!

https://codereview.chromium.org/1315023004/diff/90001/content/test/test_rende...
File content/test/test_render_frame_host.cc (right):

https://codereview.chromium.org/1315023004/diff/90001/content/test/test_rende...
content/test/test_render_frame_host.cc:341: REQUEST_CONTEXT_TYPE_HYPERLINK);
On 2015/10/06 14:03:29, clamy wrote:
> Should we be using REQUEST_CONTEXT_TYPE_LOCATION here?

This is for a renderer-initiated navigation so I think it makes more sense to
use HYPERLINK here.

[nasko, 2015-10-06 14:15:08]

LGTM

[Fabrice (no longer in Chrome), 2015-10-06 14:17:15]

https://codereview.chromium.org/1315023004/diff/90001/content/common/navigati...
File content/common/navigation_params.cc (right):

https://codereview.chromium.org/1315023004/diff/90001/content/common/navigati...
content/common/navigation_params.cc:65:
request_context_type(REQUEST_CONTEXT_TYPE_HYPERLINK) {}
On 2015/10/06 14:03:29, clamy wrote:
> nit: I thought the request context type should be
REQUEST_CONTEXT_TYPE_LOCATION?

git add is hard.

[Fabrice (no longer in Chrome), 2015-10-06 14:17:29]

The CQ bit was checked by fdegans@chromium.org

[Fabrice (no longer in Chrome), 2015-10-06 14:17:30]

The patchset sent to the CQ was uploaded after l-g-t-m from clamy@chromium.org
Link to the patchset: https://codereview.chromium.org/1315023004/#ps110001
(title: "LOCATION bis")

[commit-bot: I haz the power, 2015-10-06 14:18:12]

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

[commit-bot: I haz the power, 2015-10-06 15:30:24]

Committed patchset #7 (id:110001)

[commit-bot: I haz the power, 2015-10-06 15:31:01]

Patchset 7 (id:??) landed as
https://crrev.com/69788e1aa9aa74ac643fb40f7a39a05a258f344d
Cr-Commit-Position: refs/heads/master@{#352608}