Last Comment Bug 693172 - Move all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache, and then merge the classes
: Move all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache...
Status: RESOLVED FIXED
[mentor=khuey][lang=c++]
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla13
Assigned To: Edmund Wong (:ewong)
:
Mentors:
http://mxr.mozilla.org/mozilla-centra...
Depends on:
Blocks: 725312
  Show dependency treegraph
 
Reported: 2011-10-09 08:59 PDT by Kyle Huey [:khuey] (khuey@mozilla.com)
Modified: 2012-02-09 13:12 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (49.77 KB, patch)
2011-12-26 19:25 PST, Edmund Wong (:ewong)
no flags Details | Diff | Review
Moved all subclasses of nsDOMEventTargetWrapperCache to nsDOMEventTargetHelper and merged the classes. (43.35 KB, patch)
2011-12-28 01:26 PST, Edmund Wong (:ewong)
bugs: review+
khuey: feedback+
Details | Diff | Review
Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (v2) (43.46 KB, patch)
2012-01-08 04:48 PST, Edmund Wong (:ewong)
no flags Details | Diff | Review
Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (v3) (49.77 KB, patch)
2012-01-09 07:04 PST, Edmund Wong (:ewong)
bugs: review-
Details | Diff | Review
Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (v4) (46.39 KB, patch)
2012-01-12 05:12 PST, Edmund Wong (:ewong)
bugs: review-
Details | Diff | Review
Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (v5) (56.48 KB, patch)
2012-02-02 20:06 PST, Edmund Wong (:ewong)
no flags Details | Diff | Review
Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (v6) (72.74 KB, patch)
2012-02-03 00:37 PST, Edmund Wong (:ewong)
bugs: review+
Details | Diff | Review

Description Kyle Huey [:khuey] (khuey@mozilla.com) 2011-10-09 08:59:58 PDT
My understanding is that the long term plan is to wrapper cache everything.  This moves us further down that path.
Comment 1 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-12-19 04:44:56 PST
This might be a good second or third bug for somebody who is interested in poking around the DOM.
Comment 2 Edmund Wong (:ewong) 2011-12-26 19:25:03 PST
Created attachment 584367 [details] [diff] [review]
Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes.
Comment 3 Edmund Wong (:ewong) 2011-12-28 01:26:12 PST
Created attachment 584538 [details] [diff] [review]
Moved all subclasses of nsDOMEventTargetWrapperCache to nsDOMEventTargetHelper and merged the classes.

Over irc, khuey suggested I move the subclasses of nsDOMEventTargetWrapperCache to nsDOMEventTargetHelper and then merge the two classes.
Comment 4 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-01-02 13:44:37 PST
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.
Comment 5 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-01-07 12:31:08 PST
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.
Comment 6 Edmund Wong (:ewong) 2012-01-08 04:48:17 PST
Created attachment 586791 [details] [diff] [review]
Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (v2)
Comment 7 Edmund Wong (:ewong) 2012-01-09 07:04:33 PST
Created attachment 586987 [details] [diff] [review]
Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (v3)
Comment 8 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-01-09 09:43:09 PST
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
Comment 9 Edmund Wong (:ewong) 2012-01-12 05:12:05 PST
Created attachment 588007 [details] [diff] [review]
Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (v4)
Comment 10 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-01-18 11:20:11 PST
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.
Comment 11 Edmund Wong (:ewong) 2012-02-02 20:06:42 PST
Created attachment 594063 [details] [diff] [review]
Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (v5)
Comment 12 Edmund Wong (:ewong) 2012-02-03 00:37:37 PST
Created attachment 594080 [details] [diff] [review]
Moved all subclasses of nsDOMEventTargetHelper to nsDOMEventTargetWrapperCache and then merge the classes. (v6)

Unbitrotted patch.
Comment 13 Ed Morley [:emorley] 2012-02-08 09:00:52 PST
https://hg.mozilla.org/mozilla-central/rev/bc6a70cdc0e7
Comment 14 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-02-09 12:42:55 PST
Um, did this cause leaks. I'm seeing lots of XHR objects in my CC graph.
Comment 15 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-02-09 13:12:30 PST
Though, could be also this horrible network connection and tons of tbpl tabs open.
Investigating.

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