test_ext_contentscript_errors.js fails with parent controlled navigation
Categories
(WebExtensions :: General, task, P3)
Tracking
(firefox112 fixed)
Tracking | Status | |
---|---|---|
firefox112 | --- | fixed |
People
(Reporter: mccr8, Assigned: mccr8)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
When the pref browser.tabs.documentchannel.parent-controlled is set to true, the test test_ext_contentscript_errors.js fails.
The main failure is that the call to nsIConsoleService.unregisterListener is failing with the helpful error NS_ERROR_FAILURE. If you comment out that line, it instead fails a little later with "TypeError: can't access property "length", errors is undefined", apparently because the value of this.collectedErrors is undefined when it is returned.
This test was added in bug 1410932.
Assignee | ||
Comment 1•2 years ago
|
||
The ContentTask.spawn for the unregisterListener is ending up in a different process than the one that does the registerListener, so it is no wonder things are going awry. I'll have to look over some older issues that have been fixed for parent controlled navigation as that sounds kind of familiar.
Updated•2 years ago
|
Assignee | ||
Comment 2•2 years ago
|
||
The main goal of the changes is to make this test pass when parent-controlled
navigation is enabled.
-
Eagerly send collectedErrors back to the parent process when we get the
expected number of errors. I think this avoids issues that might arise if
parent-controlled navigation cause a process switch to happen earlier. It has
the drawback that if we don't get enough errors we'll hit a time out. We wait
for this reply using a Promise. -
Store collectedErrors on the listener object instead of the message manager.
I don't know if this matters, but hopefully it has a less weird lifetime than
the message manager object being used before. -
Use an early return in the listener as that ends up with a bit less
indentation. -
I added some basic checking for the error messages: two messages in a row
shouldn't have the same error message. An earlier version of the patch was
reporting the same error 7 times, and this checking would catch that. -
Use registerCleanupFunction to unload the extension to reduce the amount of
noise when there's a failure.
Assignee | ||
Comment 3•2 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #1)
The ContentTask.spawn for the unregisterListener is ending up in a different process than the one that does the registerListener, so it is no wonder things are going awry. I'll have to look over some older issues that have been fixed for parent controlled navigation as that sounds kind of familiar.
I looked at this again, and I think I was confused about this. The unregister listener is running in the same process as we did the registerListener, which is also in the same process as TEST_URL_1 and TEST_URL_2. I think the immediate issue of the failure is that the state on the this
object that the ContentTask function gets was cleared somehow, so this.collectedErrors
and this.consoleErrorListener
are undefined. I'm not sure why that is.
Updated•2 years ago
|
Comment 5•2 years ago
|
||
bugherder |
Description
•