Add nsHTMLStyleSheet::SizeOf()

RESOLVED FIXED in mozilla8

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

Trunk
mozilla8
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

6 years ago
Created attachment 542573 [details] [diff] [review]
Patch v1
Attachment #542573 - Flags: feedback?(bzbarsky)
(Assignee)

Comment 1

6 years ago
Created attachment 542633 [details] [diff] [review]
Patch v2

Trying something...
Attachment #542573 - Attachment is obsolete: true
Attachment #542633 - Flags: review?(bzbarsky)
Attachment #542573 - Flags: feedback?(bzbarsky)
(Assignee)

Updated

6 years ago
Whiteboard: [needs review]
(Assignee)

Updated

6 years ago
Blocks: 667887

Comment 2

6 years ago
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+
(Assignee)

Comment 3

6 years ago
(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
(Assignee)

Comment 4

6 years ago
Created attachment 543955 [details] [diff] [review]
Patch v3

r=bz
Attachment #542633 - Attachment is obsolete: true
(Assignee)

Comment 5

6 years ago
Created attachment 543956 [details] [diff] [review]
Patch v3
Attachment #543955 - Attachment is obsolete: true
(Assignee)

Comment 6

6 years ago
Oups, comment 3 was expecting a reply from you Boris but you were not in CC ;)
Version: 7 Branch → Trunk

Comment 7

6 years ago
>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.
(Assignee)

Updated

6 years ago
Flags: in-testsuite-
Whiteboard: [needs review] → [inbound]
(Assignee)

Updated

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.