I investigated and found the problem.

In void ServiceWorkerHandler::OnWorkerVersionUpdated()
```
    RenderFrameHostImpl* render_frame_host = RenderFrameHostImpl::FromID(
        client.second.process_id, client.second.route_id);
    WebContents* web_contents =
        WebContents::FromRenderFrameHost(render_frame_host);
    // There is a possibility that the frame is already deleted
    // because of the thread hopping.
    if (!web_contents)
      continue;
```
process_id(-1) and route_id(-2) and invalid. They will be set to something meaningful as soon as ServiceWorkerProviderHost::FinalizeInitialization will be called with PlzNavigate.

We talk about this with clamy@. The solution would be to have a WebContentsGetter here, provided by a resource_request_info somewhere else.

https://codereview.chromium.org/2917643002/
fixes this issue.

Without this CL. In ServiceWorkerHandler::OnWorkerVersionUpdated, the data of some ServiceWorkerVersionInfo::ClientInfo will be missing.

falken@ and dgozman@: PlzNavigate will probably be 50%-enabled on beta for M60 and the branch cut happened last week.
Do you know if this bug could have impact on the user if this CL is not present?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a7dc4e822a4cddc14b9929a6f7aa2a85e69252d8

commit a7dc4e822a4cddc14b9929a6f7aa2a85e69252d8
Author: arthursonzogni <arthursonzogni@chromium.org>
Date: Thu Jun 01 17:06:25 2017

PlzNavigate: Fix ServiceWorkerHandler not finding WebContents.

With PlzNavigate, the RenderFrameHost associated with a navigation is
known only at commit time. For this reason, in
ServiceWorkerHandler::OnWorkerVersionUpdated(), client's process_id(-1)
and route_id(-2) are invalid. They will be set to something meaningful
as soon as ServiceWorkerProviderHost::FinalizeInitialization will be
called with PlzNavigate, but it's too late.

The solution with PlzNavigate is to use a WebContentsGetter instead. In
this way, there is no need to know the RenderFrameHost.

BUG= 725818 
TEST=http/tests/inspector/service-workers/service-workers-redundant.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel

Review-Url: https://codereview.chromium.org/2917643002
Cr-Commit-Position: refs/heads/master@{#476331}

[modify] https://crrev.com/a7dc4e822a4cddc14b9929a6f7aa2a85e69252d8/content/browser/devtools/protocol/service_worker_handler.cc
[modify] https://crrev.com/a7dc4e822a4cddc14b9929a6f7aa2a85e69252d8/content/browser/service_worker/service_worker_context_core.cc
[modify] https://crrev.com/a7dc4e822a4cddc14b9929a6f7aa2a85e69252d8/content/browser/service_worker/service_worker_context_observer.h
[modify] https://crrev.com/a7dc4e822a4cddc14b9929a6f7aa2a85e69252d8/content/browser/service_worker/service_worker_context_watcher.cc
[modify] https://crrev.com/a7dc4e822a4cddc14b9929a6f7aa2a85e69252d8/content/browser/service_worker/service_worker_context_watcher.h
[modify] https://crrev.com/a7dc4e822a4cddc14b9929a6f7aa2a85e69252d8/content/browser/service_worker/service_worker_info.cc
[modify] https://crrev.com/a7dc4e822a4cddc14b9929a6f7aa2a85e69252d8/content/browser/service_worker/service_worker_info.h
[modify] https://crrev.com/a7dc4e822a4cddc14b9929a6f7aa2a85e69252d8/content/browser/service_worker/service_worker_version.cc
[modify] https://crrev.com/a7dc4e822a4cddc14b9929a6f7aa2a85e69252d8/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation

It seems to me that the bug will be that in DevTools for service workers, some pages won't show up as clients even though they should be. I'm not sure if the page is just not shown while navigation is still occurring, or if it will continue to be not shown even after navigation.

So users of DevTools could potentially be affected.

Thanks!

I did a manual test. In DevTools > Application > Service Workers, the ServiceWorker's clients (see screenshot) are always empty when this CL is not applied (i.e. it continue to be not shown after navigation).

Knowing this, it might be something we would like to merge. What do you think clamy@, nasko@ and jam@?

Deleted: OnPaste.20170602-102254.png 115 KB

	OnPaste.20170602-102254.png 115 KB View Download

Not showing the clients at all in DevTools would be a fairly bad regression. I'd vote to merge.

Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a6620175c0a4d2990daaef296777ee8ab65d4def

commit a6620175c0a4d2990daaef296777ee8ab65d4def
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Jun 08 17:18:50 2017

PlzNavigate: Fix ServiceWorkerHandler not finding WebContents.

With PlzNavigate, the RenderFrameHost associated with a navigation is
known only at commit time. For this reason, in
ServiceWorkerHandler::OnWorkerVersionUpdated(), client's process_id(-1)
and route_id(-2) are invalid. They will be set to something meaningful
as soon as ServiceWorkerProviderHost::FinalizeInitialization will be
called with PlzNavigate, but it's too late.

The solution with PlzNavigate is to use a WebContentsGetter instead. In
this way, there is no need to know the RenderFrameHost.

BUG= 725818 
TEST=http/tests/inspector/service-workers/service-workers-redundant.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel

Review-Url: https://codereview.chromium.org/2917643002
Cr-Original-Commit-Position: refs/heads/master@{#476331}
Review-Url: https://codereview.chromium.org/2924023004 .
Cr-Commit-Position: refs/branch-heads/3112@{#256}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/a6620175c0a4d2990daaef296777ee8ab65d4def/content/browser/devtools/protocol/service_worker_handler.cc
[modify] https://crrev.com/a6620175c0a4d2990daaef296777ee8ab65d4def/content/browser/service_worker/service_worker_context_core.cc
[modify] https://crrev.com/a6620175c0a4d2990daaef296777ee8ab65d4def/content/browser/service_worker/service_worker_context_observer.h
[modify] https://crrev.com/a6620175c0a4d2990daaef296777ee8ab65d4def/content/browser/service_worker/service_worker_context_watcher.cc
[modify] https://crrev.com/a6620175c0a4d2990daaef296777ee8ab65d4def/content/browser/service_worker/service_worker_context_watcher.h
[modify] https://crrev.com/a6620175c0a4d2990daaef296777ee8ab65d4def/content/browser/service_worker/service_worker_info.cc
[modify] https://crrev.com/a6620175c0a4d2990daaef296777ee8ab65d4def/content/browser/service_worker/service_worker_info.h
[modify] https://crrev.com/a6620175c0a4d2990daaef296777ee8ab65d4def/content/browser/service_worker/service_worker_version.cc
[modify] https://crrev.com/a6620175c0a4d2990daaef296777ee8ab65d4def/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation

@arthursonzogni: Could you please let us know the exact steps and html file to test this issue from TE end

Thanks in Advance.

@rbasuvula:

It can be manually tested.
1) Run chrome with the --enable-browser-side-navigation flag.
2) Open an url with a ServiceWorker. For instance: https://googlechrome.github.io/samples/service-worker/custom-offline-page/
3) Open devtool (Ctrl + Shift + I) > Application tab > Service Workers item.
4) Reload the page.
5) The "clients" line (highlighted in the screenshot) should be present.

Otherwise, a blink layout test can be used:
http://127.0.0.1:8000/inspector/service-workers/service-workers-redundant.html

Command line:
python third_party/WebKit/Tools/Scripts/run-webkit-tests -t out/Debug --additional-driver-flag="--enable-browser-side-navigation" http://127.0.0.1:8000/inspector/service-workers/service-workers-redundant.html

Deleted: service-worker-client-screenshot.png 48.5 KB

	service-worker-client-screenshot.png 48.5 KB View Download