As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 632064 - remove/deprecate JS_GetScopeChain in lieu of JS_GetGlobalForScopeChain
: remove/deprecate JS_GetScopeChain in lieu of JS_GetGlobalForScopeChain
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla10
Assigned To: Luke Wagner [:luke]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: 659577
  Show dependency treegraph
 
Reported: 2011-02-07 09:25 PST by Luke Wagner [:luke]
Modified: 2012-04-28 12:14 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
rm (21.53 KB, patch)
2011-09-29 10:52 PDT, Luke Wagner [:luke]
mrbkap: review+
Details | Diff | Splinter Review
rm xml uses of GetScopeChain (2.43 KB, patch)
2011-10-04 15:44 PDT, Luke Wagner [:luke]
igor: review+
Details | Diff | Splinter Review

Description User image Luke Wagner [:luke] 2011-02-07 09:25:21 PST
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.
Comment 1 User image Luke Wagner [:luke] 2011-02-07 11:42:07 PST
(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).
Comment 2 User image Luke Wagner [:luke] 2011-02-14 16:04:00 PST
Waldo's on it!

*** This bug has been marked as a duplicate of bug 511781 ***
Comment 3 User image Wan-Teh Chang 2011-02-14 16:10:05 PST
Luke: bug 511781 is not the bug you have in mind.
Comment 4 User image Luke Wagner [:luke] 2011-02-14 16:52:44 PST
D'oh!

*** This bug has been marked as a duplicate of bug 631135 ***
Comment 5 User image Luke Wagner [:luke] 2011-09-27 15:03:15 PDT
Bug 511781 never got fixed, but it will be fixed by compartment-per-global, so I'll fix this bug now.
Comment 6 User image Luke Wagner [:luke] 2011-09-29 10:52:26 PDT
Created attachment 563462 [details] [diff] [review]
rm

This patch also fixes some annoying && || warnings.
Comment 7 User image Luke Wagner [:luke] 2011-10-04 15:44:33 PDT
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?
Comment 8 User image Igor Bukanov 2011-10-05 02:23:59 PDT
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.
Comment 11 User image Tom Schuster [:evilpie] 2012-04-28 12:14:18 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.