[webkit-reviews] review denied: [Bug 39621] Extreme memory growth on DOM Hanoi test : [Attachment 57024] proposed fix

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 39621: Extreme memory growth on DOM Hanoi test
https://bugs.webkit.org/show_bug.cgi?id=39621

Attachment 57024: proposed fix
https://bugs.webkit.org/attachment.cgi?id=57024&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   * html/HTMLTextAreaElement.cpp:
(WebCore::HTMLTextAreaElement::HTMLTextAreaElement):
> +	   Don't call notifyFormStateChanged() - since the element starts with
refcount 0, it's not
> +	   safe to call functions that are likely to create temporary wrappers
(wrapper destructor
> +	   would bring refcount back to 0, and destroy HTMLTextAreaElement from
within its constructor).

This could also be resolved by making HTMLTextAreaElement start with a
reference count of 1. We do plan to do this eventually for all DOM classes, and
we could do this one ahead of time.

I assume that the real reason it's OK not to call this is that people don't
need the call. Your change log entry talks about why the call caused problems,
but does not address why it's OK to remove the call.

> +	   Need a short description and bug URL (OOPS!)

I assume that your comment here was going to say something about his private
WebUIDelegate method being no longer needed since it's no longer used by Safari
nor any other client of the Mac OS X version of WebKit.

Perhaps we could remove formStateDidChange entirely. Does any platform use it
in WebKit?

Generally speaking the issue of autoreleased objects could be addressed by
adding autorelease pools as well.

I'm going to say review- on this, but really the code change might be OK if you
put clearer comments in