[webkit-reviews] review granted: [Bug 39621] Extreme memory growth on DOM Hanoi test : [Attachment 57027] now with a saved ChangeLog

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted 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 57027: now with a saved ChangeLog
https://bugs.webkit.org/attachment.cgi?id=57027&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
>  {
>      ASSERT(hasTagName(textareaTag));
>      setFormControlValueMatchesRenderer(true);
> -    notifyFormStateChanged(this);
>  }

This patch still does not explain why this change is OK. The WebKit part of the
patch removes the Mac OS X code entirely, which does two things:

    1) Makes it clear this change is OK on Mac OS X.
    2) Does some of the cleanup of this unused feature.

But this does not prove that this change is OK for non-Mac-OS-X versions of
WebKit, nor does it do all the cleanup that's possible. I think it's fine to
remove this unused function call, but I don't see why you'd want to do some but
not all of the cleanup at the same time.

And this code change seems to have an effect on Chromium.
ChromeClientImpl::formStateDidChange for chromium calls
didUpdateCurrentHistoryItem. That code is probably incorrect, but this code
change will mean it doesn't get called any more.

I'm going to say r=me if you keep the code in inside a PLATFORM(CHROMIUM)
ifdef, leaving a known but in the Chromium case that can be addressed next.
Then we should clean all this out entirely as quickly as possible.