[meta] Move/expose Cu.import to/on ChromeUtils or similar webidl machinery and rewrite chrome JS to use that instead

RESOLVED WORKSFORME

Status

()

enhancement
P3
normal
RESOLVED WORKSFORME
2 years ago
Last year

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

(Depends on 1 bug, Blocks 1 bug, {meta})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 affected)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Assignee

Description

2 years ago
Every time we use Cu.import, we incur a tiny additional cost on top of whatever the actual implementation does, because xpconnect is really general, the implementation lives in C++, and callsites are in JS.

This wouldn't matter so much if we didn't use Cu.import a lot. But we do. Like, really a lot, and this will likely just increase given we now have shared compartments for jsms so we feel less constrained by cost there.

Even if the overhead of an individual xpconnect call on a pgo build on every user's machine was limited to 1µs (and I see slightly higher numbers on my opt (but non-pgo) build on a fast mbp, so that seems generous for most actual users on weaker hardware), we call this code thousands of time over normal browser usage. Bobby said overhead from webidl should be on the order of nanoseconds because it's all codegen. So it seems like it might be a good idea to either move this to webidl or add a webidl equivalent assuming that is a relatively easy thing to do.
Now with handling of null target obj arg fixed...
Attachment #8937226 - Attachment is obsolete: true
Assignee

Comment 3

2 years ago
I'm noticing 2 issues when mass-replacing Cu.import calls on top of this patch.

1) JS components don't have access to ChromeUtils, so this breaks them. Kind of annoying for my global search/replace because we don't have a good way to filter those...

Is there a reasonable way to get a ref to ChromeUtils from a JS component, or are those just out of luck?

2) something weird is happening in some of the webextension scripts. Specifically, I see:

ReferenceError: ExtensionParent is not defined[Learn More] ext-backgroundPage.js:10:1

and similar errors, coming from code that now does this:


ChromeUtils.import("resource://gre/modules/ExtensionParent.jsm");
var {
  HiddenExtensionPage,
  promiseExtensionViewLoaded,
} = ExtensionParent;

in ext-backgroundPage.js ( https://dxr.mozilla.org/mozilla-central/rev/1624b88874765bf57e9feba176d30149c748d9d2/toolkit/components/extensions/ext-backgroundPage.js#6 )

It seems like perhaps the global situation for that script is weird, or something? I'm not really sure what the issue is and if it's easy to fix.
Flags: needinfo?(bzbarsky)
Assignee

Comment 4

2 years ago
(I checked that ExtensionParent is in EXPORTED_SYMBOLS)
> Is there a reasonable way to get a ref to ChromeUtils from a JS component, or are those just out of luck?

Hmm.  I would expect ChromeUtils to be exposed in JS components, just like everything else that uses a BackstagePass as the global.  Can you point to a specific file where you're seeing this not work, and I'll take a look?

> ReferenceError: ExtensionParent is not defined[Learn More] ext-backgroundPage.js:10:1

I'll take a look.
> ReferenceError: ExtensionParent is not defined[Learn More] ext-backgroundPage.js:10:1

OK, this one I have sort of figured out.

We're running code from chrome://extensions/content/ext-backgroundPage.js in a sandbox, as far as I can tell.  I haven't checked where that sandbox comes from, but apparently it defines a Cu in that sandbox global.  So when Cu.import examines the caller compartment it sees the sandbox global and defines the "ExtensionParent" property on that global.

On the other hand, the ChromeUtils.import call is calling through a cross-compartment proxy.  By the time we land in import, our current compartment is a BackstagePass global, not the sandbox.  And that BackstagePass is where ChromeUtils.import ends up defining the property.  Then we unwind and the property doesn't exist on the sandbox global.  

OK, so why the difference?  The sandbox is coming from this code in ExtensionCommon.jsm, in _createExtGlobal:

    let global = Cu.Sandbox(Services.scriptSecurityManager.getSystemPrincipal(), {
      wantXrays: false,
      sandboxName: `Namespace of ext-*.js scripts for ${this.processType} (from: resource://gre/modules/ExtensionCommon.jsm)`,
    });

    Object.assign(global, {global, Cc, Ci, Cu, Cr, XPCOMUtils, ChromeUtils, ChromeWorker, ExtensionAPI, ExtensionCommon, MatchPattern, MatchPatternSet, MatchGlob, StructuredCloneHolder, extensions: this});

When we do that Object.assign, we wrap all those things into the sandbox global's compartment.  The thing is, when we wrap "Cu", we notice it's an XPCWrappedNative for an XPCOM C++ object, and just create a new XPCWrappedNative for it in the new compartment (!).  But when we wrap "ChromeUtils", it's just a plain object as far as the wrapping machinery is concerned, so we give it a cross-compartment wrapper.

We could probably fix this specific case by adding a way to request that a ChromeUtils specific to the sandbox be defined on the sandbox (via the wantGlobalProperties machinery) and using it for this sandbox instead of grabbing the one from ExtensionCommon.jsm.
Flags: needinfo?(bzbarsky)
With this patch, toolkit/components/extensions/test/mochitest/test_chrome_ext_background_page.html passes, which is a good sign.  We probably want something similar done to devtools/shared/builtin-modules.js but would need to hunt down where the relevant sandboxes are created.
Attachment #8938256 - Flags: review?(kmaglione+bmo)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Assignee: bzbarsky → nobody
Status: ASSIGNED → NEW
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #6)
> We're running code from chrome://extensions/content/ext-backgroundPage.js in
> a sandbox, as far as I can tell.  I haven't checked where that sandbox comes
> from, but apparently it defines a Cu in that sandbox global.  So when
> Cu.import examines the caller compartment it sees the sandbox global and
> defines the "ExtensionParent" property on that global.

I really hate these sandboxes... I've been hoping to get rid of them, if I ever get a review for bug 1393270.
Attachment #8938256 - Flags: review?(kmaglione+bmo) → review+
Assignee

Comment 9

2 years ago
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #5)
> > Is there a reasonable way to get a ref to ChromeUtils from a JS component, or are those just out of luck?
> 
> Hmm.  I would expect ChromeUtils to be exposed in JS components, just like
> everything else that uses a BackstagePass as the global.  Can you point to a
> specific file where you're seeing this not work, and I'll take a look?

This was XPIProviderUtils.js . Looks like it's being manually loaded onto a sandbox via the subscript loader etc., much like the stuff you're fixing in the other patch you put up.
Ok, good.  Then just throwing in wantGlobalProperties bits on top of my other patch will fix it.
(In reply to :Gijs from comment #9)
> This was XPIProviderUtils.js . Looks like it's being manually loaded onto a
> sandbox via the subscript loader etc., much like the stuff you're fixing in
> the other patch you put up.

That's another sandbox that I hate and want to get rid of. r=me if you want to just replace it with a plain object.
Assignee

Comment 12

2 years ago
Sigh. So I couldn't get devtools to work quickly, so I excluded that from any changes. Then I pushed to try with talos to try to get numbers on how much this buys us (not sure that'd really show much, but I figured it was worth a shot), but tpaint and friends don't run to completion anymore, so one of my ~2900 replaces must be breaking them. There's no useful error messages, they just time out. I'll look at this more after the break.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Assignee

Comment 13

2 years ago
OK. On my local Windows machine, against a 366-ish ms baseline when running ts_paint at 50 cycles, these changes + replacing Cu.import in 950 files (excluding tests, excluding devtools/, and excluding testing/talos because it has other stuff that it subscript loads in strange ways on non-privileged things that have had extra permissions put on them via UniversalXPConnect), this nets us about 1-2ms on the median, 1.8ms on the average. That's basically 0.5%. Talos on infra is too noisy to draw any real conclusions, as far as I can tell, but https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=ff9f99050bc6f23f5dd2595ef12b878365e40d86&newProject=try&newRevision=c4457b8f3e6e00c403ff9d33f2ad447d2bfec77a&framework=1 is there if people want to look at it.

What I will add is that for the second run (which, though it's reported in the set of results unlike the first run, I think gets dropped from the averages/medians for being too skewed), I'm seeing larger impacts, more around the 1% mark both on talos and locally.

0.5-1% on startup is pretty slim, and is likely to be lower on machines where startup times are more IO-bound, and switching our main "module" system in chrome JS land to a different API is non-trivial (though the rewrite scripts help!), but I think on balance this is just about still worth doing, also because the patches are essentially 80% of the way there, AFAICT...

However, I'd like a second opinion. Florian/Mike? :-)
Flags: needinfo?(mconley)
Flags: needinfo?(florian)
(In reply to :Gijs from comment #13)

> However, I'd like a second opinion. Florian/Mike? :-)

If we believe it's faster, and talos shows no regression and even a slight win, then the question becomes: do we think this makes the code nicer to read?

I'm leaning toward a yes, mostly because replacing it everywhere makes the code base more consistent, but I guess I could also be convinced that replacing "Components.utils" with "Cu" everywhere is another reasonable way to make this more consistent.

We could also use scripts to remove many now-unused 'Cu' variables, and that may add another tiny win.
Flags: needinfo?(florian)
This seems like small-ish potatoes, but a win is a win. If this is almost done, and assuming the last 20% isn't 90% of the work, then this seems like it's worth finishing off.

Note that we'll need to update documentation, and do something about deprecating Cu.import. to avoid re-introducing it.
Flags: needinfo?(mconley)
It may not be a huge improvement now, but moving the module import API to WebIDL may allow us to make much bigger improvements.

(In reply to Florian Quèze [:florian] from comment #14)
> I'm leaning toward a yes, mostly because replacing it everywhere makes the
> code base more consistent, but I guess I could also be convinced that
> replacing "Components.utils" with "Cu" everywhere is another reasonable way
> to make this more consistent.

That's not as likely to make a huge difference as you might expect. There's a layer of caching that makes subsequent access to Components properties relatively cheap, so switching to Cu everywhere isn't likely to be a huge win. And there's a chance we'll just wind up with extra, unused Cu definitions that just wind up being more expensive in the long run.
Assignee

Comment 17

2 years ago
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #16)
> It may not be a huge improvement now, but moving the module import API to
> WebIDL may allow us to make much bigger improvements.

Can you elaborate a bit on this and what you have in mind? I'm still kind of on the fence about making this change. 


(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews and needinfos from comment #15)
> This seems like small-ish potatoes, but a win is a win. If this is almost
> done, and assuming the last 20% isn't 90% of the work, then this seems like
> it's worth finishing off.

As far as I can tell, we'd need to:

1) get review on the ChromeUtils addition (ie patch 1 here)
2) fix the eslint plugin so it detects the ChromeUtils thing as adding globals to the relevant scope
3) create a custom eslint rule to produce errors for Cu.import
4) re-educate everyone and MDN on "how to import jsms in chrome scope"

5) either:
a) fix all the consumers that break with naive replacements, including tests, devtools and talos (devtools and talos being the ones I know cause some issues, but I expect there might be other edgecases that I'd run into when running stuff on try)

OR:
b) i)  auto-replace all the autoreplace-able instances
   ii) file followup bugs for the remainder
   iii) selectively make exceptions to the eslint rule


That's a bunch of work (I'm a bit worried about (4), especially that we're not moving to something more standards-based like ES modules (which I *get*, obviously, is an even/much bigger switch that we can't make quickly)) for 0.5-1% ts_paint gains.

To be clear, I'm not saying we *shouldn't* do it, but I'd be happier sinking in some more time to try to get there if I had a better idea of the gains it would enable.
Flags: needinfo?(kmaglione+bmo)
Comment on attachment 8937227 [details] [diff] [review]
part 1.  Add a ChromeUtils.import that does what Cu.import does

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

::: dom/base/ChromeUtils.cpp
@@ +376,5 @@
>  /* static */ void
> +ChromeUtils::Import(const GlobalObject& aGlobal,
> +                    const nsAString& aResourceURI,
> +                    const Optional<JS::Handle<JSObject*>>& aTargetObj,
> +                    JS::MutableHandle<JSObject*> aRetval,

Nit: JS::HandleObject and JS::MutableHandleObject, please.

@@ +384,5 @@
> +  RefPtr<mozJSComponentLoader> moduleloader = mozJSComponentLoader::Get();
> +  MOZ_ASSERT(moduleloader);
> +
> +  NS_ConvertUTF16toUTF8 registryLocation(aResourceURI);
> +  

Nit: Useless whitespace.

@@ +389,5 @@
> +  AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(
> +    "ChromeUtils::Import", OTHER, registryLocation);
> +
> +  JSContext* cx = aGlobal.Context();
> +  JS::Rooted<JS::Value> targetObj(cx);

Nit: JS::RootedValue please.

@@ +408,5 @@
> +    return;
> +  }
> +
> +  // Import() on the component loader can return NS_OK while leaving an
> +  // exception on the JSContext.  Check for that case.

I wonder if we should fix that...

::: dom/webidl/ChromeUtils.webidl
@@ +201,5 @@
>    void idleDispatch(IdleRequestCallback callback,
>                      optional IdleRequestOptions options);
> +
> +  /**
> +   *

Nit: Remove blank line.

@@ +216,5 @@
> +   * on the caller's global object. If 'EXPORTED_SYMBOLS' is not
> +   * found, an error is thrown.
> +   *
> +   * @param aResourceURI A resource:// URI string to load the module from.
> +   * @param aTargetObj  the object to install the exported properties on.

Nit: Or null.
Attachment #8937227 - Flags: review+
Flags: needinfo?(kmaglione+bmo)
(In reply to :Gijs from comment #17)
> (In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're
> blocked) from comment #16)
> > It may not be a huge improvement now, but moving the module import API to
> > WebIDL may allow us to make much bigger improvements.
> 
> Can you elaborate a bit on this and what you have in mind? I'm still kind of
> on the fence about making this change.

Probably the biggest thing would be moving the lazy import machinery to native
code. Just defining and re-defining the lazy property descriptors is pretty
expensive in JS. But, beyond that, every time we delazify a property, we:

- Create a temporary object.
- Call Cu.import, which invokes all of the normal import machinery.
- Look up the EXPORTED_SYMBOLS property, first on the module lexical scope, then
  on the module global.
- For each property listed, enter the module compartment, look up that symbol
  first on the module lexical scope, then on the module global. Enter the target
  object scope, wrap the value, assign it to a property on that object.
- Lookup the symbol on the temporary object.
- Redefine the property descriptor.

If ChromeUtils were responsible for the module import machinery, we could skip
most of the above, and just lookup the property that we currently care about.

We could probably do that without Cu.import to ChromeUtils, but I'd rather have
all of these things in one place.

> (In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews and
> needinfos from comment #15)
> As far as I can tell, we'd need to:
>
> 1) get review on the ChromeUtils addition (ie patch 1 here)

r=me

> That's a bunch of work (I'm a bit worried about (4), especially that we're
> not moving to something more standards-based like ES modules (which I *get*,
> obviously, is an even/much bigger switch that we can't make quickly)) for
> 0.5-1% ts_paint gains.

It's not really that much work. The ESLint changes are pretty trivial, and the
docs on MDN are mostly archived at this point.
May I ask a naieve question: why do we allow a second argument to at all?

If the goal is to migrate to the `import` of ES modules, the second argument prevents a context-free syntactic reorganization.  Is it that we'd need to do this migration at some point, and there's no advantage to doing it now?  Is there some behaviour that can't be expressed without the second argument?

Perhaps this is too much of a change?  I see that we prevent `Cu.import("foo")` in https://searchfox.org/mozilla-central/rev/41925c0b6c6d58578690121de439d2a8d3d690f3/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-single-arg-cu-import.js and even `let foo = Cu.import("foo", bar)` in https://searchfox.org/mozilla-central/rev/41925c0b6c6d58578690121de439d2a8d3d690f3/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-import-into-var-and-global.js.
Flags: needinfo?(gijskruitbosch+bugs)
> Nit: JS::HandleObject and JS::MutableHandleObject, please.
> Nit: JS::RootedValue please.

Hmm.  This is DOM vs XPConnect style differences.  Looks like this file is now a mix of styles already (started out DOM-style).  I was assuming it was still DOM-style...

> Nit: Useless whitespace.

Fixed.

> I wonder if we should fix that...

Depends on whether we plan to keep the xpidl bits at all.

> Nit: Remove blank line.

Fixed.
Assignee: gijskruitbosch+bugs → bzbarsky
Status: NEW → ASSIGNED
(In reply to Nick Alexander :nalexander from comment #20)
> May I ask a naieve question: why do we allow a second argument to at all?
> 
> If the goal is to migrate to the `import` of ES modules, the second argument
> prevents a context-free syntactic reorganization.  Is it that we'd need to
> do this migration at some point, and there's no advantage to doing it now? 
> Is there some behaviour that can't be expressed without the second argument?

The second argument prevents importing all exported items into the global scope. For example for these statements:

1) Cu.import("resource://gre/modules/AddonManager.jsm");
2) var { AddonManager } = Cu.import("resource://gre/modules/AddonManager.jsm");
3) var { AddonManager } = Cu.import("resource://gre/modules/AddonManager.jsm", {});

For 1) AddonManager, AddonManagerPrivate are defined in the global scope.
For 2) AddonManager, AddonManagerPrivate are defined in the global scope, and AddonManager would be additionally defined as a variable in whatever scope the statement was in.
For 3) only the AddonManager variable gets defined, nothing extra in the global scope.

Having everything imported into the global scope makes it difficult for ESLint currently - we have to jump through a few hoops to work out what globals are actually defined (e.g. https://searchfox.org/mozilla-central/source/tools/lint/eslint/modules.json).

Additionally, the single argument form makes it harder to work out if we're importing files into different scopes unnecessarily.

Personally, I'd much rather we moved towards a form which only allowed importing named items from modules as it would make it clearer what was expected to be used where.

From a quick scan, I think most of the two-argument forms in the tree equate to either an empty object or the global scope itself being passed in, which could potentially be replaced fairly easily. 

> Perhaps this is too much of a change?  I see that we prevent
> `Cu.import("foo")` in
> https://searchfox.org/mozilla-central/rev/
> 41925c0b6c6d58578690121de439d2a8d3d690f3/tools/lint/eslint/eslint-plugin-
> mozilla/lib/rules/no-single-arg-cu-import.js and even `let foo =
> Cu.import("foo", bar)` in
> https://searchfox.org/mozilla-central/rev/
> 41925c0b6c6d58578690121de439d2a8d3d690f3/tools/lint/eslint/eslint-plugin-
> mozilla/lib/rules/no-import-into-var-and-global.js.

no-single-arg-cu-import is only used by devtools currently. Mossop and I discussed moving to it for all of mozilla-central around the time we started doing major work on ESLint, and decided it would be too much work at that time (and possibly risky).

no-import-into-var-and-global.js was designed to avoid case 2 above, which wasn't very clear to developers.

I'll leave the NI to Gijs in case he has additional thoughts.
Thanks Mark, very helpful.

(In reply to Mark Banner (:standard8) from comment #23)
> (In reply to Nick Alexander :nalexander from comment #20)
> > May I ask a naieve question: why do we allow a second argument to at all?
> > 
> > If the goal is to migrate to the `import` of ES modules, the second argument
> > prevents a context-free syntactic reorganization.  Is it that we'd need to
> > do this migration at some point, and there's no advantage to doing it now? 
> > Is there some behaviour that can't be expressed without the second argument?
> 
> The second argument prevents importing all exported items into the global
> scope. For example for these statements:
> 
> 1) Cu.import("resource://gre/modules/AddonManager.jsm");
> 2) var { AddonManager } =
> Cu.import("resource://gre/modules/AddonManager.jsm");
> 3) var { AddonManager } =
> Cu.import("resource://gre/modules/AddonManager.jsm", {});
> 
> For 1) AddonManager, AddonManagerPrivate are defined in the global scope.
> For 2) AddonManager, AddonManagerPrivate are defined in the global scope,
> and AddonManager would be additionally defined as a variable in whatever
> scope the statement was in.
> For 3) only the AddonManager variable gets defined, nothing extra in the
> global scope.
> 
> Having everything imported into the global scope makes it difficult for
> ESLint currently - we have to jump through a few hoops to work out what
> globals are actually defined (e.g.
> https://searchfox.org/mozilla-central/source/tools/lint/eslint/modules.json).
> 
> Additionally, the single argument form makes it harder to work out if we're
> importing files into different scopes unnecessarily.
> 
> Personally, I'd much rather we moved towards a form which only allowed
> importing named items from modules as it would make it clearer what was
> expected to be used where.

Ah, what I really think we should move toward is one argument with implicit {} for the second argument.  That is, don't _ever_ pollute global scope, and make consumers be explicit about what they import into their local scope.  That's what syntax-`import` does, right?

> From a quick scan, I think most of the two-argument forms in the tree equate
> to either an empty object or the global scope itself being passed in, which
> could potentially be replaced fairly easily. 
> 
> > Perhaps this is too much of a change?  I see that we prevent
> > `Cu.import("foo")` in
> > https://searchfox.org/mozilla-central/rev/
> > 41925c0b6c6d58578690121de439d2a8d3d690f3/tools/lint/eslint/eslint-plugin-
> > mozilla/lib/rules/no-single-arg-cu-import.js and even `let foo =
> > Cu.import("foo", bar)` in
> > https://searchfox.org/mozilla-central/rev/
> > 41925c0b6c6d58578690121de439d2a8d3d690f3/tools/lint/eslint/eslint-plugin-
> > mozilla/lib/rules/no-import-into-var-and-global.js.
> 
> no-single-arg-cu-import is only used by devtools currently. Mossop and I
> discussed moving to it for all of mozilla-central around the time we started
> doing major work on ESLint, and decided it would be too much work at that
> time (and possibly risky).

Thanks for this context.
 
> no-import-into-var-and-global.js was designed to avoid case 2 above, which
> wasn't very clear to developers.

I agree!
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #21)
> > Nit: JS::HandleObject and JS::MutableHandleObject, please.
> > Nit: JS::RootedValue please.
> 
> Hmm.  This is DOM vs XPConnect style differences.  Looks like this file is
> now a mix of styles already (started out DOM-style).  I was assuming it was
> still DOM-style...

Fair enough. Most of this code came from devtools and xpconnect, which use JS style, so it wound up with JS naming conventions. But it is technically a DOM module.

> > I wonder if we should fix that...
> 
> Depends on whether we plan to keep the xpidl bits at all.

I think we should probably keep them until this is available on all supported branches, to make migration easier for system add-ons. After that, I think we can remove them.
(In reply to Nick Alexander :nalexander from comment #24)
> Ah, what I really think we should move toward is one argument with implicit
> {} for the second argument.  That is, don't _ever_ pollute global scope, and
> make consumers be explicit about what they import into their local scope. 
> That's what syntax-`import` does, right?

That's easier said than done. Assuming that everywhere we currently import into the global scope, we can actually get a reference to the global scope object (which I'm not 100% sure of), we could probably make this work.

The problem with that is that the default behavior in that case would be to just return the module global. And using the module global is something we generally want to discourage, but still unfortunately need to support.

It would be nice if we could just return a temporary object containing the exported symbols when no second argument is passed, but that would break existing use cases that depend on getting the module global.

The alternative would probably be to either only return the module global when `null` is passed, or to add a separate import method that returns the module global. That's probably doable, but is a lot more work. And since I'd rather we get this done sooner than later, I'd rather we stick with the mostly compatible interface for now, and aim for a larger refactoring in a follow-up.
bzexport fail...
Assignee: bzbarsky → gijskruitbosch+bugs
Assignee

Comment 28

Last year
(In reply to Mark Banner (:standard8) from comment #23)
> I'll leave the NI to Gijs in case he has additional thoughts.

I don't really have any. I think ES.next modules are a good idea, but it's going to be a long slog, and I'd like to keep this focused on the incremental improvement of switching to something (even only slightly) better than Cu.import. As comment 19 suggests, there's further improvements that this will enable. If one of them is eventually ES.next modules, so much the better, but not right here/now, because of lazy loading, process-singleton semantics, and other concerns. AFAICT runtime/dynamic import of modules is still tbd for ES.next as well, so perhaps in the near future that story will improve for us through standards rather than through making up our own story for it. :-)
Flags: needinfo?(gijskruitbosch+bugs)
Keywords: meta
Summary: Move/expose Cu.import to/on ChromeUtils or similar webidl machinery and rewrite chrome JS to use that instead → [meta] Move/expose Cu.import to/on ChromeUtils or similar webidl machinery and rewrite chrome JS to use that instead
Assignee

Updated

Last year
Depends on: 1431057
Assignee

Updated

Last year
Attachment #8938256 - Attachment is obsolete: true
Assignee

Updated

Last year
Attachment #8937227 - Attachment is obsolete: true
Assignee

Updated

Last year
Attachment #8942992 - Attachment is obsolete: true
Assignee

Updated

Last year
Depends on: 1431069
Assignee

Comment 29

Last year
This is the non-JS/xpconnect bit of the earlier patches that'll need to land when ChromeUtils support and the eslint fix in bug 1431069 has landed.
Attachment #8943246 - Flags: review+
Assignee

Updated

Last year
Depends on: 1431104

Comment 30

Last year
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d463ab1b36b
part 2.  Allow sandboxes to import the ChromeUtils API.  r=kmag
I wound up doing the ESLint part of this in bug 1431533, as part of the rule I added for that API. That takes care of migrating most of the callers in code that ESLint runs on, but leaves devtools and the parts of the tree in the ESLint ignore list mostly untouched.
Assignee

Comment 34

Last year
bug 1431533 does most of the remaining work here, so going to just mark it as a dep. :-)
Depends on: 1431533
Depends on: 1434373
Depends on: 1434374
Assignee

Comment 35

Last year
The lion's share of the work here is done, so going to close this as wfm. Thanks everyone, especially Kris and bz, for all the work!
Status: ASSIGNED → RESOLVED
Closed: Last year
Keywords: leave-open
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.