Closed Bug 1422823 Opened 2 years ago Closed 2 years ago

figure out why test_ext_contentScripts_register.js fails after fixing long shutdown delay in bug 1420594

Categories

(WebExtensions :: Android, enhancement, P3)

enhancement

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1477688

People

(Reporter: bkelly, Assigned: rpl)

References

Details

Attachments

(1 file)

I fixed a shutdown delay in bug 1420594.  This seems to have caused test_ext_contentScripts_register.js to start failing.  I'm disabling for now since we really need to correct the shutdown delay to avoid more leaks, errors being added.
AFAICT this test was added while the shutdown delay was in place and probably just doesn't handle shutdown correctly.
Blocks: 1332273
Luca, can you investigate why this fails when shutdown is not delayed?  I see some other tests in this directory are disabled on android debug.  Is this just similar to those reasons?
Flags: needinfo?(lgreco)
(In reply to Ben Kelly [:bkelly] from comment #2)
> Luca, can you investigate why this fails when shutdown is not delayed?

Yes, absolutely, I'm looking into it.
(and I think that it is likely to be related to the "unregistering of programmatically registered content script when the extension is shutting down", which we could and we should avoid if the extension is shutting down because the entire browser
is exiting).

> I see some other tests in this directory are disabled on android debug.  Is
> this just similar to those reasons?

The reasons behind the WebExtensions tests that are currently disabled only on android debug are usually related to the fact that the arm emulator is pretty slow and it is even slower when we are running on a debug build, and so these tests have an higher rate of failures on android debug (on the contrary the tests that are disabled on all the android targets are usually related to unimplemented APIs on Android, and in lower number of cases because of a testing strategy that works on Desktop but not on Android).
Assignee: nobody → lgreco
Status: NEW → ASSIGNED
Flags: needinfo?(lgreco)
Follows a try push related to the attached patch:

- https://treeherder.mozilla.org/#/jobs?repo=try&revision=afdc165dabbde784ae36996824e82923f5fe7b8d

The attach patch is re-enabling the test file and skip a single test case, test_contentscripts_register_js (https://searchfox.org/mozilla-central/rev/f5f1c3f294f89cfd242c3af9eb2c40d19d5e04e7/toolkit/components/extensions/test/xpcshell/test_ext_contentScripts_register.js#248), as a proof that even if the crash is happening in the test case that follows (test_contentscripts_register_all_options, https://searchfox.org/mozilla-central/rev/f5f1c3f294f89cfd242c3af9eb2c40d19d5e04e7/toolkit/components/extensions/test/xpcshell/test_ext_contentScripts_register.js#414), it is actually triggered by the test that runs before it.

It is also worth to mention that a crash with the same error messages is also happening on Android debug on another of the WebExtensions xpcshell tests, test_ext_redirects.js (filed as Bug 1423475).

The following error messages and assertion errors are in common between the crashes happened on test_ext_contentScripts_register.js and test_ext_redirects.js:

JavaScript error: jar:jar:file:///data/local/xpcb/target.apk!/assets/omni.ja!/components/nsAsyncShutdown.js, line 119: Error: We have already registered a distinct blocker with the same name: ClientManagerService: start destroying IPC actors early
WARNING: '!mClientSource', file /builds/worker/workspace/build/src/dom/base/nsGlobalWindowInner.cpp, line 1804
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /builds/worker/workspace/build/src/dom/base/nsGlobalWindowInner.cpp, line 1856
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /builds/worker/workspace/build/src/dom/base/nsGlobalWindowOuter.cpp, line 2015
WARNING: ContentViewer Initialization failed: file /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp, line 9702
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp, line 7517
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp, line 8399
WARNING: NS_ENSURE_TRUE(mContentViewer) failed: file /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp, line 8255
WARNING: NS_ENSURE_SUCCESS(EnsureContentViewer(), nullptr) failed with result 0x8000FFFF: file /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp, line 4759
Segmentation fault

Follows links to the failures logs on treeherder:

- test_ext_contentScripts_register.js xpcshell-test crash: https://treeherder.mozilla.org/logviewer.html#?job_id=149165468&repo=mozilla-inbound&lineNumber=1875
- test_ext_redirects.js xpcshell-test crash: https://treeherder.mozilla.org/logviewer.html#?job_id=150077116&repo=mozilla-inbound&lineNumber=1775

Based on a brief chat over IRC with bkelly, it seems that the logged errors can be fixed by the patch attached to Bug 1423913 Comment 12.

I'm going to try to trigger the issue again in a push to try on top of the changes that https://bugzilla.mozilla.org/show_bug.cgi?id=1423913#c12 is going to introduce.
See Also: → 1423475, 1423913
Priority: -- → P3
The push to try with the patches from Bug 1423913:

- https://treeherder.mozilla.org/#/jobs?repo=try&revision=45cc3a1d7424161fa2054b6d357f64e96b0d2637

it doesn't trigger the xpcshell-test crash anymore on this test file.

I'm adding Bug 1423913 as a blocker for this bug.
Depends on: 1423913
See Also: 1423913
See Also: → 1419605
Product: Toolkit → WebExtensions
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1477688
You need to log in before you can comment on or make changes to this bug.