682264 - Make EventListenerManager participate to the DOM Memory Reporter

Status: RESOLVED FIXED

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

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 2

[Mounir Lamouri (: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?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 556076 [details] [diff] [review]
patch

r=me

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/906413a7ae6b