Open Bug 1429805 Opened 7 years ago Updated 2 years ago

display service worker console messages on related windows in different child processes

Categories

(DevTools :: Console, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: bkelly, Unassigned)

References

(Blocks 3 open bugs)

Details

(Whiteboard: SW-CLEANUP)

Today when a ServiceWorker script calls console.log() the devtools code checks windows in the current process to see if they care about. It does this using the window.shouldReportForServiceWorkerScope() call: https://searchfox.org/mozilla-central/rev/88439dc6c5b02e167032374071cdc697a056efa1/devtools/server/actors/webconsole/listeners.js#286 It seems, though, this does not propagate to other child processes to check windows there. To see this, follow these steps: 1. Open https://fetch-event-echo.glitch.me/ in a number of tabs until you have some tabs in the same process and some in different processes. 2. Open the web console in all of the tabs. 3. In one of the tabs that is in the same process as another tab, run this code in the console: `fetch('/?blarg')` 4. The service worker uses console.log() to display a bunch of stuff about the FetchEvent resulting from the fetch() call. 5. Look at each tab to see if they show something like `url: "https://fetch-event-echo.glitch.me/?blarg"` 6. Note that only the tabs in the same process include this console message while the other tabs do not show it. Brian, do you think it would be possible to propagate service worker console messages to all child process to check windows? In the future we may move the service worker to a worker-specific process with no windows in it, so this will be a blocker. Also, we want to make SWM use the same console reporting mechanism from the parent process. So it would need to propagate to child processes from there as well.
Flags: needinfo?(bgrinstead)
baku may know what's going on here given recent work with the console.
Flags: needinfo?(amarchesini)
Andrea, in particular are you sure this is a devtools issue? It seems that dom/console fires a same-process nsIObserverService "console-api-log-event" topic.
Note, it would also be nice to add the principal to the console event for a couple reasons. 1. We could create an API like ClientManager::GetProcessListForPrincipal(nsIPrincipal*) that asynchronously returns a list of processes that contains windows/workers of the given principal. We could then only broadcast messages to the appropriate processes instead of all of them. 2. Right now all tabs in different containers with same scope will all show each other's messages. Getting the principal with its origin attributes would let us fix that. This could be a follow-on, though. Just getting the message to all processes would for service worker logs would be an ok first step.
I think Andrea would know better, and I won't be able to look closer at this until next week, but Comment 2 seems accurate. The only cases where we hit the isMessageRelevant/shouldReportForServiceWorkerScope are either: 1) We call ConsoleAPIStorage.getCachedMessages (if the log happened before the console was opened) 2) We observe the "console-api-log-event" notification (if the log happens while the console is opened) Based on the STR here it seems we should be hitting case (2) but we aren't seeing the notification in the console actor. I just did a quick test and even case (1) doesn't show the messages cross-process (I tested this by skipping step 2 in your STR for one of the tabs in a different process and opening it after triggering the logging in another tab's console).
IIUC this is pretty much the same problem (and solution) as when a SharedWorker runs in a different process with a page as discussed starting at https://bugzilla.mozilla.org/show_bug.cgi?id=1438945#c28 and going through https://bugzilla.mozilla.org/show_bug.cgi?id=1438945#c37. Right now we don't have a way to send messages across from one content process to another, although we do have a way to send messages from content processes up to the parent process (in the case of the Browser Console). It's possible to build something to send messages from one child processes to another but the site isolation work (in particular bug 1441711) will be a better solution because: * we'll be able to connect to and get messages from the SW process directly from the tab's toolbox * the objects in those messages will be inspectable (no more [Object object] from logs coming from a worker)
Flags: needinfo?(bgrinstead)
Flags: needinfo?(amarchesini)
What's the timeline on the SW running in a worker specific process? If we could detect the case when SW messages are going to be missing from the tab's console and send a message to that tab pointing the user to about:debugging#workers where they can debug the SW directly that may be a decent workaround ahead of the devtools site isolation work.
Note, its similar to SharedWorker, but there is a difference. In the SharedWorker case we can usually derive a precise set of window IDs for the windows that are attaching to it. In the ServiceWorker case we have the fuzzier logic defined in: https://searchfox.org/mozilla-central/rev/769222fadff46164f8cc0dc7a0bae5a60dc2f335/dom/base/nsGlobalWindowInner.cpp#2461-2529 In regards to timeline, we are hoping to get service workers running in a separate process in FF61. It won't be a separate worker process, but we have to deal with the separate content process issue still. Our plan is to enable this in nightly only at first even with this stuff broken. Once devtools and other polish issues are resolved we can let it ride the trains.
Product: Firefox → DevTools
Whiteboard: SW-CLEANUP
Priority: -- → P3
Hello. I was curious what the timeline for this fix is? Also to clarify, the fix is to have a ServiceWorker run in its unique, single process correct? My current issue with this bug is that Chrome handles serviceworkers by running them in a single process (no matter how many tabs), where as firefox will run a new serviceworker process for each tab. Thanks!
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.