Add SpecialPowers API for importing a jsm in a chrome process

RESOLVED FIXED in Firefox 45

Status

Testing
Mochitest
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

unspecified
mozilla45
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
A somewhat common pattern is for a test to require that some script be imported in the chrome process, which requires a fair amount of boilerplate to be e10s-compatible:

if (SpecialPowers.isMainProcess()) {
  SpecialPowers.Cu.import("resource://gre/modules/ContactService.jsm");
} else {
  SpecialPowers.loadChromeScript(SimpleTest.getTestFileURL('contacts_chromescript.js'));
}

...where contacts_chromescript.js is a file that has a single line that does the import of ContactService.jsm.

It would be nice to be able to do something like |SpecialPowers.importChromeScript("resource://gre/modules/ContactService.jsm")| and have it do whatever.

The code above probably works even without the test on isMainProcess(). So basically you need a generic chrome script that will take a message containing a string that is the script to load in the chrome process.
(Assignee)

Comment 1

2 years ago
Suggestions welcome on the name. importInMainProcess()? importInChromeProcess()?
(Assignee)

Updated

2 years ago
Assignee: nobody → continuation
(Assignee)

Comment 2

2 years ago
Other test files that do something similar:
  - dom/settings/tests/test_settings_service.js
  - dom/tests/mochitest/geolocation/test_mozsettings.html
  - dom/tests/mochitest/geolocation/test_mozsettingsWatch.html
  - dom/permission/tests/test_permission_basics.html
  - dom/datastore/tests/*
  - dom/requestsync/tests/*
  - devtools/server/tests/mochitest/test_settings.html
  - devtools/client/webide/test/test_device_settings.html
(Assignee)

Comment 3

2 years ago
Created attachment 8689909 [details] [diff] [review]
Add SpecialPowers API for importing a jsm into the main process.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=212d4ba2f332
Attachment #8689909 - Flags: review?(jmaher)
Comment on attachment 8689909 [details] [diff] [review]
Add SpecialPowers API for importing a jsm into the main process.

Review of attachment 8689909 [details] [diff] [review]:
-----------------------------------------------------------------

thanks for writing a test for this.
Attachment #8689909 - Flags: review?(jmaher) → review+

Comment 5

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fd167583034
(Assignee)

Updated

2 years ago
Blocks: 1226708
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/e5c344b2785e for android m(16) failures:
https://treeherder.mozilla.org/logviewer.html#?job_id=17620095&repo=mozilla-inbound
Flags: needinfo?(continuation)
(Assignee)

Comment 7

2 years ago
The test I added times out on Android.
do we have the ability for loadChromeScript on android?  |./mach android-emulator| works great to test stuff locally if you have a local android objdir.

it could be that since we have a java front end that this is just failing to import/load the chrome script and would not be applicable to android.  If that is the case we should find a better error strategy to help future test authors reduce the pain.
(Assignee)

Comment 9

2 years ago
"ImportTesting.jsm shouldn't be loaded before we import it" passes, so the loadChromeScript of importtesting_chromescript.js should be working.
Flags: needinfo?(continuation)
(Assignee)

Comment 10

2 years ago
The second call to importInMainProcess() is failing with NS_ERROR_FILE_NOT_FOUND in the parent process.
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb073252dbcf
I'm guessing this is from the attempt to import "resource://testing-common/ImportTesting.jsm". Maybe the way testing files are packaged up on Android is different somehow.
(Assignee)

Comment 11

2 years ago
modules/ImportTesting.jsm exists in the common.tests.zip files on both Android and Linux, so maybe the chrome manifest isn't being created properly on Android.
(Assignee)

Comment 12

2 years ago
Are you okay with me landing with this test disabled on Android? I'll file a bug and refer to it, but it would be better for somebody with experience with Mochitests on Android to look at it. The code itself in my patch shouldn't behave differently on Android than on desktop.
Flags: needinfo?(jmaher)
that is just fine, please cc :gbrown on the new bug.
Flags: needinfo?(jmaher)
(Assignee)

Comment 14

2 years ago
Created attachment 8692097 [details] [diff] [review]
Improve error checking and disable on Android.

This does a few things:

1. Check that the exception thrown is actually what we are expecting.

2. Put a try-catch block around the second call to
importInMainProcess. This is nice because it turns failures like the
Android one into an actual test failure rather than just causing the
test to hang forever.

3. Disable the test on Android. I filed bug 1228060 for this.

I'll land this folded into the other patch.
Attachment #8692097 - Flags: review?(jmaher)
Comment on attachment 8692097 [details] [diff] [review]
Improve error checking and disable on Android.

Review of attachment 8692097 [details] [diff] [review]:
-----------------------------------------------------------------

thanks for filing the bug and finishing this up!
Attachment #8692097 - Flags: review?(jmaher) → review+

Comment 16

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/34bbdad6b990

Comment 17

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9350db715954

Comment 18

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/34bbdad6b990
https://hg.mozilla.org/mozilla-central/rev/9350db715954
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.