Last Comment Bug 705188 - Use IDL for mozIJSSubScriptLoader::LoadSubScript, {xpcIJSModuleLoader,nsIXPCComponents_Utils}::Import
: Use IDL for mozIJSSubScriptLoader::LoadSubScript, {xpcIJSModuleLoader,nsIXPCC...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: :Ms2ger (⌚ UTC+1/+2)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: 705324
  Show dependency treegraph
 
Reported: 2011-11-24 12:57 PST by :Ms2ger (⌚ UTC+1/+2)
Modified: 2011-12-18 07:12 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (28.46 KB, patch)
2011-11-24 12:57 PST, :Ms2ger (⌚ UTC+1/+2)
no flags Details | Diff | Splinter Review
Patch v2 (26.61 KB, patch)
2011-12-03 14:35 PST, :Ms2ger (⌚ UTC+1/+2)
bzbarsky: review+
Details | Diff | Splinter Review

Description :Ms2ger (⌚ UTC+1/+2) 2011-11-24 12:57:56 PST
Created attachment 576821 [details] [diff] [review]
Patch v1

It's some nice cleanup, but I'm not sure if JS_GetGlobalObject(aCx) is correct (or even what behaviour these methods want, and how that correlates with JSAPI).
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2011-11-28 17:26:01 PST
JS_GetGlobalObject is almost certainly different from what we used to do if the code that's running doesn't have the same global as the context it's running on (think cross-window call).

I'm not exactly comfortable with this review; who touched this code last?
Comment 2 :Ms2ger (⌚ UTC+1/+2) 2011-11-28 23:55:28 PST
sayrer, rginda@netscape.com, jst, and, well, you. How about I leave the global-getting code alone and just clean up the arguments-converting code?
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2011-11-29 01:03:17 PST
Gah.  Are you serious?  Can I still disclaim responsibility somehow?

Leaving the global stuff alone and cleaning up the argument mess sounds great.
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2011-12-03 14:35:14 PST
Created attachment 578859 [details] [diff] [review]
Patch v2

Let's try this...
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2011-12-05 19:09:40 PST
Comment on attachment 578859 [details] [diff] [review]
Patch v2

r=me
Comment 6 :Ms2ger (⌚ UTC+1/+2) 2011-12-18 07:12:46 PST
https://hg.mozilla.org/mozilla-central/rev/93ab106616a2

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