Last Comment Bug 745211 - Remove SetScriptContext from nsIScriptGlobalObject
: Remove SetScriptContext from nsIScriptGlobalObject
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: :Ms2ger (⌚ UTC+1/+2)
:
Mentors:
Depends on:
Blocks:
  Show dependency tree / graph
 
Reported: 2012-04-13 08:51 PDT by :Ms2ger (⌚ UTC+1/+2)
Modified: 2012-05-05 10:25 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Attachments
Patch v1 (12.20 KB, patch)
2012-04-13 08:51 PDT, :Ms2ger (⌚ UTC+1/+2) 		no flags Details | Diff | Splinter Review
Patch v1.1 (12.19 KB, patch)
2012-04-13 09:12 PDT, :Ms2ger (⌚ UTC+1/+2) 		mrbkap: review+
Details | Diff | Splinter Review
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Description :Ms2ger (⌚ UTC+1/+2) 2012-04-13 08:51:33 PDT 
Created attachment 614801 [details] [diff] [review]
Patch v1

Apart from the one call in nsXBLDocumentInfo::~nsXBLDocumentInfo(), SetScriptContext is only called from EnsureScriptEnvironment(), so it is rather silly to put it on nsIScriptGlobalObject. Also, inlining the implementations allows to simplify them, as we know the argument isn't null.
Comment 1 :Ms2ger (⌚ UTC+1/+2) 2012-04-13 09:12:45 PDT 
Created attachment 614820 [details] [diff] [review]
Patch v1.1

And now a patch that compiles...
Comment 2 Blake Kaplan (:mrbkap) 2012-04-16 10:54:04 PDT 
Comment on attachment 614820 [details] [diff] [review]
Patch v1.1

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

One substantive comment. r=me with it addressed.

::: dom/base/nsIScriptGlobalObject.h
@@ -138,5 @@
> -   * Set a new language context for this global.  The native global for the
> -   * context is created by the context's GetNativeGlobal() method.
> -   */
> -
> -  virtual nsresult SetScriptContext(nsIScriptContext *aContext) = 0;

You need to bump the IID for nsIScriptGlobalObject.
Comment 3 :Ms2ger (⌚ UTC+1/+2) 2012-05-05 10:25:19 PDT 
https://hg.mozilla.org/mozilla-central/rev/fb420fe0c2a4
Note You need to log in before you can comment on or make changes to this bug.