Closed
Bug 1461751
Opened 7 years ago
Closed 7 years ago
Simplify module resolve hook
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: jonco, Assigned: jonco)
References
Details
Attachments
(1 file)
(deleted),
patch
|
luke
:
review+
baku
:
review+
|
Details | Diff | Splinter Review |
Currently the module resolve hook (HostResolveImportedModule) is supplied as a JSFunction which is set on the global. The browser wants to implement this in C++ so it would be simpler to have this as a standard C++ function pointer like we do for all the other embedding hooks we have. It can also be set on the runtime since it doesn't vary between globals.
Assignee | ||
Comment 1•7 years ago
|
||
Patch to turn the module resolve hook into a function pointer. This moves some complexity from the browser into the shell (which does have to call a JSFunction) which seems like the right thing to do.
Attachment #8975886 -
Flags: review?(luke)
Comment 2•7 years ago
|
||
Comment on attachment 8975886 [details] [diff] [review]
bug1461751-simplify-module-resolve-hook
Review of attachment 8975886 [details] [diff] [review]:
-----------------------------------------------------------------
That does seem like the right trade off. Feel free to ignore suggestion if there is clear local precedent against.
::: js/src/jsapi.h
@@ +4142,5 @@
> extern JS_PUBLIC_API(bool)
> Evaluate(JSContext* cx, const ReadOnlyCompileOptions& options,
> const char* filename, JS::MutableHandleValue rval);
>
> +using ModuleResolveHook = JSObject* (*)(JSContext*, HandleObject, HandleString);
Maybe MutableHandleObject instead of JSObject* return value? In the limit, the callee could have an RAII object on the stack that causes a GC that makes returning a JSObject* fundamentally unsafe...
Attachment #8975886 -
Flags: review?(luke) → review+
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Luke Wagner [:luke] from comment #2)
> Maybe MutableHandleObject instead of JSObject* return value? In the limit,
> the callee could have an RAII object on the stack that causes a GC that
> makes returning a JSObject* fundamentally unsafe...
We do this in a lot of places, so I'm going to leave it. I'm pretty sure the rooting analysis would catch the problem you describe, but I'll confirm that with Steve.
Assignee | ||
Comment 4•7 years ago
|
||
Comment on attachment 8975886 [details] [diff] [review]
bug1461751-simplify-module-resolve-hook
Requesting additional review for script loader changes.
Attachment #8975886 -
Flags: review?(amarchesini)
Updated•7 years ago
|
Attachment #8975886 -
Flags: review?(amarchesini) → review+
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e862899dca3f
Simplify module resolve hook to be a function pointer r=luke r=baku
Comment 6•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in
before you can comment on or make changes to this bug.
Description
•