Closed Bug 668013 Opened 9 years ago Closed 9 years ago

Add nsHTMLStyleSheet::SizeOf()

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: mounir, Assigned: mounir)

References

Details

Attachments

(1 file, 3 obsolete files)

Attached patch Patch v1 (obsolete) — Splinter Review
No description provided.
Attachment #542573 - Flags: feedback?(bzbarsky)
Attached patch Patch v2 (obsolete) — Splinter Review
Trying something...
Attachment #542573 - Attachment is obsolete: true
Attachment #542633 - Flags: review?(bzbarsky)
Attachment #542573 - Flags: feedback?(bzbarsky)
Whiteboard: [needs review]
Blocks: 667887
Comment on attachment 542633 [details] [diff] [review]
Patch v2

I wonder whether the |size += mFoo ? sizeof(*mFoo.get()) : 0| pattern could just use a macro...

>+  if (entry->mAttributes) {

mAttributes is never null.

The sizeof(nsIURI) thing there is of limited utility, for what it's worth.  It'll just claim to be one word, I bet.

>+    size += sizeof(MappedAttrTable_Ops);

Why?

>+    size += sizeof(*entry->mAttributes);

And I guess a followup to get nsMappedAttributes::SizeOf() ?

r=me with the nits fixed (in particular, take out or really explain the _Ops thing).
Attachment #542633 - Flags: review?(bzbarsky) → review+
(In reply to comment #2)
> I wonder whether the |size += mFoo ? sizeof(*mFoo.get()) : 0| pattern could
> just use a macro...

I agree. Though, I will keep that for a follow-up. There are a few situations that could use a macro or a helper method.

> >+  if (entry->mAttributes) {
> 
> mAttributes is never null.

I've put an NS_ASSERTION.

> The sizeof(nsIURI) thing there is of limited utility, for what it's worth. 
> It'll just claim to be one word, I bet.

Do you mean:
> size += mURL ? sizeof(*mURL.get()) : 0;

> >+    size += sizeof(MappedAttrTable_Ops);
> 
> Why?

Removed. Sorry, that was stupid.

> >+    size += sizeof(*entry->mAttributes);
> 
> And I guess a followup to get nsMappedAttributes::SizeOf() ?

bug 667887
Version: Trunk → 7 Branch
Attached patch Patch v3 (obsolete) — Splinter Review
r=bz
Attachment #542633 - Attachment is obsolete: true
Attached patch Patch v3Splinter Review
Attachment #543955 - Attachment is obsolete: true
Oups, comment 3 was expecting a reply from you Boris but you were not in CC ;)
Version: 7 Branch → Trunk
>Do you mean:
>> size += mURL ? sizeof(*mURL.get()) : 0;

Yes.  *mURL.get() is of type nsIURI, which is a class with nothing but a vtable and only one vtable pointer.

And come to think of it, the mURL of an nsHTMLStyleSheet is always the URI of the document.  So we should just not count it here at all (apart from the space the mURL COMPtr takes in nsHTMLStyleSheet, which sizeof(*this) covered.
Flags: in-testsuite-
Whiteboard: [needs review] → [inbound]
Flags: in-testsuite-
this has been backed out by ehsan due to bustage with all the other changesets in the same push
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/1ec9c2051c07
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.