89258 2012-06-15 18:02:33 -0700 [Chromium] Ensure layout has happened before calling into hasNonEmptyBoundingBox() 2012-06-21 14:26:34 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 isherman isherman eric fishd isherman jamesr tkent webkit.review.bot oldest_to_newest 650659 0 isherman 2012-06-15 18:02:33 -0700 [Chromium] Ensure layout has happened before calling into hasNonEmptyBoundingBox() 650660 1 147929 isherman 2012-06-15 18:03:20 -0700 Created attachment 147929 Patch 650668 2 147929 jamesr 2012-06-15 18:13:27 -0700 Comment on attachment 147929 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=147929&action=review > Source/WebKit/chromium/src/WebNode.cpp:218 > + m_private->document()->updateLayout(); most code in WebCore of this nature calls updateLayoutIgnorePendingStylesheets() - it deals with a m_hasNodesWithPlaceholderStyle case that seems important 650693 3 147939 isherman 2012-06-15 19:32:52 -0700 Created attachment 147939 Patch 650694 4 isherman 2012-06-15 19:33:46 -0700 (In reply to comment #2) > (From update of attachment 147929 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=147929&action=review > > > Source/WebKit/chromium/src/WebNode.cpp:218 > > + m_private->document()->updateLayout(); > > most code in WebCore of this nature calls updateLayoutIgnorePendingStylesheets() - it deals with a m_hasNodesWithPlaceholderStyle case that seems important Ok, switched to using that method. 651846 5 147939 webkit.review.bot 2012-06-18 16:04:01 -0700 Comment on attachment 147939 Patch Clearing flags on attachment: 147939 Committed r120637: <http://trac.webkit.org/changeset/120637> 651847 6 webkit.review.bot 2012-06-18 16:04:16 -0700 All reviewed patches have been landed. Closing bug. 652972 7 eric 2012-06-19 16:44:22 -0700 What are the side-effects of this change? Also I'm surprised by the lack of testing? is testing impossible/impractical? 654882 8 isherman 2012-06-21 14:26:34 -0700 (In reply to comment #7) > What are the side-effects of this change? Side-effects include correctness improvements for callers of the two affected methods, and presumably some minimal performance degradations to compensate. (The impact on performance appears not to be causing any issues for existing clients.) > Also I'm surprised by the lack of testing? is testing impossible/impractical? AFAIK, the Chromium WebKit API is generally not tested within the WebKit project. However, the correctness issue was caught by a Chromium test, so there is some test coverage. 147929 2012-06-15 18:03:20 -0700 2012-06-15 19:32:48 -0700 Patch bug-89258-20120615180319.patch text/plain 1296 isherman U3VidmVyc2lvbiBSZXZpc2lvbjogMTIwNDU1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L2No cm9taXVtL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCmluZGV4 IGJlZTk2MzhiZjNiNDM1M2FhZGFmMDg2YWU0MDM0ZDUzYTRmZDA1NWQuLjIzZTkxNmFjNTQwNzIy NjZhNjRjYzE0MzAxMWQyMThjMDg4ZjhjNzkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvY2hy b21pdW0vQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMTMgQEAKKzIwMTItMDYtMTUgIElseWEgU2hlcm1hbiAgPGlzaGVybWFuQGNocm9t aXVtLm9yZz4KKworICAgICAgICBbQ2hyb21pdW1dIEVuc3VyZSBsYXlvdXQgaGFzIGhhcHBlbmVk IGJlZm9yZSBjYWxsaW5nIGludG8gaGFzTm9uRW1wdHlCb3VuZGluZ0JveCgpCisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04OTI1OAorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogc3JjL1dlYk5vZGUuY3BwOgor ICAgICAgICAoV2ViS2l0OjpXZWJOb2RlOjpoYXNOb25FbXB0eUJvdW5kaW5nQm94KTogRm9yY2Ug bGF5b3V0IGJlZm9yZSBjaGVja2luZyB0aGUgYm91bmRpbmcgYm94CisKIDIwMTItMDYtMTUgIFNh bWkgS3lvc3RpbGEgIDxza3lvc3RpbEBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgW2Nocm9taXVt XSBBbGxvdyBzY3JvbGxpbmcgbm9uLXJvb3QgbGF5ZXJzIGluIHRoZSBjb21wb3NpdG9yIHRocmVh ZApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViTm9kZS5jcHAgYi9T b3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJOb2RlLmNwcAppbmRleCA2OTkwOGM5Yjc0MGNk MmYzODg3MDBlNDFiNzZlZGQxZmNlMzg4NzdlLi4xZDA2ZTQ3YjZmNWJkZTQ1ZTQ5MTZjMzUwZGM2 NTgwNDcwYTgyYzYwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJO b2RlLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJOb2RlLmNwcApAQCAt MjE1LDYgKzIxNSw3IEBAIFdlYkVsZW1lbnQgV2ViTm9kZTo6cm9vdEVkaXRhYmxlRWxlbWVudCgp IGNvbnN0CiAKIGJvb2wgV2ViTm9kZTo6aGFzTm9uRW1wdHlCb3VuZGluZ0JveCgpIGNvbnN0CiB7 CisgICAgbV9wcml2YXRlLT5kb2N1bWVudCgpLT51cGRhdGVMYXlvdXQoKTsKICAgICByZXR1cm4g bV9wcml2YXRlLT5oYXNOb25FbXB0eUJvdW5kaW5nQm94KCk7CiB9CiAK 147939 2012-06-15 19:32:52 -0700 2012-06-18 16:04:01 -0700 Patch bug-89258-20120615193251.patch text/plain 1733 isherman U3VidmVyc2lvbiBSZXZpc2lvbjogMTIwNDgyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L2No cm9taXVtL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCmluZGV4 IGJlZTk2MzhiZjNiNDM1M2FhZGFmMDg2YWU0MDM0ZDUzYTRmZDA1NWQuLjUxOWRlNzU5ODk0NzA2 YmNkMjk0Y2NkNjRjNGIyYTRmZTg3ZjE3MWQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvY2hy b21pdW0vQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTItMDYtMTUgIElseWEgU2hlcm1hbiAgPGlzaGVybWFuQGNocm9t aXVtLm9yZz4KKworICAgICAgICBbQ2hyb21pdW1dIEVuc3VyZSBsYXlvdXQgaGFzIGhhcHBlbmVk IGJlZm9yZSBjYWxsaW5nIGludG8gaGFzTm9uRW1wdHlCb3VuZGluZ0JveCgpCisgICAgICAgIEFs c28gdXBkYXRlIGV4aXN0aW5nIGNvZGUgdG8gdXNlIHVwZGF0ZUxheW91dElnbm9yZVBlbmRpbmdT dHlsZXNoZWV0cygpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD04OTI1OAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgICogc3JjL1dlYk5vZGUuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJOb2RlOjppc0ZvY3Vz YWJsZSk6IFVzZSB1cGRhdGVMYXlvdXRJZ25vcmVQZW5kaW5nU3R5bGVzaGVldHMoKQorICAgICAg ICAoV2ViS2l0OjpXZWJOb2RlOjpoYXNOb25FbXB0eUJvdW5kaW5nQm94KTogRm9yY2UgbGF5b3V0 IGJlZm9yZSBjaGVja2luZyB0aGUgYm91bmRpbmcgYm94CisKIDIwMTItMDYtMTUgIFNhbWkgS3lv c3RpbGEgIDxza3lvc3RpbEBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgW2Nocm9taXVtXSBBbGxv dyBzY3JvbGxpbmcgbm9uLXJvb3QgbGF5ZXJzIGluIHRoZSBjb21wb3NpdG9yIHRocmVhZApkaWZm IC0tZ2l0IGEvU291cmNlL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViTm9kZS5jcHAgYi9Tb3VyY2Uv V2ViS2l0L2Nocm9taXVtL3NyYy9XZWJOb2RlLmNwcAppbmRleCA2OTkwOGM5Yjc0MGNkMmYzODg3 MDBlNDFiNzZlZGQxZmNlMzg4NzdlLi40OWRjZGQ5OGVjYzkxZmQ4MGQyM2FhZjlmYTI2MWM4ZGJk ZjhlZDVlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJOb2RlLmNw cAorKysgYi9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJOb2RlLmNwcApAQCAtMTUzLDcg KzE1Myw3IEBAIGJvb2wgV2ViTm9kZTo6aXNUZXh0Tm9kZSgpIGNvbnN0CiAKIGJvb2wgV2ViTm9k ZTo6aXNGb2N1c2FibGUoKSBjb25zdAogewotICAgIG1fcHJpdmF0ZS0+ZG9jdW1lbnQoKS0+dXBk YXRlTGF5b3V0KCk7CisgICAgbV9wcml2YXRlLT5kb2N1bWVudCgpLT51cGRhdGVMYXlvdXRJZ25v cmVQZW5kaW5nU3R5bGVzaGVldHMoKTsKICAgICByZXR1cm4gbV9wcml2YXRlLT5pc0ZvY3VzYWJs ZSgpOwogfQogCkBAIC0yMTUsNiArMjE1LDcgQEAgV2ViRWxlbWVudCBXZWJOb2RlOjpyb290RWRp dGFibGVFbGVtZW50KCkgY29uc3QKIAogYm9vbCBXZWJOb2RlOjpoYXNOb25FbXB0eUJvdW5kaW5n Qm94KCkgY29uc3QKIHsKKyAgICBtX3ByaXZhdGUtPmRvY3VtZW50KCktPnVwZGF0ZUxheW91dEln bm9yZVBlbmRpbmdTdHlsZXNoZWV0cygpOwogICAgIHJldHVybiBtX3ByaXZhdGUtPmhhc05vbkVt cHR5Qm91bmRpbmdCb3goKTsKIH0KIAo=