74974 2011-12-20 16:41:07 -0800 [chromium] m_triggerIdlePaints not reset after a compositeAndReadback 2011-12-20 20:36:35 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 groby epenner cc-bugs epenner jamesr webkit.review.bot oldest_to_newest 524957 0 groby 2011-12-20 16:41:07 -0800 [Coverity] removed dead code 524964 1 120115 groby 2011-12-20 16:50:19 -0800 Created attachment 120115 Patch 524978 2 120115 jamesr 2011-12-20 17:04:26 -0800 Comment on attachment 120115 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=120115&action=review I am significantly skeptical of the correctness of this patch. What's the reasoning behind it? > Source/WebCore/ChangeLog:8 > + No new tests. (OOPS!) this will fail an SVN presubmit check 524989 3 groby 2011-12-20 17:13:39 -0800 I'm significantly certain that code after a return is never executed :) This is a simple removal of dead code - the deleted line can NEVER be hit. (fix for ChangeLog coming) 524992 4 jamesr 2011-12-20 17:15:57 -0800 Oh! I misread the code. In that case the code is buggy and the fix is wrong - we really intend to set that bool member back to false before returning from the function. Eric, you wrote this - can you fix it? 524994 5 jamesr 2011-12-20 17:17:41 -0800 What's really going on here is compositeAndReadback() is effectively disabling idle paints. Since we do compositeAndReadback() on every navigation (for thumbnailing) prepainting is effectively off for all pages currently. Definitely not what we intend. Hurray for coverity for finding this. 524998 6 epenner 2011-12-20 17:24:48 -0800 (In reply to comment #5) > What's really going on here is compositeAndReadback() is effectively disabling idle paints. Since we do compositeAndReadback() on every navigation (for thumbnailing) prepainting is effectively off for all pages currently. Definitely not what we intend. Hurray for coverity for finding this. Arg. This was a last minute change too. It was previously in another function and I 'cleaned it up'. The reason it's not noticeable is that scrolling will trigger sufficient commits anyway. I will fix it. 525002 7 groby 2011-12-20 17:28:59 -0800 Works for me :) I'll leave this one to Eric, then. 525007 8 epenner 2011-12-20 17:35:57 -0800 (In reply to comment #7) > Works for me :) I'll leave this one to Eric, then. Thanks for finding it! :) 525036 9 120132 epenner 2011-12-20 18:32:34 -0800 Created attachment 120132 Patch 525048 10 120132 jamesr 2011-12-20 18:58:27 -0800 Comment on attachment 120132 Patch Test coverage would be even better, but this looks correct. 525071 11 120132 webkit.review.bot 2011-12-20 20:36:31 -0800 Comment on attachment 120132 Patch Clearing flags on attachment: 120132 Committed r103388: <http://trac.webkit.org/changeset/103388> 525072 12 webkit.review.bot 2011-12-20 20:36:35 -0800 All reviewed patches have been landed. Closing bug. 120115 2011-12-20 16:50:19 -0800 2011-12-20 18:32:31 -0800 Patch bug-74974-20111220165018.patch text/plain 1377 groby U3VidmVyc2lvbiBSZXZpc2lvbjogMTAzMzUzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMzdhNDNkZDFhZmRiNDll MGI4ZDlmNjQ0ZTBiMjRjODY3MTE4ZjZhNy4uMmEyNTk2NDUwZmY4YTRlNWJmNmQyNDQwNzg2N2M3 YmRmNjlhNWM2NCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDExLTEyLTIwICBSYWNo ZWwgQmx1bSAgPGdyb2J5QGNocm9taXVtLm9yZz4KKworICAgICAgICBbQ292ZXJpdHldIHJlbW92 ZWQgZGVhZCBjb2RlCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD03NDk3NAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIE5vIG5ldyB0ZXN0cy4gKE9PUFMhKQorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3Mv Y2hyb21pdW0vY2MvQ0NMYXllclRyZWVIb3N0LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkNDTGF5 ZXJUcmVlSG9zdDo6Y29tcG9zaXRlQW5kUmVhZGJhY2spOgorCiAyMDExLTEyLTIwICBUb255IENo YW5nICA8dG9ueUBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgc2l6ZW9mKFJlbmRlclN0eWxlKSBp cyA2NCBpbnN0ZWFkIG9mIDU2IG9uIFdpbmRvd3MgKHg4NikKZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL2NjL0NDTGF5ZXJUcmVlSG9zdC5jcHAg Yi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9jYy9DQ0xheWVyVHJl ZUhvc3QuY3BwCmluZGV4IDI5YzgxYWE1NTUwMDYyNjAyZjZmOGRhM2RmNDMxYTZjZGQzYWFjZTIu LjNkMDc0ZGU0M2IzOTlhNzM4NzQ1NDBkMDZmYTQ4M2U2MjM0NjA5MzcgMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL2NjL0NDTGF5ZXJUcmVlSG9z dC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2hyb21pdW0vY2Mv Q0NMYXllclRyZWVIb3N0LmNwcApAQCAtMTk0LDcgKzE5NCw2IEBAIGJvb2wgQ0NMYXllclRyZWVI b3N0Ojpjb21wb3NpdGVBbmRSZWFkYmFjayh2b2lkICpwaXhlbHMsIGNvbnN0IEludFJlY3QmIHJl Y3QpCiB7CiAgICAgbV90cmlnZ2VySWRsZVBhaW50cyA9IGZhbHNlOwogICAgIHJldHVybiBtX3By b3h5LT5jb21wb3NpdGVBbmRSZWFkYmFjayhwaXhlbHMsIHJlY3QpOwotICAgIG1fdHJpZ2dlcklk bGVQYWludHMgPSB0cnVlOwogfQogCiB2b2lkIENDTGF5ZXJUcmVlSG9zdDo6ZmluaXNoQWxsUmVu ZGVyaW5nKCkK 120132 2011-12-20 18:32:34 -0800 2011-12-20 20:36:31 -0800 Patch bug-74974-20111220183233.patch text/plain 1497 epenner U3VidmVyc2lvbiBSZXZpc2lvbjogMTAzMzA4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMWFiY2Y5MTNiNDNhMDE2 OWVjNjliYzAwNmE5MmE1MTA3ZjQ0ZDliMS4uMzU0NjU1YWY3ZmZjNDJhNTAwMjkxOTNlYmJmNmIy YzRkNzA5OTRmOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDExLTEyLTIwICBFcmlj IFBlbm5lciAgPGVwZW5uZXJAZ29vZ2xlLmNvbT4KKworICAgICAgICBbY2hyb21pdW1dIG1fdHJp Z2dlcklkbGVQYWludHMgbm90IHJlc2V0IGFmdGVyIGEgY29tcG9zaXRlQW5kUmVhZGJhY2sKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTc0OTc0CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9n cmFwaGljcy9jaHJvbWl1bS9jYy9DQ0xheWVyVHJlZUhvc3QuY3BwOgorICAgICAgICAoV2ViQ29y ZTo6Q0NMYXllclRyZWVIb3N0Ojpjb21wb3NpdGVBbmRSZWFkYmFjayk6CisKIDIwMTEtMTItMjAg IFNoZXJpZmYgQm90ICA8d2Via2l0LnJldmlldy5ib3RAZ21haWwuY29tPgogCiAgICAgICAgIFVu cmV2aWV3ZWQsIHJvbGxpbmcgb3V0IHIxMDMyOTEuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9jYy9DQ0xheWVyVHJlZUhvc3QuY3BwIGIvU291 cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2hyb21pdW0vY2MvQ0NMYXllclRyZWVIb3N0 LmNwcAppbmRleCAyOWM4MWFhNTU1MDA2MjYwMmY2ZjhkYTNkZjQzMWE2Y2RkM2FhY2UyLi5jZDc1 NmQ2MDBkYzBmMGUzZjMwMWIzMmQ3ZThlODRmYzAwYWIwNzYxIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9jYy9DQ0xheWVyVHJlZUhvc3QuY3Bw CisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL2NjL0NDTGF5 ZXJUcmVlSG9zdC5jcHAKQEAgLTE5Myw4ICsxOTMsOSBAQCBHcmFwaGljc0NvbnRleHQzRCogQ0NM YXllclRyZWVIb3N0Ojpjb250ZXh0KCkKIGJvb2wgQ0NMYXllclRyZWVIb3N0Ojpjb21wb3NpdGVB bmRSZWFkYmFjayh2b2lkICpwaXhlbHMsIGNvbnN0IEludFJlY3QmIHJlY3QpCiB7CiAgICAgbV90 cmlnZ2VySWRsZVBhaW50cyA9IGZhbHNlOwotICAgIHJldHVybiBtX3Byb3h5LT5jb21wb3NpdGVB bmRSZWFkYmFjayhwaXhlbHMsIHJlY3QpOworICAgIGJvb2wgcmV0ID0gbV9wcm94eS0+Y29tcG9z aXRlQW5kUmVhZGJhY2socGl4ZWxzLCByZWN0KTsKICAgICBtX3RyaWdnZXJJZGxlUGFpbnRzID0g dHJ1ZTsKKyAgICByZXR1cm4gcmV0OwogfQogCiB2b2lkIENDTGF5ZXJUcmVlSG9zdDo6ZmluaXNo QWxsUmVuZGVyaW5nKCkK