remove/deprecate JS_GetScopeChain in lieu of JS_GetGlobalForScopeChain

RESOLVED FIXED in mozilla10

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: luke, Assigned: luke)

Tracking

({dev-doc-complete})

unspecified
mozilla10
dev-doc-complete
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

7 years ago
mxr shoes that 100% of the uses of JS_GetScopeChain in Mozilla are to get at the scope chain's global.  (Is there any other valid thing to do with it?  Hopefully not implement some form of dynamic name lookup.)  Since JS_GetGlobalForScopeChain is what everybody wants (and its more efficient in more than one way), can we nix JS_GetScopeChain or deprecate it and just have it return JS_GetGlobalForScopeChain?  My motivation is that the less leaky API will give us more flexibility (bug 631135 is a weak example) and, when we get around to the scope chain overhaul (bug ?), scope chains won't be objects (while global objects clearly will).

Simple patch.  Will ask on the listserv if no immediate objections.
(Assignee)

Comment 1

7 years ago
(In reply to comment #0)
> (bug 631135 is a weak example)

Hah, 1 hour later it becomes a strong example (end of bug 631135 comment 23).
(Assignee)

Comment 2

7 years ago
Waldo's on it!
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 511781

Comment 3

7 years ago
Luke: bug 511781 is not the bug you have in mind.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
(Assignee)

Comment 4

7 years ago
D'oh!
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 631135
(Assignee)

Comment 5

6 years ago
Bug 511781 never got fixed, but it will be fixed by compartment-per-global, so I'll fix this bug now.
Assignee: general → luke
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
(Assignee)

Updated

6 years ago
Blocks: 659577
(Assignee)

Comment 6

6 years ago
Created attachment 563462 [details] [diff] [review]
rm

This patch also fixes some annoying && || warnings.
Attachment #563462 - Flags: review?(mrbkap)
(Assignee)

Comment 7

6 years ago
Created attachment 564690 [details] [diff] [review]
rm xml uses of GetScopeChain

The assumption of this patch is that, whatever E4X wants to find on the scope chain, it causes a call-obj to be created and put on the scope chain, hence the lazy call-obj creation done by GetScopeChain is unnecessary.  Is that correct Igor?
Attachment #564690 - Flags: review?(igor)

Comment 8

6 years ago
Comment on attachment 564690 [details] [diff] [review]
rm xml uses of GetScopeChain

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

::: js/src/jsxml.cpp
@@ +1762,5 @@
>  
>      {
>          Parser parser(cx);
>          if (parser.init(chars, length, filename, lineno, cx->findVersion())) {
> +            JSObject *scopeChain = GetCurrentScopeChain(cx);

IIRC this scope chain can be replaced by the global. If so, then we can fold the GetCurrentScopeChain into js_GetDefaultXMLNamespace. But this can wait another bug.
Attachment #564690 - Flags: review?(igor) → review+

Updated

6 years ago
Attachment #563462 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 9

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/151f190c27ab
https://hg.mozilla.org/integration/mozilla-inbound/rev/d749864ebd01
Target Milestone: --- → mozilla10
Keywords: dev-doc-needed
https://hg.mozilla.org/mozilla-central/rev/151f190c27ab
https://hg.mozilla.org/mozilla-central/rev/d749864ebd01
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago6 years ago
Resolution: --- → FIXED
Short notice at https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/JS_GetScopeChain, that suggests using JS_GetGlobalForScopeChain.
Added entry to 1.8.7 release notes.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.