Open Bug 1604543 Opened 5 years ago Updated 2 years ago

ServiceWorkerManager::ReportToAllClients needs to be updated for sw-e10s [syntax errors in script evaluation receive the reported-to-content MSG_SW_INSTALL_ERROR but not the syntax errors]

Categories

(Core :: DOM: Service Workers, defect, P2)

defect

Tracking

()

People

(Reporter: Harald, Unassigned)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

Attached image Chrome vs Firefox (deleted) —

STR:

  • Load a SW with a syntax error

AR: Generic error message TypeError: ServiceWorker script at X for scope Y encountered an error during installation.

ER: Syntax error reported for correct file.

Andrew, would this be on DOM team or JS?

Flags: needinfo?(bugmail)

This is one of those: "devtools could technically do this right now without ServiceWorker backend changes, but the right thing is a ServiceWorker backend change".

I think the syntax errors are getting emitted in the parent process right now as a ConsoleEvent by ConsoleUtils::ReportForServiceWorkerScopeInternal and are emitting stuff to "@mozilla.org/consoleAPI-storage;1" with an inner window ID of "ServiceWorker" and an outer window ID of the ServiceWorker scope.

I see handling for this case in ConsoleAPIListener.isMessageRelevant.

I assume the situation is that this worked under child-intercept because the ServiceWorkerManager emitting the console message, the listening actor, and the window lived in the same process. Now the ServiceWorkerManager and the console message live in the parent and the listening actor and its window live in a content process. Unless the devtools actors are propagating the messages?

It seems like the place where the semantics mismatch is happening is at ServiceWorkerManager::ReportToAllClients. Under parent-intercept, the thing ServiceWorkers should be doing is should be finding all the matching clients and sending the error down to them where they can report it in their process. This seems appropriate given the idea that a user should be able to open devtools after a bad thing has happened and see the bad thing. This would presumably require no devtools changes, although I haven't 100% processed what's going on.

I think we probably also need to add test coverage, as it's likely we don't have it and that's how we missed it as part of the sw-e10s devtools steps.

Unfortunately, given the imminent holidays, this might not make it into 73 unless it gets uplifted early in the cycle.

Flags: needinfo?(bugmail)
Summary: Syntax errors in Service Workers are overriden by generic load error → ServiceWorkerManager::ReportToAllClients needs to be updated for sw-e10s [syntax errors in script evaluation receive the reported-to-content MSG_SW_INSTALL_ERROR but not the syntax errors]

This is one of those: "devtools could technically do this right now without ServiceWorker backend changes, but the right thing is a ServiceWorker backend change".

Perry, given Andrew's comment, do you think this is more trivial do solve now?

Flags: needinfo?(perry)

It seems easier to solve yeah, but I won't be able to get to it unfortunately.

Flags: needinfo?(perry)
Severity: normal normal → S3 S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: