Last Comment Bug 694867 - nsIScriptContext::ExecuteScript's first parameter should be a JSScript
: nsIScriptContext::ExecuteScript's first parameter should be a JSScript
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla10
Assigned To: :Ms2ger (⌚ UTC+1/+2)
:
:
Mentors:
Depends on: 694781
Blocks: 695753
  Show dependency treegraph
 
Reported: 2011-10-16 12:30 PDT by :Ms2ger (⌚ UTC+1/+2)
Modified: 2011-10-30 05:28 PDT (History)
2 users (show)
Ms2ger: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part a: Pass a JSScript to nsIScriptContext::Serialize and store a JSScript in nsXULPrototypeScript::ScriptObjectHolder::mObject (5.24 KB, patch)
2011-10-16 12:31 PDT, :Ms2ger (⌚ UTC+1/+2)
mounir: review+
Details | Diff | Splinter Review
Part b: Pass a JSScript to nsXULPrototypeScript::Set (6.10 KB, patch)
2011-10-16 12:33 PDT, :Ms2ger (⌚ UTC+1/+2)
mounir: review+
Details | Diff | Splinter Review
Part c: Return a JSScript from nsXULPrototypeCache::GetScript (4.54 KB, patch)
2011-10-16 12:33 PDT, :Ms2ger (⌚ UTC+1/+2)
mounir: review+
Details | Diff | Splinter Review
Part d: Store a JSScript in nsXULPrototypeCache (3.15 KB, patch)
2011-10-16 12:36 PDT, :Ms2ger (⌚ UTC+1/+2)
mounir: review+
Details | Diff | Splinter Review
Part e: Pass JSScript to nsXULDocument::ExecuteScript (1.68 KB, patch)
2011-10-16 12:37 PDT, :Ms2ger (⌚ UTC+1/+2)
mounir: review+
Details | Diff | Splinter Review
Part f: Pass a JSScript to nsIScriptContext::ExecuteScript (4.39 KB, patch)
2011-10-16 12:38 PDT, :Ms2ger (⌚ UTC+1/+2)
mounir: review+
Details | Diff | Splinter Review

Description :Ms2ger (⌚ UTC+1/+2) 2011-10-16 12:30:11 PDT
As requested in bug 694781 comment 1.
Comment 1 :Ms2ger (⌚ UTC+1/+2) 2011-10-16 12:31:54 PDT
Created attachment 567349 [details] [diff] [review]
Part a: Pass a JSScript to nsIScriptContext::Serialize and store a JSScript in nsXULPrototypeScript::ScriptObjectHolder::mObject
Comment 2 :Ms2ger (⌚ UTC+1/+2) 2011-10-16 12:33:07 PDT
Created attachment 567351 [details] [diff] [review]
Part b: Pass a JSScript to nsXULPrototypeScript::Set
Comment 3 :Ms2ger (⌚ UTC+1/+2) 2011-10-16 12:33:59 PDT
Created attachment 567352 [details] [diff] [review]
Part c: Return a JSScript from nsXULPrototypeCache::GetScript
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2011-10-16 12:36:15 PDT
Created attachment 567353 [details] [diff] [review]
Part d: Store a JSScript in nsXULPrototypeCache

Note that I don't have to change the consumers because they pass the mObject member of ScriptObjectHolder, which I changed in part a.
Comment 5 :Ms2ger (⌚ UTC+1/+2) 2011-10-16 12:37:16 PDT
Created attachment 567354 [details] [diff] [review]
Part e: Pass JSScript to nsXULDocument::ExecuteScript

Same comment as for part d.
Comment 6 :Ms2ger (⌚ UTC+1/+2) 2011-10-16 12:38:23 PDT
Created attachment 567355 [details] [diff] [review]
Part f: Pass a JSScript to nsIScriptContext::ExecuteScript

Note that nsXULDocument::ExecuteScript is its only caller.
Comment 7 Mounir Lamouri (:mounir) 2011-10-17 05:16:53 PDT
Everything seems fine but it would have been easier to read in one file given that most patches are basically adding some static_cast which are removed by other patches.
Comment 8 :Ms2ger (⌚ UTC+1/+2) 2011-10-23 08:48:32 PDT
Comment on attachment 567351 [details] [diff] [review]
Part b: Pass a JSScript to nsXULPrototypeScript::Set

>--- a/dom/base/nsDOMScriptObjectHolder.h
>+++ b/dom/base/nsDOMScriptObjectHolder.h

>+  JSScript* getObject() const {

Looking back at the patch, mind if I make it getScript?
Comment 9 Mounir Lamouri (:mounir) 2011-10-23 16:14:22 PDT
(In reply to Ms2ger from comment #8)
> Looking back at the patch, mind if I make it getScript?

Makes more sense indeed.

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