453571 - Replace nsAXPCNativeCallContext users with [optional] IDL arguments etc.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Johnny Stenback (:jst)]

We've got a fair bit of code that uses nsAXPCNativeCallContext to deal with optional arguments and to access the current JS context or what not, all of which can be done in easier ways. In most cases we don't need to use the native call context at all, so this bug is about removing as many users of it as possible.

Comment 1

[Johnny Stenback (:jst)]

Attachment: Stage one. Get the current JS context with less code.

Comment 2

[Johnny Stenback (:jst)]

Attachment: Stage 2a, use more [optional].

This makes us use [optional] in the cases where it's obvious we should and easy to do so. There's one more patch coming that deals with the window.open mess, but I'll hold off until these are in to attach that...

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 336947 [details] [diff] [review]
Stage 2a, use more [optional].

r+sr=bzbarsky.  Leaving the request for Jonas to review in case you still want him to look at this, but you could also just land it.

Comment 4

[Johnny Stenback (:jst)]

Stage one and 2a checked in. Jonas, feel free to have a look if you want to.

Comment 5

[Johnny Stenback (:jst)]

Attachment: Stage 2b. Clean up how we deal with window.open() and its arguments.

Boris, this makes us use [optional] for window.open* as well. Pretty straight forward, once I realized I could use the [binaryname()] IDL flag here.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 337155 [details] [diff] [review]
Stage 2b. Clean up how we deal with window.open() and its arguments.

>+++ b/dom/public/idl/base/nsIDOMJSWindow.idl	Fri Sep 05 16:57:05 2008 -0700
>+   * This is the scriptable versions of 

"version"

>+++ b/dom/src/base/nsGlobalWindow.cpp	Fri Sep 05 16:57:05 2008 -0700
>@@ -4962,59 +4977,7 @@ nsGlobalWindow::Open(const nsAString& aU
>-                      nsnull,            // aJSCallerContext
...
>+                      nsContentUtils::GetCurrentJSContext(), // aJSCallerContext

Don't we want to keep passing null like we used to?

r+sr=bzbarsky with those fixed.  This is good stuff!

Comment 7

[Johnny Stenback (:jst)]

Stage 2b landed. http://hg.mozilla.org/mozilla-central/rev/89f146305b76

Comment 8

[Nochum Sossonko [:Natch]]

Is this RESO FIXED now?

Comment 9

[Johnny Stenback (:jst)]

Yup, closing out this bug, we'll open new bugs on more stuff as it comes up.