New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 716609 link

Starred by 8 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 739782
issue 785532
issue 810633



Sign in to add a comment

Show ServiceWorker tasks in TaskManager

Project Member Reported by w...@chromium.org, Apr 28 2017

Issue description

Up to and including Chrome 59.0.3071.25, we do not seem to show ServiceWorker tasks in TaskManager.  We should add TaskProvider for ServiceWorkers!
 

Comment 1 by falken@chromium.org, May 10 2017

NextAction: 2017-07-01
Status: Available (was: Untriaged)
I think this makes sense. At least we should expose running service worker threads.

wez@ do you know a specific owner or team for this, or should service worker team  own it?

Comment 2 by w...@chromium.org, May 17 2017

Re #1: This seems a good "starter project" scoped piece of work, once we have input from ServiceWorker team on what the structures the Browser process uses to track active workers are - then it boils down to implementing a TaskGroup provider for them.

Comment 3 by falken@chromium.org, May 18 2017

Some pointers:

ServiceWorkerVersion keeps track of service workers on the browser process. Those for which ServiceWorkerVersion::running_status() is not STOPPED (STARTING, RUNNING, or STOPPING) are actively running.

ServiceWorkerContextCore holds most state on the browser process. I think you probably want to be a ServiceWorkerContextObserver. Then you can use ServiceWorkerContextObserver::OnRunningStateChanged to see globally when a version starts and stops. That's how chrome://serviceworker-internals works.

Comment 4 by nick@chromium.org, May 31 2017

Cc: cburn@google.com
My intern, cburn@, will try to look at this.

Comment 5 by nick@chromium.org, May 31 2017

Cc: kenjibaheux@chromium.org
 Issue 728384  has been merged into this issue.

Comment 6 by nick@chromium.org, May 31 2017

Labels: DevRel-Facebook
In  bug 728384 , vdjeric@fb.com says this would be useful, making the following points:

 - The process only shows up in chrome://serviceworker-internals and chrome://tracing

 - Fixing this would be helpful with diagnosing ServiceWorker perf problems, e.g. ServiceWorker thread causing 100% cpu usage https://bugs.chromium.org/p/chromium/issues/detail?id=728013

Comment 7 by nick@chromium.org, May 31 2017

Part of the challenge here will be figuring out how to plumb this info from content/, where the serviceworker processes are tracked, to chrome/, where the TaskManager lives.

content/public/browser/service_worker_context.h is already exposed, which gives precedent for exposing its Observer (ServiceWorkerContextObserver) as well.

Comment 8 by nick@chromium.org, May 31 2017

It also looks like chrome://inspect/#service-workers manages to populate the set of active ServiceWorkers.

Another couple questions for UI / serviceworker-knowledgeable folks:

 - If a ServiceWorker row in the task manager is double-clicked, is there any action we ought to trigger? For comparison, today when you double-click on an extension background page, it focuses that extension's entry on chrome://extensions.

 - Ordering-wise, would it be useful to place ServiceWorkers rows just after tabs that are currently loading a page of that origin (like we do for subframe rows)? Is there, conceptually, a set of WebContentses that are keeping the ServiceWorker alive?

 - Is there a favicon associated with a ServiceWorker? Should we consider loading a favicon derived from the serviceworker's url?

 - The URL is the only metadata I see being surfaced on serviceworker-internals. This suggests a row appearance like "ServiceWorker: https://adactio.com/serviceworker.js". If there is a more human-readable name, or title, or other data source we should consider, please point it out.
Replies in order:
 - I think it would be useful to open up devtools on the relevant service worker.
 - A service worker can serve several tabs. Can we always group the tabs as we wish? Do we have sortable columns? Maybe highlighting the relevant tabs while hover the service worker?
 - there is on favicon associated with a SW. Tabs controlled by a SW might have different favicons. It's probably better to reuse the iconography in Devtools (IIRC, a gear).
 - other valuable piece of information to consider surfacing: origin and scope of the service worker.


Hope this helps.
The NextAction date has arrived: 2017-07-01
NextAction: 2017-10-01
nick@, cburn@: Are you still planning to work on this?
Blocking: 739782
Labels: -Pri-2 -M-60 Pri-3
nick@, cburn@: any update? we're considering this as a project for the service worker team.

Comment 14 by cburn@google.com, Jul 18 2017

I am currently working on the task manager having it show service workers and other currently untracked processes. The processes I am working on showing are part of this bug https://bugs.chromium.org/p/chromium/issues/detail?id=739782&desc=6 .
Owner: nick@chromium.org
Status: Started (was: Available)
cburn: Got it, thanks! Setting this to Started then.
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 2 2017

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

commit d154ad87e7a64c56f49cf745c898af1bcfc3eb30
Author: cburn <cburn@google.com>
Date: Wed Aug 02 17:25:35 2017

This creates a task manager provider that tracks the life and death of
RenderProcessHosts. This is useful because it will track processes that
are otherwise not included in the current task manager. An example of
this is the NTP which has a service worker that lives beyond navigating
away from the NTP. With this you can accurately see when the
termination of the RenderProcessHost that is running this service
worker.

This version is requires the commandline flag of "--task-manager-show-extra-renderers" to be used.

BUG=716609

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

[modify] https://crrev.com/d154ad87e7a64c56f49cf745c898af1bcfc3eb30/chrome/browser/BUILD.gn
[modify] https://crrev.com/d154ad87e7a64c56f49cf745c898af1bcfc3eb30/chrome/browser/task_manager/providers/child_process_task.cc
[add] https://crrev.com/d154ad87e7a64c56f49cf745c898af1bcfc3eb30/chrome/browser/task_manager/providers/render_process_host_task_provider.cc
[add] https://crrev.com/d154ad87e7a64c56f49cf745c898af1bcfc3eb30/chrome/browser/task_manager/providers/render_process_host_task_provider.h
[modify] https://crrev.com/d154ad87e7a64c56f49cf745c898af1bcfc3eb30/chrome/browser/task_manager/providers/web_contents/renderer_task.cc
[modify] https://crrev.com/d154ad87e7a64c56f49cf745c898af1bcfc3eb30/chrome/browser/task_manager/sampling/task_manager_impl.cc
[modify] https://crrev.com/d154ad87e7a64c56f49cf745c898af1bcfc3eb30/chrome/common/chrome_switches.cc
[modify] https://crrev.com/d154ad87e7a64c56f49cf745c898af1bcfc3eb30/chrome/common/chrome_switches.h
[modify] https://crrev.com/d154ad87e7a64c56f49cf745c898af1bcfc3eb30/content/public/browser/render_process_host.h

Comment 17 by w...@chromium.org, Aug 3 2017

Re #16: This appears to add an extra Task for every TaskGroup, called "Renderer", in addition to any of the Tasks we already show that are running in the renderer, but still doesn't indicate that a renderer is hosting a ServiceWorker (or other similar resource).

What I had in mind when filing this was to have a Task implementation for ServiceWorkers, so that we can see clearly why a renderer process is still running.

Having TaskManager show child processes that no other TaskProvider provides details fo, as some sort of Unknown Process, similar to what your CL does, but extended to all child processes, makes sense too, but I'd prefer that we only show a "pseudo-task" for those processes if no other TaskProvider claims them.
Comment 17: Yes, that's the intention of the project, IIUC.  r491420 is just the first step.

Comment 19 by nick@chromium.org, Aug 3 2017

Our plan is:
  - Create a task provider that shows a task for every RPH. Make it disabled by default, behind a command line flag.
  - Turn this into a "fallback" task provider that only shows a Task if it isn't reported in a timely fashion via WebContentsTaskProvider. This will cover a ton of cases -- serviceworker, pending RFH, pending delete RFH, "instant process", and who knows what else.
  - Once we're satisfied that the above isn't unnecessarily noisy, remove the flag and have it enabled by default.

Then: 
  - Add a serviceworker-aware task provider (when I scoped it out, this seems like it'll require exposing a fair amount of plumbing to content/public -- if you have a simpler way to do it, please propose). When the ServiceWorker task provider creates a Task, the fallback task is suppressed, as with WebContentsTaskProvider.

Comment 20 by w...@chromium.org, Aug 3 2017

Re #19: That plan SGTM - thanks for the clarification.

Comment 21 by w...@chromium.org, Aug 15 2017

FWIW I've been running Chrome Canary with this flag enabled, and I'm seeing
crashes if I try to run a Chrome App.

Comment 22 by w...@chromium.org, Aug 15 2017

OK, you can see an example crash report at id 86283e993283320b.

Comment 23 by nick@chromium.org, Aug 15 2017

Thanks for the report Wez. Chuck and I debugged it and it looks like the problem is the unconditional call to HideTask() -- he's uploading a fix now.
Project Member

Comment 24 by bugdroid1@chromium.org, Aug 15 2017

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

commit c44f45fa322f6fa2c7d4ff9e82736c9fc2f64459
Author: Charles Burnell <cburn@google.com>
Date: Tue Aug 15 23:45:20 2017

[Task Manager] Fix a crash during HideTask() on a not-shown task.

This adds a guard to HideTask() so that it won't try to hide the same
task twice, which could be seen when adding another primary task that
then tried to hide the secondary task again.

Bug: 716609
Change-Id: I59c9dfee8491a01470470ca428b8a4e402cce735
Reviewed-on: https://chromium-review.googlesource.com/616162
Reviewed-by: Charlie Reis (OOO Aug 17-24) <creis@chromium.org>
Reviewed-by: Nick Carter <nick@chromium.org>
Commit-Queue: Nick Carter <nick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494608}
[modify] https://crrev.com/c44f45fa322f6fa2c7d4ff9e82736c9fc2f64459/chrome/browser/task_manager/providers/fallback_task_provider.cc
[modify] https://crrev.com/c44f45fa322f6fa2c7d4ff9e82736c9fc2f64459/chrome/browser/task_manager/providers/fallback_task_provider_unittest.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Aug 18 2017

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

commit 71cc44db52801a26d60ac1111139878c554f290e
Author: Charles Burnell <cburn@google.com>
Date: Fri Aug 18 21:33:18 2017

Added time smoothing to fallback task provider.

This updates the FallbackTaskProvider so that showing a secondary tasks
waits for a period of time, currently 750 ms, to show a secondary task. 
This is intended to smooth creation of temporary "Renderer:" rows that 
will be hidden very shortly after they are shown. 

Bug: 716609
Change-Id: I14f8efafa47aa376f5aefb5a915b1f23a4bc49c3
Reviewed-on: https://chromium-review.googlesource.com/617243
Reviewed-by: Nick Carter <nick@chromium.org>
Commit-Queue: Nick Carter <nick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495693}
[modify] https://crrev.com/71cc44db52801a26d60ac1111139878c554f290e/chrome/browser/task_manager/providers/fallback_task_provider.cc
[modify] https://crrev.com/71cc44db52801a26d60ac1111139878c554f290e/chrome/browser/task_manager/providers/fallback_task_provider.h
[modify] https://crrev.com/71cc44db52801a26d60ac1111139878c554f290e/chrome/browser/task_manager/providers/fallback_task_provider_unittest.cc

The NextAction date has arrived: 2017-10-01
nick@, cburn@: Thanks for working on this! We're tracking this as a Blink>ServiceWorker bug whose NextAction date has expired. Would you be able to provide a status update?
Blocking: 785532

Comment 29 by horo@chromium.org, Jan 12 2018

Any updates?

Comment 30 by creis@chromium.org, Jan 12 2018

Nick is on paternity leave, cburn@'s internship is over, and no one else here has time to finish up the implementation at the moment.
Cc: -cburn@google.com
NextAction: ----
Status: Assigned (was: Started)
Project Member

Comment 32 by PranavkRobot, Mar 19 2018

Labels: crash-BugIsFixed
Blocking: 810633
Labels: WorkerBacklog
Owner: ----
Owner: minggang...@intel.com
Hi all, I have investigated this requirement for a couple of days, and I wrote a document to summarize the current status and what we want to implement in the future.

You could review the doc at https://drive.google.com/open?id=1zwMyPoRyrJL6E7Ret66cxzbGnJyGy2KNzQ-KMlb5sRE for the TODO section, I think it's better to achieve a basic alignment on it. If you meet the access right problem, I will grant it ASAP. Anybody who is interested or has worked on this issue, you are welcome to raise your suggestion and leave your comment!

Sign in to add a comment