242111 - decomify nsPIDOMWindow

Status: RESOLVED FIXED

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

Description

[Brian Ryner (not reading)]

In addition to "standard" deCOMtamination, I'm also going to make nsPIDOMWindow
inherit from nsIDOMWindowInternal, to avoid extra QI'ing.

Comment 1

[Brian Ryner (not reading)]

Attachment: patch

Comment 2

[Brian Ryner (not reading)]

Attachment: diff -w patch

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 147340 [details] [diff] [review]
patch

- In nsDocShell::SetupNewViewer():

-	       if (curwin == NS_STATIC_CAST(nsIDOMWindow*, ourFocusedWindow)) {
+	       if (curwin == NS_STATIC_CAST(nsIDOMWindow*, ourWindow)) {

Couldn't you loose that cast?

- In dom/public/base/nsPIDOMWindow.h:

+  nsIChromeEventHandler *mChromeEventHandler; // strong
+  nsIDOMDocument	 *mDocument; // strong
+  nsIDOMElement	 *mFrameElement; // weak
+  nsIURI		 *mOpenerScriptURL; // strong; used to determine
whether to clear scope

Make those strong references nsCOMPtr's, I really don't like the fact that we
now need to manually refcount these members, and having the initializers and
destruction code for these spread out across nsPIDOMWindow and GlobalWindowImpl
kida sucks.

- In GlobalWindowImpl::GetPrivateParent():

+  if (parent) {
+    return NS_STATIC_CAST(GlobalWindowImpl*,
+			   NS_STATIC_CAST(nsIDOMWindow*, parent));

Make that last cast parent.get()?

r+sr=jst with those strong references changed to nsCOMPtr's.

Comment 4

[Brian Ryner (not reading)]

Attachment: new patch

Using nsCOMPtrs for the strong references required me to pull in the headers
for those interfaces, so there are some additional REQUIRES-type changes here.

I also moved all of the initializers into the GlobalWindow ctor so they aren't
split.

Still look ok jst?

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 147438 [details] [diff] [review]
new patch

Looks good.

Comment 6

[R.K.Aa.]

Something checked in after 2004-05-03 14:32 but before 2004-05-03 22:37 caused
bug 242560. (Focus stuck - scrollwheeling stuck - ctrl-keys dead - clicking in
page doesn't set focus.) I *think* this is the most suspicious checkin in the
given timeframe.

Comment 7

[Brian Ryner (not reading)]

checked in

Comment 8

[Boris Zbarsky [:bzbarsky]]

I think this caused bug 296300