92008 2012-07-23 10:40:50 -0700 [chromium] WebViewBenchmarkSupport::paint API does not work with recording canvas 2012-07-25 12:50:49 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 alokp alokp abarth dglazkov dmurph fishd jamesr nduca tkent+wkapi webkit.review.bot oldest_to_newest 675807 0 alokp 2012-07-23 10:40:50 -0700 WebViewBenchmarkSupport owns the canvas created by PaintClient. This does not work in the case of a recording canvas, which is owned by SkPicture. 676021 1 153866 alokp 2012-07-23 14:45:15 -0700 Created attachment 153866 proposed patch 676027 2 webkit.review.bot 2012-07-23 14:48:51 -0700 Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI. 676073 3 153866 nduca 2012-07-23 15:23:15 -0700 Comment on attachment 153866 proposed patch Dumb quesiton --- why not just not doing the ownptr thing and clarifying on the comment that the client must ensure the created canvases get deleted? E.g. no destroyCanvas call 676078 4 alokp 2012-07-23 15:30:05 -0700 (In reply to comment #3) > (From update of attachment 153866 [details]) > Dumb quesiton --- why not just not doing the ownptr thing and clarifying on the comment that the client must ensure the created canvases get deleted? E.g. no destroyCanvas call Yeah - if we clarify that the canvas can be disposed of after didPaint call. I thought being explicit would be better. Darin: What do you think? 676084 5 dmurph 2012-07-23 15:36:09 -0700 So the issue is that it wasn't destructing? I'm doing the following: SkDevice* device = new SkDevice(SkBitmap::kARGB_8888_Config, size.width, size.height, false); WebCanvas* canvas = new WebCanvas(device); device->unref(); return canvas; for creating a canvas, which bascically matches the SkCanvas constructor for creating a canvas using a bitmap 676112 6 dmurph 2012-07-23 15:52:13 -0700 Looking at SkPicture, this makes sense. Alok suggested having willPaint return a canvas, which sounds sounds good. 676147 7 fishd 2012-07-23 16:23:43 -0700 (In reply to comment #6) > Looking at SkPicture, this makes sense. Alok suggested having willPaint return a canvas, which sounds sounds good. I like the idea of having willPaint allocate the WebCanvas and didPaint destroy the canvas. WebCanvas* willPaint(const WebSize&) = 0; void didPaint(WebCanvas*) = 0; Then, we can just drop the create/destroyCanvas methods. Fewer methods == more better. 676963 8 154096 alokp 2012-07-24 10:17:48 -0700 Created attachment 154096 proposed patch Got rid of create/destroyCanvas. Modified will/didPaint interface and added comments. 678277 9 154096 nduca 2012-07-25 11:23:17 -0700 Comment on attachment 154096 proposed patch I like this. @fishd? 678352 10 154096 webkit.review.bot 2012-07-25 12:50:44 -0700 Comment on attachment 154096 proposed patch Clearing flags on attachment: 154096 Committed r123645: <http://trac.webkit.org/changeset/123645> 678353 11 webkit.review.bot 2012-07-25 12:50:49 -0700 All reviewed patches have been landed. Closing bug. 153866 2012-07-23 14:45:15 -0700 2012-07-24 10:17:48 -0700 proposed patch 92008.patch text/plain 2548 alokp SW5kZXg6IFNvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCShyZXZpc2lvbiAxMjMzODYpCisrKyBTb3Vy Y2UvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3 IEBACisyMDEyLTA3LTIzICBBbG9rIFByaXlhZGFyc2hpICA8YWxva3BAY2hyb21pdW0ub3JnPgor CisgICAgICAgIFtjaHJvbWl1bV0gV2ViVmlld0JlbmNobWFya1N1cHBvcnQ6OnBhaW50IEFQSSBk b2VzIG5vdCB3b3JrIHdpdGggcmVjb3JkaW5nIGNhbnZhcworICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTIwMDgKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBBZGRlZCBQYWludENsaWVudDo6ZGVzdHJveUNhbnZh cyBtZXRob2QgZm9yIHRoZSBQYWludENsaWVudCB0byBkZXN0cm95IHRoZSBjYW52YXMgaXQgY3Jl YXRlZC4KKworICAgICAgICAqIHB1YmxpYy9XZWJWaWV3QmVuY2htYXJrU3VwcG9ydC5oOgorICAg ICAgICAoUGFpbnRDbGllbnQpOgorICAgICAgICAqIHNyYy9XZWJWaWV3QmVuY2htYXJrU3VwcG9y dEltcGwuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJWaWV3QmVuY2htYXJrU3VwcG9ydEltcGw6 OnBhaW50TGF5ZXIpOgorCiAyMDEyLTA3LTIzICBTaW1vbiBGcmFzZXIgIDxzaW1vbi5mcmFzZXJA YXBwbGUuY29tPgogCiAgICAgICAgIFBhcnQgMiBvZjogSW1wbGVtZW50IHN0aWNreSBwb3NpdGlv bmluZwpJbmRleDogU291cmNlL1dlYktpdC9jaHJvbWl1bS9wdWJsaWMvV2ViVmlld0JlbmNobWFy a1N1cHBvcnQuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0L2Nocm9taXVtL3B1YmxpYy9X ZWJWaWV3QmVuY2htYXJrU3VwcG9ydC5oCShyZXZpc2lvbiAxMjMzODEpCisrKyBTb3VyY2UvV2Vi S2l0L2Nocm9taXVtL3B1YmxpYy9XZWJWaWV3QmVuY2htYXJrU3VwcG9ydC5oCSh3b3JraW5nIGNv cHkpCkBAIC01Miw2ICs1Miw3IEBAIHB1YmxpYzoKICAgICBjbGFzcyBQYWludENsaWVudCB7CiAg ICAgcHVibGljOgogICAgICAgICB2aXJ0dWFsIFdlYkNhbnZhcyogY3JlYXRlQ2FudmFzKGNvbnN0 IFdlYlNpemUmKSA9IDA7CisgICAgICAgIHZpcnR1YWwgdm9pZCBkZXN0cm95Q2FudmFzKFdlYkNh bnZhcyopID0gMDsKIAogICAgICAgICAvLyBDYWxsZWQgYnkgdGhlIFdlYlZpZXdCZW5jaG1hcmtT dXBwb3J0IHdoZW4gcGFpbnRpbmcgaXMgYWJvdXQgdG8KICAgICAgICAgLy8gb2NjdXIuCkluZGV4 OiBTb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJWaWV3QmVuY2htYXJrU3VwcG9ydEltcGwu Y3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYlZpZXdCZW5j aG1hcmtTdXBwb3J0SW1wbC5jcHAJKHJldmlzaW9uIDEyMzM4MSkKKysrIFNvdXJjZS9XZWJLaXQv Y2hyb21pdW0vc3JjL1dlYlZpZXdCZW5jaG1hcmtTdXBwb3J0SW1wbC5jcHAJKHdvcmtpbmcgY29w eSkKQEAgLTQ3LDEyICs0NywxMyBAQCBuYW1lc3BhY2UgV2ViS2l0IHsKIHZvaWQgV2ViVmlld0Jl bmNobWFya1N1cHBvcnRJbXBsOjpwYWludExheWVyKFBhaW50Q2xpZW50KiBwYWludENsaWVudCwg R3JhcGhpY3NMYXllciYgbGF5ZXIsIGNvbnN0IEludFJlY3QmIGNsaXApCiB7CiAgICAgV2ViU2l6 ZSBjYW52YXNTaXplKGNsaXAud2lkdGgoKSwgY2xpcC5oZWlnaHQoKSk7Ci0gICAgT3duUHRyPFdl YkNhbnZhcz4gY2FudmFzID0gYWRvcHRQdHIocGFpbnRDbGllbnQtPmNyZWF0ZUNhbnZhcyhjYW52 YXNTaXplKSk7Ci0gICAgR3JhcGhpY3NDb250ZXh0QnVpbGRlciBidWlsZGVyKGNhbnZhcy5nZXQo KSk7CisgICAgV2ViQ2FudmFzKiBjYW52YXMgPSBwYWludENsaWVudC0+Y3JlYXRlQ2FudmFzKGNh bnZhc1NpemUpOworICAgIEdyYXBoaWNzQ29udGV4dEJ1aWxkZXIgYnVpbGRlcihjYW52YXMpOwog Ci0gICAgcGFpbnRDbGllbnQtPndpbGxQYWludCgqY2FudmFzLmdldCgpKTsKKyAgICBwYWludENs aWVudC0+d2lsbFBhaW50KCpjYW52YXMpOwogICAgIGxheWVyLnBhaW50R3JhcGhpY3NMYXllckNv bnRlbnRzKGJ1aWxkZXIuY29udGV4dCgpLCBjbGlwKTsKLSAgICBwYWludENsaWVudC0+ZGlkUGFp bnQoKmNhbnZhcy5nZXQoKSk7CisgICAgcGFpbnRDbGllbnQtPmRpZFBhaW50KCpjYW52YXMpOwor ICAgIHBhaW50Q2xpZW50LT5kZXN0cm95Q2FudmFzKGNhbnZhcyk7CiB9CiAKIHZvaWQgV2ViVmll d0JlbmNobWFya1N1cHBvcnRJbXBsOjphY2NlbGVyYXRlZFBhaW50VW5jbGlwcGVkKFBhaW50Q2xp ZW50KiBwYWludENsaWVudCwgR3JhcGhpY3NMYXllciYgbGF5ZXIpCg== 154096 2012-07-24 10:17:48 -0700 2012-07-25 12:50:44 -0700 proposed patch 92008.patch text/plain 3931 alokp SW5kZXg6IFNvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCShyZXZpc2lvbiAxMjM0ODYpCisrKyBTb3Vy Y2UvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIw IEBACisyMDEyLTA3LTI0ICBBbG9rIFByaXlhZGFyc2hpICA8YWxva3BAY2hyb21pdW0ub3JnPgor CisgICAgICAgIFtjaHJvbWl1bV0gV2ViVmlld0JlbmNobWFya1N1cHBvcnQ6OnBhaW50IEFQSSBk b2VzIG5vdCB3b3JrIHdpdGggcmVjb3JkaW5nIGNhbnZhcworICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTIwMDgKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBNb3ZlZCB0aGUgb3duZXJzaGlwIG9mIGNhbnZhcyB0 byBXZWJWaWV3QmVuY2htYXJrU3VwcG9ydDo6UGFpbnRDbGllbnQuCisKKyAgICAgICAgKiBwdWJs aWMvV2ViVmlld0JlbmNobWFya1N1cHBvcnQuaDoKKyAgICAgICAgKFBhaW50Q2xpZW50KToKKyAg ICAgICAgKFdlYktpdDo6V2ViVmlld0JlbmNobWFya1N1cHBvcnQ6OlBhaW50Q2xpZW50Ojp3aWxs UGFpbnQpOgorICAgICAgICAoV2ViS2l0OjpXZWJWaWV3QmVuY2htYXJrU3VwcG9ydDo6UGFpbnRD bGllbnQ6OmRpZFBhaW50KToKKyAgICAgICAgKiBzcmMvV2ViVmlld0JlbmNobWFya1N1cHBvcnRJ bXBsLmNwcDoKKyAgICAgICAgKFdlYktpdDo6V2ViVmlld0JlbmNobWFya1N1cHBvcnRJbXBsOjpw YWludExheWVyKToKKyAgICAgICAgKFdlYktpdDo6V2ViVmlld0JlbmNobWFya1N1cHBvcnRJbXBs Ojpzb2Z0d2FyZVBhaW50KToKKwogMjAxMi0wNy0yNCAgS2Vpc2hpIEhhdHRvcmkgIDxrZWlzaGlA d2Via2l0Lm9yZz4KIAogICAgICAgICBbQ2hyb21pdW1dIFBhZ2UgcG9wdXAgc2hvdWxkIGNsb3Nl IG9uIG1vdXNlIGRvd24KSW5kZXg6IFNvdXJjZS9XZWJLaXQvY2hyb21pdW0vcHVibGljL1dlYlZp ZXdCZW5jaG1hcmtTdXBwb3J0LmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktpdC9jaHJvbWl1 bS9wdWJsaWMvV2ViVmlld0JlbmNobWFya1N1cHBvcnQuaAkocmV2aXNpb24gMTIzNDg1KQorKysg U291cmNlL1dlYktpdC9jaHJvbWl1bS9wdWJsaWMvV2ViVmlld0JlbmNobWFya1N1cHBvcnQuaAko d29ya2luZyBjb3B5KQpAQCAtNTEsMTQgKzUxLDE1IEBAIHB1YmxpYzoKICAgICAvLyB3aGVuIHBh aW50aW5nIGlzIGFib3V0IHRvIG9jY3VyIGFuZCB3aGVuIHBhaW50aW5nIGlzIGNvbXBsZXRlLgog ICAgIGNsYXNzIFBhaW50Q2xpZW50IHsKICAgICBwdWJsaWM6Ci0gICAgICAgIHZpcnR1YWwgV2Vi Q2FudmFzKiBjcmVhdGVDYW52YXMoY29uc3QgV2ViU2l6ZSYpID0gMDsKLQotICAgICAgICAvLyBD YWxsZWQgYnkgdGhlIFdlYlZpZXdCZW5jaG1hcmtTdXBwb3J0IHdoZW4gcGFpbnRpbmcgaXMgYWJv dXQgdG8KLSAgICAgICAgLy8gb2NjdXIuCi0gICAgICAgIHZpcnR1YWwgdm9pZCB3aWxsUGFpbnQo Y29uc3QgV2ViQ2FudmFzJikgeyB9CisgICAgICAgIC8vIENhbGxlZCBieSB0aGUgV2ViVmlld0Jl bmNobWFya1N1cHBvcnQgd2hlbiBwYWludGluZyBpcyBhYm91dCB0byBvY2N1ci4KKyAgICAgICAg Ly8gUGFpbnRDbGllbnQgaXMgZXhwZWN0ZWQgdG8gcmV0dXJuIGFuIGFwcHJvcHJpYXRlbHktc2l6 ZWQgY2FudmFzCisgICAgICAgIC8vIGZvciB0aGUgV2ViVmlld0JlbmNobWFya1N1cHBvcnQgdG8g cGFpbnQgb24uCisgICAgICAgIHZpcnR1YWwgV2ViQ2FudmFzKiB3aWxsUGFpbnQoY29uc3QgV2Vi U2l6ZSYpIHsgcmV0dXJuIDA7IH0KIAogICAgICAgICAvLyBDYWxsZWQgYnkgdGhlIFdlYlZpZXdC ZW5jaG1hcmtTdXBwb3J0IHdoZW4gcGFpbnRpbmcgaXMgY29tcGxldGUuCi0gICAgICAgIHZpcnR1 YWwgdm9pZCBkaWRQYWludChjb25zdCBXZWJDYW52YXMmKSB7IH0KKyAgICAgICAgLy8gVGhlIGNh bnZhcyB3aWxsIG5vdCBiZSB1c2VkIGFmdGVyIHRoaXMgY2FsbCBhbmQgY2FuIGJlIGRlc3Ryb3ll ZAorICAgICAgICAvLyBpZiBuZWNlc3NhcnkuCisgICAgICAgIHZpcnR1YWwgdm9pZCBkaWRQYWlu dChXZWJDYW52YXMqKSB7IH0KICAgICBwcm90ZWN0ZWQ6CiAgICAgICAgIHZpcnR1YWwgflBhaW50 Q2xpZW50KCkgeyB9CiAgICAgfTsKSW5kZXg6IFNvdXJjZS9XZWJLaXQvY2hyb21pdW0vc3JjL1dl YlZpZXdCZW5jaG1hcmtTdXBwb3J0SW1wbC5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktp dC9jaHJvbWl1bS9zcmMvV2ViVmlld0JlbmNobWFya1N1cHBvcnRJbXBsLmNwcAkocmV2aXNpb24g MTIzNDg1KQorKysgU291cmNlL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViVmlld0JlbmNobWFya1N1 cHBvcnRJbXBsLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNDcsMTIgKzQ3LDExIEBAIG5hbWVzcGFj ZSBXZWJLaXQgewogdm9pZCBXZWJWaWV3QmVuY2htYXJrU3VwcG9ydEltcGw6OnBhaW50TGF5ZXIo UGFpbnRDbGllbnQqIHBhaW50Q2xpZW50LCBHcmFwaGljc0xheWVyJiBsYXllciwgY29uc3QgSW50 UmVjdCYgY2xpcCkKIHsKICAgICBXZWJTaXplIGNhbnZhc1NpemUoY2xpcC53aWR0aCgpLCBjbGlw LmhlaWdodCgpKTsKLSAgICBPd25QdHI8V2ViQ2FudmFzPiBjYW52YXMgPSBhZG9wdFB0cihwYWlu dENsaWVudC0+Y3JlYXRlQ2FudmFzKGNhbnZhc1NpemUpKTsKLSAgICBHcmFwaGljc0NvbnRleHRC dWlsZGVyIGJ1aWxkZXIoY2FudmFzLmdldCgpKTsKKyAgICBXZWJDYW52YXMqIGNhbnZhcyA9IHBh aW50Q2xpZW50LT53aWxsUGFpbnQoY2FudmFzU2l6ZSk7CisgICAgR3JhcGhpY3NDb250ZXh0QnVp bGRlciBidWlsZGVyKGNhbnZhcyk7CiAKLSAgICBwYWludENsaWVudC0+d2lsbFBhaW50KCpjYW52 YXMuZ2V0KCkpOwogICAgIGxheWVyLnBhaW50R3JhcGhpY3NMYXllckNvbnRlbnRzKGJ1aWxkZXIu Y29udGV4dCgpLCBjbGlwKTsKLSAgICBwYWludENsaWVudC0+ZGlkUGFpbnQoKmNhbnZhcy5nZXQo KSk7CisgICAgcGFpbnRDbGllbnQtPmRpZFBhaW50KGNhbnZhcyk7CiB9CiAKIHZvaWQgV2ViVmll d0JlbmNobWFya1N1cHBvcnRJbXBsOjphY2NlbGVyYXRlZFBhaW50VW5jbGlwcGVkKFBhaW50Q2xp ZW50KiBwYWludENsaWVudCwgR3JhcGhpY3NMYXllciYgbGF5ZXIpCkBAIC04NCwxMCArODMsOSBA QCB2b2lkIFdlYlZpZXdCZW5jaG1hcmtTdXBwb3J0SW1wbDo6c29mdHdhCiAgICAgfQogCiAgICAg V2ViU2l6ZSBjYW52YXNTaXplKHBhaW50U2l6ZS53aWR0aCwgcGFpbnRTaXplLmhlaWdodCk7Ci0g ICAgT3duUHRyPFdlYkNhbnZhcz4gY2FudmFzID0gYWRvcHRQdHIocGFpbnRDbGllbnQtPmNyZWF0 ZUNhbnZhcyhjYW52YXNTaXplKSk7Ci0gICAgcGFpbnRDbGllbnQtPndpbGxQYWludCgqY2FudmFz LmdldCgpKTsKLSAgICBtX3dlYlZpZXdJbXBsLT5wYWludChjYW52YXMuZ2V0KCksIHBhaW50U2l6 ZSk7Ci0gICAgcGFpbnRDbGllbnQtPmRpZFBhaW50KCpjYW52YXMuZ2V0KCkpOworICAgIFdlYkNh bnZhcyogY2FudmFzID0gcGFpbnRDbGllbnQtPndpbGxQYWludChjYW52YXNTaXplKTsKKyAgICBt X3dlYlZpZXdJbXBsLT5wYWludChjYW52YXMsIHBhaW50U2l6ZSk7CisgICAgcGFpbnRDbGllbnQt PmRpZFBhaW50KGNhbnZhcyk7CiB9CiAKIHZvaWQgV2ViVmlld0JlbmNobWFya1N1cHBvcnRJbXBs OjpwYWludChQYWludENsaWVudCogcGFpbnRDbGllbnQsIFBhaW50TW9kZSBwYWludE1vZGUpCg==