381693 - Allow null targetObj arg to xpcIJSModuleLoader::import()

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Alex Fritze]

xpcIJSModuleLoader::import() was intended to have 3 different usages:

1. Import symbols from the EXPORTED_SYMBOLS array to the caller's global object.
2. Import symbols from EXPORTED_SYMBOLS to a targetObj passed into import().
3. Gain access to a module's global object.

The 3rd functionality is helpful for debugging or for importing XPCOM JS modules that don't define EXPORTED_SYMBOLS into other JS code. To support this functionality, all we have to do is to allow a null targetObj argument, so such that 

  var foo = Components.utils.import("gre:myModule.js", null);

returns the global object of myModule.js into foo, but doesn't try to install any symbols on foo's global object.

Patch coming up.

Comment 1

[Alex Fritze]

Attachment: Proposed patch

Comment 2

[Nickolay_Ponomarev]

Please also fix the IDL docs: http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/idl/xpccomponents.idl#172
(as well as the duplicated comment)

Comment 3

[Alex Fritze]

Attachment: documentation changes

Comment 4

[Jeff Walden [:Waldo]]

Can you write a couple tests for this functionality?  There are examples of how to do this in the original bug; testing it shouldn't be much work.

Comment 5

[Robert Sayre]

Good catch. I can never keep this straight, because

js> typeof(null)
object

Do we want/need to deal with |undefined| as well?

Comment 6

[Alex Fritze]

Attachment: testcases

Comment 7

[Alex Fritze]

(In reply to comment #5)
> Do we want/need to deal with |undefined| as well?

I think we should let |undefined| throw an exception like it does now.

Comment 8

[Robert Sayre]

Comment on attachment 265780 [details] [diff] [review]
Proposed patch

looks ok to me.

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 265780 [details] [diff] [review]
Proposed patch

>Index: mozJSComponentLoader.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/js/src/xpconnect/loader/mozJSComponentLoader.cpp,v
>retrieving revision 1.133
>diff -u -p -r1.133 mozJSComponentLoader.cpp
>--- mozJSComponentLoader.cpp	15 May 2007 23:27:40 -0000	1.133
>+++ mozJSComponentLoader.cpp	23 May 2007 08:40:04 -0000
>@@ -1338,7 +1338,7 @@ mozJSComponentLoader::Import(const nsACS
>         jsval *argv = nsnull;
>         rv = cc->GetArgvPtr(&argv);
>         NS_ENSURE_SUCCESS(rv, rv);
>-        if (JSVAL_IS_PRIMITIVE(argv[1]) ||
>+        if ((JSVAL_IS_PRIMITIVE(argv[1]) && !JSVAL_IS_NULL(argv[1])) ||

See jsapi.h:

#define JSVAL_IS_PRIMITIVE(v)   (!JSVAL_IS_OBJECT(v) || JSVAL_IS_NULL(v))

So all you need here is 'if (!JSVAL_IS_OBJECT(v) ||'.

>             !JS_ValueToObject(cx, argv[1], &targetObject)) {
>             return ReportOnCaller(cc, ERROR_SCOPE_OBJ,
>                                   PromiseFlatCString(registryLocation).get());

Given that, you don't need JS_ValueToObject -- just set targetObject = JSVAL_TO_OBJECT(argv[1]).

sr=me with that.

/be

Comment 10

[Alex Fritze]

Thanks for the reviews.

Checked in 2007-07-15 11:21.

xpcIJSModuleLoader.idl 1.5
xpccomponents.idl 1.36
mozJSComponentLoader.cpp 1.142
test_bogus_files.js 1.4
test_import.js 1.5