Closed Bug 242111 Opened 21 years ago Closed 21 years ago

decomify nsPIDOMWindow

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bryner, Assigned: bryner)

References

Details

Attachments

(3 files)

In addition to "standard" deCOMtamination, I'm also going to make nsPIDOMWindow
inherit from nsIDOMWindowInternal, to avoid extra QI'ing.
Attached patch patchSplinter Review
Attached patch diff -w patchSplinter Review
Attachment #147340 - Flags: superreview?(jst)
Attachment #147340 - Flags: review?(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.
Attachment #147340 - Flags: superreview?(jst)
Attachment #147340 - Flags: superreview+
Attachment #147340 - Flags: review?(jst)
Attachment #147340 - Flags: review+
Attached patch new patchSplinter Review
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?
Attachment #147438 - Flags: review?(jst)
Comment on attachment 147438 [details] [diff] [review]
new patch

Looks good.
Attachment #147438 - Flags: superreview+
Attachment #147438 - Flags: review?(jst)
Attachment #147438 - Flags: review+
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.
checked in
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Depends on: 296300
I think this caused bug 296300
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: