Last Comment Bug 676048 - Report RuleCascadeData and friends in about:memory
: Report RuleCascadeData and friends in about:memory
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla8
Assigned To: Kyle Huey [:khuey] (khuey@mozilla.com)
:
Mentors:
Depends on: 702984
Blocks: 671299 676057
  Show dependency treegraph
 
Reported: 2011-08-02 12:12 PDT by Kyle Huey [:khuey] (khuey@mozilla.com)
Modified: 2011-11-16 09:42 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (6.21 KB, patch)
2011-08-02 12:12 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
bzbarsky: review+
Details | Diff | Review
Patch w/review comments (6.72 KB, patch)
2011-08-02 13:19 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
khuey: review+
Details | Diff | Review

Description Kyle Huey [:khuey] (khuey@mozilla.com) 2011-08-02 12:12:50 PDT
Created attachment 550155 [details] [diff] [review]
Patch

Add up lots of hashtables and arrays
Comment 1 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-08-02 12:35:34 PDT
Comment on attachment 550155 [details] [diff] [review]
Patch

> +  // We don't bother enumerating this one since all it does is hold a ref to
> an atom

Add a comment in the entry class to fix that if the entry class ever changes?

Please file a followup bug on me to nuke AttributeSelectorEntry and just use AtomSelectorEntry for the attributes table?  I have no idea why that wasn't done in bug 645491 or some followup...  I think it was because I had some other changes to revamp all this selector hashtable stuff but those never landed.

Please use static_cast, not C-style casts.

>+  for (RuleCascadeData * const *cascadep = &mRuleCascades, *cascade;
>+       (cascade = *cascadep); cascadep = &cascade->mNext) {

How about:

  for (RuleCascadeData* cascade = mRuleCascades; cascade; 
       cascade = cascade->mNext) {

It's not like you have to _modify_ the list, unlike RefreshRuleCascades.

r=me with those changes.
Comment 2 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-08-02 13:19:34 PDT
Created attachment 550180 [details] [diff] [review]
Patch w/review comments

Review comments addressed.  Turns out we should traverse those other hashtables, since RuleHashTagTableEntry inherits from RuleHashTableEntry, we were traversing.
Comment 3 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-08-02 13:22:41 PDT
http://hg.mozilla.org/mozilla-central/rev/f185f48d35dc
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-08-02 13:33:57 PDT
Comment on attachment 550180 [details] [diff] [review]
Patch w/review comments

This could still use a comment on RuleHashTagTableEntry that says that if any members are added the traverse code will need changing...
Comment 5 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-08-10 08:13:03 PDT
Added a few more comments.

http://hg.mozilla.org/mozilla-central/rev/e1bc7f08fa69

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