Closed Bug 693172 Opened 13 years ago Closed 13 years ago

Move all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache, and then merge the classes

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla13

People

(Reporter: khuey, Assigned: ewong)

References

(
URL
)

Details

(Whiteboard: [mentor=khuey][lang=c++])

Attachments

(1 file, 6 obsolete files)

Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (v6)
72.74 KB, patch
smaug
: review+
Details | Diff | Splinter Review
My understanding is that the long term plan is to wrapper cache everything. This moves us further down that path.
This might be a good second or third bug for somebody who is interested in poking around the DOM.
URL: http://mxr.mozilla.org/mozilla-centra...
Whiteboard: [mentor=khuey][lang=c++]
Over irc, khuey suggested I move the subclasses of nsDOMEventTargetWrapperCache to nsDOMEventTargetHelper and then merge the two classes.
Assignee: nobody → ewong
Attachment #584367 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #584367 - Flags: review?(khuey)
Attachment #584538 - Flags: review?(khuey)
Comment on attachment 584538 [details] [diff] [review] Moved all subclasses of nsDOMEventTargetWrapperCache to nsDOMEventTargetHelper and merged the classes. Looks pretty good to me. Going to toss it over to smaug for the official review though, since he owns event handling.
Attachment #584538 - Flags: review?(khuey)
Attachment #584538 - Flags: review?(bugs)
Attachment #584538 - Flags: feedback+
Comment on attachment 584538 [details] [diff] [review] Moved all subclasses of nsDOMEventTargetWrapperCache to nsDOMEventTargetHelper and merged the classes. Looks ok, but I think there has been some changes to this code so could you update the patch.
Attachment #584538 - Flags: review?(bugs) → review+
Attachment #586791 - Flags: review?(bugs)
Comment on attachment 586987 [details] [diff] [review] Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (v3) This makes the hg annotation messy. Could you just update the previous patch to contain the changes done to the nsDOMEventTargetWrapperCache so that the code is copied to nsDOMEventTargetHelper
Attachment #586987 - Flags: review?(bugs) → review-
Comment on attachment 588007 [details] [diff] [review] Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (v4) You're not moving the cycle collection related parts for *Cache to the *Helper.
Attachment #588007 - Flags: review?(bugs) → review-
Unbitrotted patch.
Attachment #594063 - Attachment is obsolete: true
Attachment #594063 - Flags: review?(bugs)
Attachment #594080 - Flags: review?(bugs)
Attachment #594080 - Flags: review?(bugs) → review+
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/bc6a70cdc0e7
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Um, did this cause leaks. I'm seeing lots of XHR objects in my CC graph.
Though, could be also this horrible network connection and tons of tbpl tabs open. Investigating.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: