58788 2011-04-18 08:07:32 -0700 When flushing GrContext in ~PlatformContextSkia be sure correct GL context is set 2011-04-19 10:42:00 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) PC All RESOLVED FIXED P2 Normal --- 1 bsalomon bsalomon abarth bsalomon commit-queue eric jamesr kbr reed webkit.review.bot oldest_to_newest 387726 0 bsalomon 2011-04-18 08:07:32 -0700 When flushing GrContext in ~PlatformContextSkia be sure correct GL context is set 387735 1 90039 bsalomon 2011-04-18 08:21:06 -0700 Created attachment 90039 Patch 387739 2 bsalomon 2011-04-18 08:30:43 -0700 This fixes corruption that can occur when changing URLS on the same domain in a tab with the skia gpu flag enabled. The flush can happen into the compositor's context causing mayhem. 387748 3 90039 eric 2011-04-18 08:45:56 -0700 Comment on attachment 90039 Patch How do we test this? Why are no tests required? (all changes require testing if possible/feasible). 387749 4 90039 eric 2011-04-18 08:46:21 -0700 Comment on attachment 90039 Patch Marking r- for now. Please feel to mark r? again with explanation. 387992 5 90039 kbr 2011-04-18 13:28:40 -0700 Comment on attachment 90039 Patch Theoretically this fix should fix failures in pixel layout tests when running with Skia's GPU backend, correct? (We aren't running tests in this configuration on bots yet.) If so then we should just go ahead and r+ this. 388011 6 jamesr 2011-04-18 13:54:55 -0700 Can you confirm that at least one test progresses with this patch when SKIA_GPU is on and cite that test in the changelog? 388021 7 bsalomon 2011-04-18 14:00:36 -0700 After talking to Vangelis I think I'll need to add a browser test to exercise this. To test this we have to open a canvas page and then load a different page in the same renderer that uses accelerated compositing. 388096 8 kbr 2011-04-18 15:21:21 -0700 In that case I would like to r+ this patch because we need to get the fix in; we can add a Chromium browser test afterward (and actually can't add such a test until this fix lands). Eric, is this okay with you? 388098 9 90039 eric 2011-04-18 15:22:33 -0700 Comment on attachment 90039 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=90039&action=review > Source/WebCore/ChangeLog:8 > + No new tests required. This line remains patently false. :) But if it's changed to explain how it can't be tested in WebKit but will be tested in Chrome code, that's fine. 388434 10 90186 bsalomon 2011-04-19 05:55:19 -0700 Created attachment 90186 Patch 388435 11 bsalomon 2011-04-19 05:56:53 -0700 (In reply to comment #9) > (From update of attachment 90039 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=90039&action=review > > > Source/WebCore/ChangeLog:8 > > + No new tests required. > > This line remains patently false. :) But if it's changed to explain how it can't be tested in WebKit but will be tested in Chrome code, that's fine. The new patch has a comment about how to test this in Chrome. 388492 12 90186 eric 2011-04-19 07:18:05 -0700 Comment on attachment 90186 Patch Thanks. 388544 13 90186 commit-queue 2011-04-19 09:25:25 -0700 Comment on attachment 90186 Patch Clearing flags on attachment: 90186 Committed r84257: <http://trac.webkit.org/changeset/84257> 388545 14 commit-queue 2011-04-19 09:25:30 -0700 All reviewed patches have been landed. Closing bug. 388590 15 webkit.review.bot 2011-04-19 10:42:00 -0700 http://trac.webkit.org/changeset/84257 might have broken Windows 7 Release (Tests) 90039 2011-04-18 08:21:06 -0700 2011-04-19 05:55:15 -0700 Patch bug-58788-20110418111810.patch text/plain 1558 bsalomon SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg0MTQ0KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDQtMTggIEJyaWFuIFNh bG9tb24gIDxic2Fsb21vbkBnb29nbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFdoZW4gZmx1c2hpbmcgR3JDb250ZXh0IGluIH5QbGF0Zm9y bUNvbnRleHRTa2lhIGJlIHN1cmUgY29ycmVjdCBHTCBjb250ZXh0IGlzIHNldAorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTg3ODgKKworICAgICAgICBO byBuZXcgdGVzdHMgcmVxdWlyZWQuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9za2lh L1BsYXRmb3JtQ29udGV4dFNraWEuY3BwOgorICAgICAgICAoV2ViQ29yZTo6UGxhdGZvcm1Db250 ZXh0U2tpYTo6flBsYXRmb3JtQ29udGV4dFNraWEpOgorCiAyMDExLTA0LTE4ICBTaGVyaWZmIEJv dCAgPHdlYmtpdC5yZXZpZXcuYm90QGdtYWlsLmNvbT4KIAogICAgICAgICBVbnJldmlld2VkLCBy b2xsaW5nIG91dCByODM4MDMuCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGlj cy9za2lhL1BsYXRmb3JtQ29udGV4dFNraWEuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJD b3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEvUGxhdGZvcm1Db250ZXh0U2tpYS5jcHAJKHJldmlz aW9uIDg0MDQ2KQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9QbGF0 Zm9ybUNvbnRleHRTa2lhLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMjMzLDggKzIzMywxMSBAQCBQ bGF0Zm9ybUNvbnRleHRTa2lhOjp+UGxhdGZvcm1Db250ZXh0U2tpCiAgICAgaWYgKG1fZ3B1Q2Fu dmFzKSB7CiAjaWYgRU5BQkxFKFNLSUFfR1BVKQogICAgICAgICAvLyBtYWtlIHN1cmUgZXZlcnl0 aGluZyByZWxhdGVkIHRvIHRoaXMgcGxhdGZvcm0gY29udGV4dCBoYXMgYmVlbiBmbHVzaGVkCi0g ICAgICAgIGlmICghbV91c2VHUFUpCi0gICAgICAgICAgICBtX2dwdUNhbnZhcy0+Y29udGV4dCgp LT5nckNvbnRleHQoKS0+Zmx1c2goMCk7CisgICAgICAgIGlmICghbV91c2VHUFUpIHsKKyAgICAg ICAgICAgIFNoYXJlZEdyYXBoaWNzQ29udGV4dDNEKiBjb250ZXh0ID0gbV9ncHVDYW52YXMtPmNv bnRleHQoKTsKKyAgICAgICAgICAgIGNvbnRleHQtPm1ha2VDb250ZXh0Q3VycmVudCgpOworICAg ICAgICAgICAgY29udGV4dC0+Z3JDb250ZXh0KCktPmZsdXNoKDApOworICAgICAgICB9CiAjZW5k aWYKICAgICAgICAgbV9ncHVDYW52YXMtPmRyYXdpbmdCdWZmZXIoKS0+c2V0V2lsbFB1Ymxpc2hD YWxsYmFjaygwKTsKICAgICB9Cg== 90186 2011-04-19 05:55:19 -0700 2011-04-19 09:25:25 -0700 Patch bug-58788-20110419085222.patch text/plain 1721 bsalomon SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg0MjQ3KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMDQtMTkgQnJpYW4gU2Fs b21vbiA8YnNhbG9tb25AZ29vZ2xlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBXaGVuIGZsdXNoaW5nIEdyQ29udGV4dCBpbiB+UGxhdGZvcm1D b250ZXh0U2tpYSBiZSBzdXJlIGNvcnJlY3QgR0wgY29udGV4dCBpcyBzZXQKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTU4Nzg4CisKKyAgICAgICAgQ2hy b21pdW0gYnJvd3NlciB0ZXN0IHdpbGwgYmUgYWRkZWQgdGhhdCBsb2FkcyBjYW52YXMgcGFnZSBh bmQgdGhlbiBhbiAKKyAgICAgICAgYWNjbGVyYXRlZC1jb21wb3NpdGVkIHBhZ2UuIFRlc3QgbXVz dCB0cmlnZ2VyIH5QbGF0Zm9ybUNvbnRleHRTa2lhKCkgCisgICAgICAgIGZvbGxvd2VkIGJ5IGlu dm9jYXRpb24gb2YgR0wgY29tcG9zaXRvci4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNz L3NraWEvUGxhdGZvcm1Db250ZXh0U2tpYS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpQbGF0Zm9y bUNvbnRleHRTa2lhOjp+UGxhdGZvcm1Db250ZXh0U2tpYSk6CisKIDIwMTEtMDQtMTkgIEtpbnVr byBZYXN1ZGEgIDxraW51a29AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERh dmlkIExldmluLgpJbmRleDogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9Q bGF0Zm9ybUNvbnRleHRTa2lhLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9ncmFwaGljcy9za2lhL1BsYXRmb3JtQ29udGV4dFNraWEuY3BwCShyZXZpc2lvbiA4NDI0 NykKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEvUGxhdGZvcm1Db250 ZXh0U2tpYS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTIzMyw4ICsyMzMsMTEgQEAgUGxhdGZvcm1D b250ZXh0U2tpYTo6flBsYXRmb3JtQ29udGV4dFNraQogICAgIGlmIChtX2dwdUNhbnZhcykgewog I2lmIEVOQUJMRShTS0lBX0dQVSkKICAgICAgICAgLy8gbWFrZSBzdXJlIGV2ZXJ5dGhpbmcgcmVs YXRlZCB0byB0aGlzIHBsYXRmb3JtIGNvbnRleHQgaGFzIGJlZW4gZmx1c2hlZAotICAgICAgICBp ZiAoIW1fdXNlR1BVKQotICAgICAgICAgICAgbV9ncHVDYW52YXMtPmNvbnRleHQoKS0+Z3JDb250 ZXh0KCktPmZsdXNoKDApOworICAgICAgICBpZiAoIW1fdXNlR1BVKSB7CisgICAgICAgICAgICBT aGFyZWRHcmFwaGljc0NvbnRleHQzRCogY29udGV4dCA9IG1fZ3B1Q2FudmFzLT5jb250ZXh0KCk7 CisgICAgICAgICAgICBjb250ZXh0LT5tYWtlQ29udGV4dEN1cnJlbnQoKTsKKyAgICAgICAgICAg IGNvbnRleHQtPmdyQ29udGV4dCgpLT5mbHVzaCgwKTsKKyAgICAgICAgfQogI2VuZGlmCiAgICAg ICAgIG1fZ3B1Q2FudmFzLT5kcmF3aW5nQnVmZmVyKCktPnNldFdpbGxQdWJsaXNoQ2FsbGJhY2so MCk7CiAgICAgfQo=