1056246 - mozJSComponentLoader causes 'WARNING (null):0 - setting a property that has only a getter' warnings

Status: RESOLVED INACTIVE

(Core :: XPConnect, defect)

Description

[Biru [:poiru]]

The JS_SetPropertyById call in mozJSComponentLoader.cpp causes a lot of warnings like:

10:24:50     INFO -  36 INFO System JS : WARNING (null):0 - setting a property that has only a getter

Comment 1

[Biru [:poiru]]

Attachment: Fix 'WARNING (null):0 - setting a property that has only a getter' warnings

Not sure if this is safe, but it seems to have survived a try run: https://tbpl.mozilla.org/?tree=Try&rev=b1ce779d5792

Comment 2

[Bobby Holley (:bholley)]

Comment on attachment 8476110 [details] [diff] [review]
Fix 'WARNING (null):0 - setting a property that has only a getter' warnings

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

Hm, it seems like we really shouldn't be using JS_SetPropertyById at all here, and should be using JS_DefinePropertyById instead.

The fact that you're encountering this warning means that we do indeed hit this case, so it's possible that switching from set to define would cause trouble. But it seems more correct to do it that way, so I think we should try.

Comment 3

[Biru [:poiru]]

Attachment: Fix 'WARNING (null):0 - setting a property that has only a getter' warnings

(In reply to (PTO 8/22 - 9/1) from comment #2)
> Hm, it seems like we really shouldn't be using JS_SetPropertyById at all
> here, and should be using JS_DefinePropertyById instead.

Done. Seems to work fine.

Comment 4

[Florian Quèze [:florian]]

(In reply to (PTO 8/22 - 9/1) from comment #2)

> The fact that you're encountering this warning means that we do indeed hit
> this case, so it's possible that switching from set to define would cause
> trouble.

I looked at the JS code causing this, the typical pattern seems to be:

One .js file doing:
XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
  "resource://gre/modules/PrivateBrowsingUtils.jsm");

And another .js file included by the same xul window doing:
Components.utils.import("resource://gre/modules/PrivateBrowsingUtils.jsm");

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

What's the status of this?

Comment 6

[Biru [:poiru]]

(In reply to David Keeler (:keeler) [use needinfo?] from comment #5)
> What's the status of this?

The patch caused a xpcshell test to fail: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=203c0d2eed84

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\xpcshell\tests\toolkit\mozapps\extensions\test\xpcshell\test_syncGUID.js | test failed (with xpcshell return code: 0), see following log:
TEST-UNEXPECTED-FAIL | resource://gre/modules/addons/XPIProvider.jsm | TypeError: XPIDatabase is undefined at resource://gre/modules/addons/XPIProvider.jsm:3702 - See following stack: 

The XPIDatabase stuff behind the failure is here: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIProvider.jsm#221

Not sure what the deal is and sadly I don't have time to dig into this at the moment. I will take another look in a few weeks, but unassigning for now.

Comment 7

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.