Set the document URL of ServiceWorkerProviderHost when redirected even if skip_service_worker is set

Set the redirected document URL of ServiceWorkerProviderHost even if skip_service_worker is set.

Currently the redirected document URL of ServiceWorkerProviderHost is not
correctly set if skip_service_worker is set. This inconsistency is causing the
renderer kill crashes in ServiceWorkerDispatcherHost::OnGetRegistration() and
OnRegisterServiceWorker().

BUG=630496, 630495
Committed: https://crrev.com/206404cd3be24367de9a34304de74c721c768fa9
Cr-Commit-Position: refs/heads/master@{#437493}

[horo, 2016-12-08 06:40:52]

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

[commit-bot: I haz the power, 2016-12-08 06:41:10]

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, 2016-12-08 07:07:23]

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

[commit-bot: I haz the power, 2016-12-08 07:07:24]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[horo, 2016-12-08 09:10:27]

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

[commit-bot: I haz the power, 2016-12-08 09:10:34]

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, 2016-12-08 10:41:06]

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

[commit-bot: I haz the power, 2016-12-08 10:41:06]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[horo, 2016-12-09 01:53:16]

Description was changed from

==========
track the document URL when redirected

BUG=
==========

to

==========
Set the redirected document URL of ServiceWorkerProviderHost even if
skip_service_worker is set.

Currently the redirected document URL of ServiceWorkerProviderHost is not
correctly if skip_service_worker is set.
This inconsistency is causing the renderer kill crashes in 
ServiceWorkerDispatcherHost:: and OnGetRegistrationOnRegisterServiceWorker.

BUG=630496,630495
==========

[horo, 2016-12-09 01:54:23]

Description was changed from

==========
Set the redirected document URL of ServiceWorkerProviderHost even if
skip_service_worker is set.

Currently the redirected document URL of ServiceWorkerProviderHost is not
correctly if skip_service_worker is set.
This inconsistency is causing the renderer kill crashes in 
ServiceWorkerDispatcherHost:: and OnGetRegistrationOnRegisterServiceWorker.

BUG=630496,630495
==========

to

==========
Set the redirected document URL of ServiceWorkerProviderHost even if
skip_service_worker is set.

Currently the redirected document URL of ServiceWorkerProviderHost is not
correctly if skip_service_worker is set. This inconsistency is causing the
renderer kill crashes in ServiceWorkerDispatcherHost::OnGetRegistration() and
OnRegisterServiceWorker().

BUG=630496,630495
==========

[horo, 2016-12-09 01:55:38]

Description was changed from

==========
Set the redirected document URL of ServiceWorkerProviderHost even if
skip_service_worker is set.

Currently the redirected document URL of ServiceWorkerProviderHost is not
correctly if skip_service_worker is set. This inconsistency is causing the
renderer kill crashes in ServiceWorkerDispatcherHost::OnGetRegistration() and
OnRegisterServiceWorker().

BUG=630496,630495
==========

to

==========
Set the redirected document URL of ServiceWorkerProviderHost even if
skip_service_worker is set.

Currently the redirected document URL of ServiceWorkerProviderHost is not
correctly set if skip_service_worker is set. This inconsistency is causing the
renderer kill crashes in ServiceWorkerDispatcherHost::OnGetRegistration() and
OnRegisterServiceWorker().

BUG=630496,630495
==========

[horo, 2016-12-09 01:56:08]

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

[horo, 2016-12-09 01:56:12]

Patchset #1 (id:1) has been deleted

[horo, 2016-12-09 01:56:15]

Patchset #1 (id:20001) has been deleted

[commit-bot: I haz the power, 2016-12-09 01:56:42]

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

[horo, 2016-12-09 01:57:15]

horo@chromium.org changed reviewers:
+ shimazu@chromium.org

[horo, 2016-12-09 01:57:15]

shimazu@
Could you please review this?

[commit-bot: I haz the power, 2016-12-09 03:50:45]

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

[commit-bot: I haz the power, 2016-12-09 03:50:45]

Dry run: This issue passed the CQ dry run.

[shimazu, 2016-12-09 05:00:46]

Thanks for the fix!
Added several comments.

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

https://codereview.chromium.org/2562523003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_provider_host.cc:42: class
ServiceWorkerURLTrackingRequestHandler
I feel it's better to add a brief comment to explain why we need this class.

https://codereview.chromium.org/2562523003/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_request_handler_unittest.cc
(right):

https://codereview.chromium.org/2562523003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_request_handler_unittest.cc:92:
ServiceWorkerRequestHandler::GetHandler(request)->MaybeCreateJob(
How about calling GetHandler which is defined above?

https://codereview.chromium.org/2562523003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_request_handler_unittest.cc:122:
CreateRequest("fpp://host/scope/doc", "GET");
s%fpp://%ftp://%

https://codereview.chromium.org/2562523003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_request_handler_unittest.cc:129:
InitializeHandlerSimpleTest("http://host/scope/doc", "GET", false,
How about adding a comment like "HTTP should have the handler because the
response is possible to be a redirect to HTTPS."?

[horo, 2016-12-09 05:21:21]

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

https://codereview.chromium.org/2562523003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_provider_host.cc:42: class
ServiceWorkerURLTrackingRequestHandler
On 2016/12/09 05:00:46, shimazu wrote:
> I feel it's better to add a brief comment to explain why we need this class.

Done.

https://codereview.chromium.org/2562523003/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_request_handler_unittest.cc
(right):

https://codereview.chromium.org/2562523003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_request_handler_unittest.cc:92:
ServiceWorkerRequestHandler::GetHandler(request)->MaybeCreateJob(
On 2016/12/09 05:00:46, shimazu wrote:
> How about calling GetHandler which is defined above?

Done.

https://codereview.chromium.org/2562523003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_request_handler_unittest.cc:122:
CreateRequest("fpp://host/scope/doc", "GET");
On 2016/12/09 05:00:46, shimazu wrote:
> s%fpp://%ftp://%

Done.

https://codereview.chromium.org/2562523003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_request_handler_unittest.cc:129:
InitializeHandlerSimpleTest("http://host/scope/doc", "GET", false,
On 2016/12/09 05:00:46, shimazu wrote:
> How about adding a comment like "HTTP should have the handler because the
> response is possible to be a redirect to HTTPS."?

Done.

[shimazu, 2016-12-09 06:43:37]

lgtm

[horo, 2016-12-09 06:55:38]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2016-12-09 06:55:55]

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, 2016-12-09 08:10:38]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481266538731940,
"parent_rev": "e6fad4ecaad668a78d3bf45b35900f0f1e301ff2", "commit_rev":
"14b8a2bc5d3b16c237fcd1235e5472a92fc43621"}

[commit-bot: I haz the power, 2016-12-09 08:11:07]

Description was changed from

==========
Set the redirected document URL of ServiceWorkerProviderHost even if
skip_service_worker is set.

Currently the redirected document URL of ServiceWorkerProviderHost is not
correctly set if skip_service_worker is set. This inconsistency is causing the
renderer kill crashes in ServiceWorkerDispatcherHost::OnGetRegistration() and
OnRegisterServiceWorker().

BUG=630496,630495
==========

to

==========
Set the redirected document URL of ServiceWorkerProviderHost even if
skip_service_worker is set.

Currently the redirected document URL of ServiceWorkerProviderHost is not
correctly set if skip_service_worker is set. This inconsistency is causing the
renderer kill crashes in ServiceWorkerDispatcherHost::OnGetRegistration() and
OnRegisterServiceWorker().

BUG=630496,630495
==========

[commit-bot: I haz the power, 2016-12-09 08:11:08]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-12-09 08:12:56]

Description was changed from

==========
Set the redirected document URL of ServiceWorkerProviderHost even if
skip_service_worker is set.

Currently the redirected document URL of ServiceWorkerProviderHost is not
correctly set if skip_service_worker is set. This inconsistency is causing the
renderer kill crashes in ServiceWorkerDispatcherHost::OnGetRegistration() and
OnRegisterServiceWorker().

BUG=630496,630495
==========

to

==========
Set the redirected document URL of ServiceWorkerProviderHost even if
skip_service_worker is set.

Currently the redirected document URL of ServiceWorkerProviderHost is not
correctly set if skip_service_worker is set. This inconsistency is causing the
renderer kill crashes in ServiceWorkerDispatcherHost::OnGetRegistration() and
OnRegisterServiceWorker().

BUG=630496,630495

Committed: https://crrev.com/206404cd3be24367de9a34304de74c721c768fa9
Cr-Commit-Position: refs/heads/master@{#437493}
==========

[commit-bot: I haz the power, 2016-12-09 08:12:57]

Patchset 2 (id:??) landed as
https://crrev.com/206404cd3be24367de9a34304de74c721c768fa9
Cr-Commit-Position: refs/heads/master@{#437493}