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 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 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 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 Wan-Teh Chang 2011-02-14 16:10:05 PST
Luke: bug 511781 is not the bug you have in mind.
Comment 4 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 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 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 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 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 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.