Auto-sized frames may be taller than expected

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Andrei Burago]

When auto-size code runs (FrameView::autoSizeIfEnabled), it makes two passes, the first one to determine the min width, and the second one to get the correct height for this width. During the initial loading time, a check is made to make sure the dimensions only increase, to avoid jittery behavior and wrong sizes because of content that is not ready. For this, we compare the newly computed size with the previous one, and only resize if the new size is larger.
However, the current logic incorrectly compares the result (in particular, height) of the second pass not with the result of the  original size but with with result of the first pass. This makes little sense and can result in frame not being autosized during load or in it becoming higher than required. 
One can see the wrong behavior on www.210computing.com/google/chrome_notifications.html However, because timing is involved, the behavior is only exhibited under Windows.

[Andrei Burago]

Created attachment 137445 [details]
Patch

[Andrei Burago]

Comment on attachment 137445 [details]
Patch

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 114315)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,18 @@
>[phone]  Andrei Burago  <[e-mail]>
>+
>+        Auto-size may not work on first load
>+        https://bugs.webkit.org/show_bug.cgi?id=84106
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No new tests. The repro steps require use of Chrome notifications on Win.
>+        
>+
>+        * page/FrameView.cpp:
>+        (WebCore::FrameView::autoSizeIfEnabled):
>+
> 2012-04-16  Levi Weintraub  <[e-mail]>
> 
>         Make borderBoxRect sub-pixel precise and add a pixel snapped version
>Index: Source/WebCore/page/FrameView.cpp
>===================================================================
>--- Source/WebCore/page/FrameView.cpp	(revision 114315)
>+++ Source/WebCore/page/FrameView.cpp	(working copy)
>@@ -2434,12 +2434,13 @@ void FrameView::autoSizeIfEnabled()
>     if (!m_didRunAutosize)
>         resize(frameRect().width(), m_minAutoSize.height());
> 
>+    IntSize size = frameRect().size();
>+
>     // Do the resizing twice. The first time is basically a rough calculation using the preferred width
>     // which may result in a height change during the second iteration.
>     for (int i = 0; i < 2; i++) {
>         // Update various sizes including contentsSize, scrollHeight, etc.
>         document->updateLayoutIgnorePendingStylesheets();
>-        IntSize size = frameRect().size();
>         int width = documentView->minPreferredLogicalWidth();
>         int height = documentRenderBox->scrollHeight();
>         IntSize newSize(width, height);

[Andrei Burago]

Created attachment 137448 [details]
Patch

[WebKit Review Bot]

Comment on attachment 137448 [details]
Patch

Clearing flags on attachment: 137448

Committed r114342: <http://trac.webkit.org/changeset/114342>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.