[webkit-reviews] review denied: [Bug 82566] RenderTextControlSingleLine::scrollWidth/Height/Left/Top should not call back to DOM tree : [Attachment 137437] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Tien-Ren Chen
<trchen at chromium.org>'s request for review:
Bug 82566: RenderTextControlSingleLine::scrollWidth/Height/Left/Top should not
call back to DOM tree
https://bugs.webkit.org/show_bug.cgi?id=82566

Attachment 137437: Patch
https://bugs.webkit.org/attachment.cgi?id=137437&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=137437&action=review


> Source/WebCore/ChangeLog:9
> +	   Calling to Element::scrollWidth/scrolHeight/scrollLeft/scrollTop/
> +	   setScrollLeft/setScrollTop will call
> +	   Document::updateLayoutIgnorePendingStyleSheets, which can trigger
> +	   unexpected layout update.

How is that possible? When we're calling scrollWidth on
RenderTextControlSingleLine via Element::scrollWidth(),
we should have already called updateLayoutIgnorePendingStylesheets().
The call to updateLayoutIgnorePendingStyleSheets() here should be no-op. Who is
the caller?
We need to fix that caller to call updateLayoutIgnorePendingStyleSheets first.
r- because this is a wrong fix.

Do we have a crash report / stack trace for this?

> Source/WebCore/ChangeLog:11
> +	   Reviewed by NOBODY (OOPS!).

This line should appear before the long description.

> Source/WebCore/ChangeLog:13
> +	   No new tests. No change in behavior.

There definitely is a behavior change. Please revise.