Make EventListenerManager participate to the DOM Memory Reporter

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Comment hidden (empty)
OS: Linux → All
Hardware: x86_64 → All
Version: unspecified → Trunk
(Assignee)

Comment 1

6 years ago
Created attachment 556029 [details] [diff] [review]
patch
Attachment #556029 - Flags: review?(mounir)
Comment on attachment 556029 [details] [diff] [review]
patch

Review of attachment 556029 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/public/nsINode.h
@@ +281,5 @@
>  
>  // IID for the nsINode interface
>  #define NS_INODE_IID \
> +{ 0x5572c8a9, 0xbda9, 0x4b78, \
> +  { 0xb4, 0x1a, 0xdb, 0x1a, 0x83, 0xef, 0x53, 0x7e } }

Do you really need that?

@@ +294,5 @@
>  {
>  public:
>    NS_DECLARE_STATIC_IID_ACCESSOR(NS_INODE_IID)
>  
> +  virtual PRInt64 SizeOf() const;

Please use NS_DECL_DOM_MEMORY_REPORTER_SIZEOF

::: content/base/src/nsGenericElement.cpp
@@ +5409,5 @@
> +  nsEventListenerManager* elm =
> +    const_cast<nsINode*>(this)->GetListenerManager(PR_FALSE);
> +  if (elm) {
> +    size += elm->SizeOf();
> +  }

nit: I would have put a blank line before nsEventListenerManager* elm and juste here too (after the }).

::: content/events/src/nsEventListenerManager.cpp
@@ +981,5 @@
> +PRInt64
> +nsEventListenerManager::SizeOf() const
> +{
> +  PRInt64 size = sizeof(*this);
> +  const nsListenerStruct* ls;

Why a pointer? I think a reference would be fine here.

@@ +991,5 @@
> +    if (jsl) {
> +      size += jsl->SizeOf();
> +    }
> +  }
> +  return size;

I don't think I'm the best suited to review that method. That seems correct but I wouldn't be able to guarantee nothing is missing.

::: dom/base/nsIJSEventListener.h
@@ +89,5 @@
>    // there is already a handler!  The handler must already be bound to
>    // the right target.
>    virtual void SetHandler(void *aHandler) = 0;
>  
> +  virtual PRInt64 SizeOf() const = 0;

None of the members are owned by this class?
Attachment #556029 - Flags: review?(mounir) → review+
(Assignee)

Comment 3

6 years ago
Created attachment 556076 [details] [diff] [review]
patch
Attachment #556076 - Flags: review?(bzbarsky)
Comment on attachment 556076 [details] [diff] [review]
patch

r=me
Attachment #556076 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 5

6 years ago
http://hg.mozilla.org/mozilla-central/rev/906413a7ae6b
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.