49560 2010-11-15 13:12:26 -0800 [chromium] Avoid copying of SkBitmap in LayerRendererChromium 2010-11-15 17:02:34 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 kbr kbr dglazkov jamesr levin thakis vangelis oldest_to_newest 309408 0 kbr 2010-11-15 13:12:26 -0800 Grace Kloba found that LayerRendererChromium was inadvertently causing a copy of the SkBitmap data structure for the root layer's canvas. If the bitmap is using an SkPixelRef internally, this can cause getPixels() to return NULL. The fix is to use a const SkBitmap& like ContentLayerChromium already does. 309410 1 73925 kbr 2010-11-15 13:14:30 -0800 Created attachment 73925 Patch 309412 2 73925 jamesr 2010-11-15 13:15:36 -0800 Comment on attachment 73925 Patch Good catch! 309413 3 kbr 2010-11-15 13:17:13 -0800 Committed r72026: <http://trac.webkit.org/changeset/72026> 309508 4 thakis 2010-11-15 16:48:29 -0800 Accessing pixels() without calling lockPixels() isn't guaranteed to work. 309512 5 jamesr 2010-11-15 16:57:23 -0800 Yeah, this really should call accessBitmap(true) as well, no? 309516 6 kbr 2010-11-15 17:02:08 -0800 (In reply to comment #4) > Accessing pixels() without calling lockPixels() isn't guaranteed to work. This is definitely true for an ordinary SkBitmap, but what about the bitmaps that are managed by an SkDevice? 309517 7 kbr 2010-11-15 17:02:34 -0800 (In reply to comment #5) > Yeah, this really should call accessBitmap(true) as well, no? No; the bitmap's pixels are only accessed to upload them to OpenGL, and aren't modified. 73925 2010-11-15 13:14:30 -0800 2010-11-15 13:15:36 -0800 Patch bug-49560-20101115131429.patch text/plain 1440 kbr SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA3MjAyNSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTAtMTEtMTUgIEtlbm5ldGggUnVzc2VsbCAgPGtickBnb29nbGUu Y29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFtj aHJvbWl1bV0gQXZvaWQgY29weWluZyBvZiBTa0JpdG1hcCBpbiBMYXllclJlbmRlcmVyQ2hyb21p dW0KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQ5NTYw CisKKyAgICAgICAgUmFuIENTUyAzRCBjb250ZW50IHRvIHRlc3QuIE5vIG90aGVyIG5ldyB0ZXN0 cy4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL0xheWVyUmVuZGVyZXJD aHJvbWl1bS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpMYXllclJlbmRlcmVyQ2hyb21pdW06OnVw ZGF0ZVJvb3RMYXllclRleHR1cmVSZWN0KToKKwogMjAxMC0xMS0xNSAgVmFuZ2VsaXMgS29ra2V2 aXMgIDx2YW5nZWxpc0BjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgS2VubmV0 aCBSdXNzZWxsLgpJbmRleDogV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9MYXll clJlbmRlcmVyQ2hyb21pdW0uY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvcGxhdGZvcm0vZ3Jh cGhpY3MvY2hyb21pdW0vTGF5ZXJSZW5kZXJlckNocm9taXVtLmNwcAkocmV2aXNpb24gNzIwMjIp CisrKyBXZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL0xheWVyUmVuZGVyZXJDaHJv bWl1bS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTI3MSw3ICsyNzEsNyBAQCB2b2lkIExheWVyUmVu ZGVyZXJDaHJvbWl1bTo6dXBkYXRlUm9vdExhCiAKICNpZiBQTEFURk9STShTS0lBKQogICAgIC8v IEdldCB0aGUgY29udGVudHMgb2YgdGhlIHVwZGF0ZWQgcmVjdC4KLSAgICBjb25zdCBTa0JpdG1h cCBiaXRtYXAgPSBtX3Jvb3RMYXllckNhbnZhcy0+Z2V0RGV2aWNlKCktPmFjY2Vzc0JpdG1hcChm YWxzZSk7CisgICAgY29uc3QgU2tCaXRtYXAmIGJpdG1hcCA9IG1fcm9vdExheWVyQ2FudmFzLT5n ZXREZXZpY2UoKS0+YWNjZXNzQml0bWFwKGZhbHNlKTsKICAgICBBU1NFUlQoYml0bWFwLndpZHRo KCkgPT0gdXBkYXRlUmVjdC53aWR0aCgpICYmIGJpdG1hcC5oZWlnaHQoKSA9PSB1cGRhdGVSZWN0 LmhlaWdodCgpKTsKICAgICB2b2lkKiBwaXhlbHMgPSBiaXRtYXAuZ2V0UGl4ZWxzKCk7CiAjZWxp ZiBQTEFURk9STShDRykK