84106 2012-04-16 17:45:31 -0700 Auto-sized frames may be taller than expected 2012-04-16 22:09:23 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 0 aburago webkit-unassigned webkit.review.bot oldest_to_newest 603626 0 aburago 2012-04-16 17:45:31 -0700 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. 603642 1 137445 aburago 2012-04-16 17:53:27 -0700 Created attachment 137445 Patch 603649 2 137445 aburago 2012-04-16 17:57:09 -0700 Comment on attachment 137445 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); 603653 3 137448 aburago 2012-04-16 18:02:27 -0700 Created attachment 137448 Patch 603790 4 137448 webkit.review.bot 2012-04-16 22:09:19 -0700 Comment on attachment 137448 Patch Clearing flags on attachment: 137448 Committed r114342: <http://trac.webkit.org/changeset/114342> 603791 5 webkit.review.bot 2012-04-16 22:09:23 -0700 All reviewed patches have been landed. Closing bug. 137445 2012-04-16 17:53:27 -0700 2012-04-16 18:02:23 -0700 Patch bug-84106-20120416175326.patch text/plain 1820 aburago SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDExNDMxNSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDEyLTA0LTAyICBBbmRyZWkg QnVyYWdvICA8YWJ1cmFnb0BjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgQXV0by1zaXplIG1heSBu b3Qgd29yayBvbiBmaXJzdCBsb2FkCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD04Mjk4OQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIE5vIG5ldyB0ZXN0cy4gVGhlIG9ubHkga25vd24gcmVwcm8gc2NlbmFyaW8g aW52b2x2ZXMgdXNpbmcgY2hyb21lIHdpdGggYSB0aGlyZC1wYXJ0eSBleHRlbnNpb24sCisgICAg ICAgIHdoaWNoIG1ha2VzIG1ha2luZyBhIHRlc3Qgb3V0IG9mIGl0IHByb2JsZW1hdGljLgorCisg ICAgICAgICogZG9tL0RvY3VtZW50LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkRvY3VtZW50Ojpp bXBsaWNpdENsb3NlKToKKyAgICAgICAgKiBwYWdlL0ZyYW1lVmlldy5jcHA6CisgICAgICAgIChX ZWJDb3JlOjpGcmFtZVZpZXc6OmF1dG9TaXplSWZFbmFibGVkKToKKwogMjAxMi0wNC0xNiAgTGV2 aSBXZWludHJhdWIgIDxsZXZpd0BjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgTWFrZSBib3JkZXJC b3hSZWN0IHN1Yi1waXhlbCBwcmVjaXNlIGFuZCBhZGQgYSBwaXhlbCBzbmFwcGVkIHZlcnNpb24K SW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BhZ2UvRnJhbWVWaWV3LmNwcAo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBT b3VyY2UvV2ViQ29yZS9wYWdlL0ZyYW1lVmlldy5jcHAJKHJldmlzaW9uIDExNDMxNSkKKysrIFNv dXJjZS9XZWJDb3JlL3BhZ2UvRnJhbWVWaWV3LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMjQzNCwx MiArMjQzNCwxMyBAQCB2b2lkIEZyYW1lVmlldzo6YXV0b1NpemVJZkVuYWJsZWQoKQogICAgIGlm ICghbV9kaWRSdW5BdXRvc2l6ZSkKICAgICAgICAgcmVzaXplKGZyYW1lUmVjdCgpLndpZHRoKCks IG1fbWluQXV0b1NpemUuaGVpZ2h0KCkpOwogCisgICAgSW50U2l6ZSBzaXplID0gZnJhbWVSZWN0 KCkuc2l6ZSgpOworCiAgICAgLy8gRG8gdGhlIHJlc2l6aW5nIHR3aWNlLiBUaGUgZmlyc3QgdGlt ZSBpcyBiYXNpY2FsbHkgYSByb3VnaCBjYWxjdWxhdGlvbiB1c2luZyB0aGUgcHJlZmVycmVkIHdp ZHRoCiAgICAgLy8gd2hpY2ggbWF5IHJlc3VsdCBpbiBhIGhlaWdodCBjaGFuZ2UgZHVyaW5nIHRo ZSBzZWNvbmQgaXRlcmF0aW9uLgogICAgIGZvciAoaW50IGkgPSAwOyBpIDwgMjsgaSsrKSB7CiAg ICAgICAgIC8vIFVwZGF0ZSB2YXJpb3VzIHNpemVzIGluY2x1ZGluZyBjb250ZW50c1NpemUsIHNj cm9sbEhlaWdodCwgZXRjLgogICAgICAgICBkb2N1bWVudC0+dXBkYXRlTGF5b3V0SWdub3JlUGVu ZGluZ1N0eWxlc2hlZXRzKCk7Ci0gICAgICAgIEludFNpemUgc2l6ZSA9IGZyYW1lUmVjdCgpLnNp emUoKTsKICAgICAgICAgaW50IHdpZHRoID0gZG9jdW1lbnRWaWV3LT5taW5QcmVmZXJyZWRMb2dp Y2FsV2lkdGgoKTsKICAgICAgICAgaW50IGhlaWdodCA9IGRvY3VtZW50UmVuZGVyQm94LT5zY3Jv bGxIZWlnaHQoKTsKICAgICAgICAgSW50U2l6ZSBuZXdTaXplKHdpZHRoLCBoZWlnaHQpOwo= 137448 2012-04-16 18:02:27 -0700 2012-04-16 22:09:19 -0700 Patch bug-84106-20120416180226.patch text/plain 1674 aburago SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDExNDMxNSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDEyLTA0LTAyICBBbmRyZWkg QnVyYWdvICA8YWJ1cmFnb0BjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgQXV0by1zaXplZCBmcmFt ZXMgbWF5IGJlIHRhbGxlciB0aGFuIGV4cGVjdGVkCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD04NDEwNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cy4gVGhlIHJlcHJvIHN0ZXBzIHJlcXVp cmUgdXNpbmcgQ2hyb21lIG5vdGlmaWNhdGlvbnMgb24gV2luLgorCisgICAgICAgICogcGFnZS9G cmFtZVZpZXcuY3BwOgorICAgICAgICAoV2ViQ29yZTo6RnJhbWVWaWV3OjphdXRvU2l6ZUlmRW5h YmxlZCk6CisKIDIwMTItMDQtMTYgIExldmkgV2VpbnRyYXViICA8bGV2aXdAY2hyb21pdW0ub3Jn PgogCiAgICAgICAgIE1ha2UgYm9yZGVyQm94UmVjdCBzdWItcGl4ZWwgcHJlY2lzZSBhbmQgYWRk IGEgcGl4ZWwgc25hcHBlZCB2ZXJzaW9uCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9wYWdlL0ZyYW1l Vmlldy5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGFnZS9GcmFtZVZpZXcuY3Bw CShyZXZpc2lvbiAxMTQzMTUpCisrKyBTb3VyY2UvV2ViQ29yZS9wYWdlL0ZyYW1lVmlldy5jcHAJ KHdvcmtpbmcgY29weSkKQEAgLTI0MzQsMTIgKzI0MzQsMTMgQEAgdm9pZCBGcmFtZVZpZXc6OmF1 dG9TaXplSWZFbmFibGVkKCkKICAgICBpZiAoIW1fZGlkUnVuQXV0b3NpemUpCiAgICAgICAgIHJl c2l6ZShmcmFtZVJlY3QoKS53aWR0aCgpLCBtX21pbkF1dG9TaXplLmhlaWdodCgpKTsKIAorICAg IEludFNpemUgc2l6ZSA9IGZyYW1lUmVjdCgpLnNpemUoKTsKKwogICAgIC8vIERvIHRoZSByZXNp emluZyB0d2ljZS4gVGhlIGZpcnN0IHRpbWUgaXMgYmFzaWNhbGx5IGEgcm91Z2ggY2FsY3VsYXRp b24gdXNpbmcgdGhlIHByZWZlcnJlZCB3aWR0aAogICAgIC8vIHdoaWNoIG1heSByZXN1bHQgaW4g YSBoZWlnaHQgY2hhbmdlIGR1cmluZyB0aGUgc2Vjb25kIGl0ZXJhdGlvbi4KICAgICBmb3IgKGlu dCBpID0gMDsgaSA8IDI7IGkrKykgewogICAgICAgICAvLyBVcGRhdGUgdmFyaW91cyBzaXplcyBp bmNsdWRpbmcgY29udGVudHNTaXplLCBzY3JvbGxIZWlnaHQsIGV0Yy4KICAgICAgICAgZG9jdW1l bnQtPnVwZGF0ZUxheW91dElnbm9yZVBlbmRpbmdTdHlsZXNoZWV0cygpOwotICAgICAgICBJbnRT aXplIHNpemUgPSBmcmFtZVJlY3QoKS5zaXplKCk7CiAgICAgICAgIGludCB3aWR0aCA9IGRvY3Vt ZW50Vmlldy0+bWluUHJlZmVycmVkTG9naWNhbFdpZHRoKCk7CiAgICAgICAgIGludCBoZWlnaHQg PSBkb2N1bWVudFJlbmRlckJveC0+c2Nyb2xsSGVpZ2h0KCk7CiAgICAgICAgIEludFNpemUgbmV3 U2l6ZSh3aWR0aCwgaGVpZ2h0KTsK