1362816 - chrome/test/unit/test_data_protocol_registration.js and test_no_remote_registration.js fail when collecting code coverage on linux64-jsdcov.

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Greg Mierzwinski [:sparky]]

Attachment: live_backing.log

The error that is available in the attached log or viewable in [1] is caused by something that happens in these tests. From some initial investigations, the file 'test_no_remote_registration.js' gives the error after the factories are unregistered. [2] shows a test run with the removal of the unregistering section. The same can't be said for 'test_data_protocol_registration.js' though as it continues to cause 'CoverageUtils.jsm' to fail when attempting to import 'osfile.jsm' through 'Components.utils.import'.

[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f96599db4d59daf6de31cd054ad520a2bb5f4d00&selectedJob=97125519
[2]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=80b3e4fb7de99e0718f13c0b1d45ad5d91468e1c&selectedJob=97013958

Comment 1

[Greg Mierzwinski [:sparky]]

Here's a simple patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=72dd9dc758e61c7b2cab11b05e3d67e3d14902e9

It turns out the the only reason 'test_data_protocol_registration.js' failed after removing the unregistering section was because it was missing XULAppInfo. Now we just need to find another way of unregistering the factories rather than simply removing that part completely.

Comment 2

[Greg Mierzwinski [:sparky]]

This patch should do it: https://hg.mozilla.org/try/rev/3a83b8c960b60cedee4e7f252f181a229676e83e

The solution was to store an already registered object with a specific contractID, unregister it, register the one needed for the test (with the same contractID but different classID), then re-register the previous object. If the test run goes well, I'll submit it for review.

Comment 3

[Greg Mierzwinski [:sparky]]

Joel, would you be able to review this patch? If not, can you tell me who I should flag as a reviewer for this?

The tests that are running with the current patch is in comment 2. Also, should I run a test with this patch on all platforms?

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

I am happy to review it!

Comment 5

[Greg Mierzwinski [:sparky]]

Attachment: Bug 1362816 - Prevent test_data_protocol_registration.js and test_no_remote_registration.js from overwriting previously registered factories.

This patch does two things. The first is that it fixes the two tests mentioned in the title by saving a previously registered XULAppInfoFactory, unregistering it, registering the new factory, and then at the end of the test re-registering the factory which was registered before the test started. The second thing this patch does is remove the skipping conditions for these two tests so we can run them on linux64-jsdcov.

Review commit: https://reviewboard.mozilla.org/r/142418/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/142418/

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8870882 [details]
Bug 1362816 - Prevent test_data_protocol_registration.js and test_no_remote_registration.js from overwriting previously registered factories.

https://reviewboard.mozilla.org/r/142418/#review146062

please make sure this runs reliably on all platforms (linux, osx, windows, android):
try -b do -p all -u xpcshell -t none

Comment 7

[Greg Mierzwinski [:sparky]]

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5af5e8933e62
Prevent test_data_protocol_registration.js and test_no_remote_registration.js from overwriting previously registered factories. r=jmaher

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5af5e8933e62