Bug 84106 - Auto-sized frames may be taller than expected
Summary: Auto-sized frames may be taller than expected
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-16 17:45 PDT by Andrei Burago
Modified: 2012-04-16 22:09 PDT (History)
1 user (show)

See Also:


Attachments
Patch (1.78 KB, patch)
2012-04-16 17:53 PDT, Andrei Burago
no flags Details | Formatted Diff | Diff
Patch (1.63 KB, patch)
2012-04-16 18:02 PDT, Andrei Burago
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrei Burago 2012-04-16 17:45:31 PDT
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.
Comment 1 Andrei Burago 2012-04-16 17:53:27 PDT
Created attachment 137445 [details]
Patch
Comment 2 Andrei Burago 2012-04-16 17:57:09 PDT
Comment on attachment 137445 [details]
Patch

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 114315)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,18 @@
>+2012-04-02  Andrei Burago  <aburago@chromium.org>
>+
>+        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  <leviw@chromium.org>
> 
>         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);
Comment 3 Andrei Burago 2012-04-16 18:02:27 PDT
Created attachment 137448 [details]
Patch
Comment 4 WebKit Review Bot 2012-04-16 22:09:19 PDT
Comment on attachment 137448 [details]
Patch

Clearing flags on attachment: 137448

Committed r114342: <http://trac.webkit.org/changeset/114342>
Comment 5 WebKit Review Bot 2012-04-16 22:09:23 PDT
All reviewed patches have been landed.  Closing bug.