90445 2012-07-03 04:14:29 -0700 Add a comment in order to clarify why BitmapImage::frameHasAlphaAtIndex returns true as default. 2012-07-03 22:19:49 -0700 1 1 1 Unclassified WebKit Images 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 0 dongseong.hwang webkit-unassigned darin eric webkit.review.bot oldest_to_newest 662110 0 dongseong.hwang 2012-07-03 04:14:29 -0700 Add an assertion and a comment in order to clarify the states of BitmapImage. 662111 1 150577 dongseong.hwang 2012-07-03 04:16:20 -0700 Created attachment 150577 patch 662112 2 150577 dongseong.hwang 2012-07-03 04:18:14 -0700 Comment on attachment 150577 patch View in context: https://bugs.webkit.org/attachment.cgi?id=150577&action=review > Source/WebCore/platform/graphics/BitmapImage.cpp:312 > + ASSERT(m_frames[index].m_isComplete); All case that call this method is guarded by if(isComplete). > Source/WebCore/platform/graphics/BitmapImage.cpp:326 > + // black. The comment was copied from ImageSource::frameHasAlphaAtIndex(size_t index) 662117 3 150579 dongseong.hwang 2012-07-03 04:26:50 -0700 Created attachment 150579 patch v.2 662118 4 150579 dongseong.hwang 2012-07-03 04:27:09 -0700 Comment on attachment 150579 patch v.2 View in context: https://bugs.webkit.org/attachment.cgi?id=150579&action=review > Source/WebCore/platform/graphics/BitmapImage.cpp:322 > + // When a frame has not finished decoding, always mark it as having alpha. The comment was copied from ImageSource::frameHasAlphaAtIndex(size_t index) 662132 5 dongseong.hwang 2012-07-03 05:00:56 -0700 (In reply to comment #0) > Add an assertion and a comment in order to clarify the states of BitmapImage. I removed the assertion in patch v.2 because I misunderstood at the time to patch v.1. 662535 6 dongseong.hwang 2012-07-03 15:54:55 -0700 (In reply to comment #4) > (From update of attachment 150579 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=150579&action=review > > > Source/WebCore/platform/graphics/BitmapImage.cpp:322 > > + // When a frame has not finished decoding, always mark it as having alpha. > > The comment was copied from ImageSource::frameHasAlphaAtIndex(size_t index) The comment of ImageSource::frameHasAlphaAtIndex is long, so I pasted just summary of the comment to BitmapImage::frameHasAlphaAtIndex. bool ImageSource::frameHasAlphaAtIndex(ImageFrame* buffer) { // When a frame has not finished decoding, always mark it as having alpha. // Ports that check the result of this function to determine their // compositing op need this in order to not draw the undecoded portion as // black. // TODO: Perhaps we should ensure that each individual decoder returns true // in this case. return !frameIsCompleteAtIndex(buffer) || buffer->hasAlpha(); } 662580 7 150579 eric 2012-07-03 16:57:37 -0700 Comment on attachment 150579 patch v.2 View in context: https://bugs.webkit.org/attachment.cgi?id=150579&action=review >>> Source/WebCore/platform/graphics/BitmapImage.cpp:322 >>> + // When a frame has not finished decoding, always mark it as having alpha. >> >> The comment was copied from ImageSource::frameHasAlphaAtIndex(size_t index) > > The comment of ImageSource::frameHasAlphaAtIndex is long, so I pasted just summary of the comment to BitmapImage::frameHasAlphaAtIndex. > > bool ImageSource::frameHasAlphaAtIndex(ImageFrame* buffer) > { > // When a frame has not finished decoding, always mark it as having alpha. > // Ports that check the result of this function to determine their > // compositing op need this in order to not draw the undecoded portion as > // black. > // TODO: Perhaps we should ensure that each individual decoder returns true > // in this case. > return !frameIsCompleteAtIndex(buffer) || buffer->hasAlpha(); > } You should add a line that said: // See ImageSource::framehasAlphaAtIndex for explanation of why incomplete images claim to have alpha. 662629 8 150704 dongseong.hwang 2012-07-03 18:29:04 -0700 Created attachment 150704 patch v.3 Absolutely! 662666 9 150704 webkit.review.bot 2012-07-03 22:19:44 -0700 Comment on attachment 150704 patch v.3 Clearing flags on attachment: 150704 Committed r121827: <http://trac.webkit.org/changeset/121827> 662667 10 webkit.review.bot 2012-07-03 22:19:49 -0700 All reviewed patches have been landed. Closing bug. 150577 2012-07-03 04:16:20 -0700 2012-07-03 04:26:50 -0700 patch 0001-Add-an-assertion-and-a-comment-in-order-to-clarify-t.patch text/plain 1457 dongseong.hwang RnJvbSBiNzhiOGY5M2YzMTY4MDk5YjQ0NmViNDQzN2FiN2Q5ZWM1NmJhY2UzIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBIdWFuZyBEb25nc3VuZyA8bHV4dGVsbGFAY29tcGFueTEwMC5u ZXQ+CkRhdGU6IFdlZCwgMjMgTWF5IDIwMTIgMTU6NTA6MjUgKzA5MDAKU3ViamVjdDogW1BBVENI XSBBZGQgYW4gYXNzZXJ0aW9uIGFuZCBhIGNvbW1lbnQgaW4gb3JkZXIgdG8gY2xhcmlmeSB0aGUK IHN0YXRlcyBvZiBCaXRtYXBJbWFnZS4KIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD05MDQ0NQoKLS0tCiBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9CaXRt YXBJbWFnZS5jcHAgfCAgICA3ICsrKysrKy0KIDEgZmlsZSBjaGFuZ2VkLCA2IGluc2VydGlvbnMo KyksIDEgZGVsZXRpb24oLSkKCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9n cmFwaGljcy9CaXRtYXBJbWFnZS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGlj cy9CaXRtYXBJbWFnZS5jcHAKaW5kZXggOGJjOTcyMi4uYzFjM2Q4MSAxMDA2NDQKLS0tIGEvU291 cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvQml0bWFwSW1hZ2UuY3BwCisrKyBiL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0JpdG1hcEltYWdlLmNwcApAQCAtMzA5LDYgKzMw OSw3IEBAIGZsb2F0IEJpdG1hcEltYWdlOjpmcmFtZUR1cmF0aW9uQXRJbmRleChzaXplX3QgaW5k ZXgpCiB7CiAgICAgaWYgKCFlbnN1cmVGcmFtZUlzQ2FjaGVkKGluZGV4KSkKICAgICAgICAgcmV0 dXJuIDA7CisgICAgQVNTRVJUKG1fZnJhbWVzW2luZGV4XS5tX2lzQ29tcGxldGUpOwogICAgIHJl dHVybiBtX2ZyYW1lc1tpbmRleF0ubV9kdXJhdGlvbjsKIH0KIApAQCAtMzE5LDggKzMyMCwxMiBA QCBOYXRpdmVJbWFnZVB0ciBCaXRtYXBJbWFnZTo6bmF0aXZlSW1hZ2VGb3JDdXJyZW50RnJhbWUo KQogCiBib29sIEJpdG1hcEltYWdlOjpmcmFtZUhhc0FscGhhQXRJbmRleChzaXplX3QgaW5kZXgp CiB7CisgICAgLy8gV2hlbiBhIGZyYW1lIGhhcyBub3QgZmluaXNoZWQgZGVjb2RpbmcsIGFsd2F5 cyBtYXJrIGl0IGFzIGhhdmluZyBhbHBoYS4KKyAgICAvLyBQb3J0cyB0aGF0IGNoZWNrIHRoZSBy ZXN1bHQgb2YgdGhpcyBmdW5jdGlvbiB0byBkZXRlcm1pbmUgdGhlaXIKKyAgICAvLyBjb21wb3Np dGluZyBvcCBuZWVkIHRoaXMgaW4gb3JkZXIgdG8gbm90IGRyYXcgdGhlIHVuZGVjb2RlZCBwb3J0 aW9uIGFzCisgICAgLy8gYmxhY2suCiAgICAgaWYgKCFlbnN1cmVGcmFtZUlzQ2FjaGVkKGluZGV4 KSkKLSAgICAgICAgcmV0dXJuIHRydWU7IC8vIFdoeSBkb2VzIGFuIGludmFsaWQgaW5kZXggbWVh biBhbHBoYT8KKyAgICAgICAgcmV0dXJuIHRydWU7CiAgICAgcmV0dXJuIG1fZnJhbWVzW2luZGV4 XS5tX2hhc0FscGhhOwogfQogCi0tIAoxLjcuOS41Cgo= 150579 2012-07-03 04:26:50 -0700 2012-07-03 18:29:04 -0700 patch v.2 0001-Add-a-comment-in-order-to-clarify-why.patch text/plain 1885 dongseong.hwang RnJvbSA5ZDM0ODAzMzFlOGI0OTc5ODQyYzAwZjYxZGQyNjk0NjM0MDZhY2Q2IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBIdWFuZyBEb25nc3VuZyA8bHV4dGVsbGFAY29tcGFueTEwMC5u ZXQ+CkRhdGU6IFdlZCwgMjMgTWF5IDIwMTIgMTU6NTA6MjUgKzA5MDAKU3ViamVjdDogW1BBVENI XSBBZGQgYSBjb21tZW50IGluIG9yZGVyIHRvIGNsYXJpZnkgd2h5CiBCaXRtYXBJbWFnZTo6ZnJh bWVIYXNBbHBoYUF0SW5kZXggcmV0dXJucyB0cnVlIGFzIGRlZmF1bHQuCiBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTA0NDUKCi0tLQogU291cmNlL1dlYkNvcmUvQ2hh bmdlTG9nICAgICAgICAgICAgICAgICAgICAgICAgIHwgICAxNCArKysrKysrKysrKysrKwogU291 cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvQml0bWFwSW1hZ2UuY3BwIHwgICAgMyArKy0K IDIgZmlsZXMgY2hhbmdlZCwgMTYgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQoKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxv ZwppbmRleCBkMmRiYmU0Li5hMWM1MDUxIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFu Z2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIw MTItMDctMDMgIEh1YW5nIERvbmdzdW5nICA8bHV4dGVsbGFAY29tcGFueTEwMC5uZXQ+CisKKyAg ICAgICAgQWRkIGEgY29tbWVudCBpbiBvcmRlciB0byBjbGFyaWZ5IHdoeQorICAgICAgICBCaXRt YXBJbWFnZTo6ZnJhbWVIYXNBbHBoYUF0SW5kZXggcmV0dXJucyB0cnVlIGFzIGRlZmF1bHQuCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD05MDQ0NQorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE5vIG5ldyB0ZXN0 cy4gVGhpcyBwYXRjaCBkb2Vzbid0IGNoYW5nZSBiZWhhdmlvci4KKworICAgICAgICAqIHBsYXRm b3JtL2dyYXBoaWNzL0JpdG1hcEltYWdlLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkJpdG1hcElt YWdlOjpmcmFtZUhhc0FscGhhQXRJbmRleCk6CisgICAgICAgIChXZWJDb3JlKToKKwogMjAxMi0w Ny0wMyAgUGhpbGlwIFJvZ2VycyAgPHBkckBnb29nbGUuY29tPgogCiAgICAgICAgIEZpeCB0ZXh0 IHBvc2l0aW9uaW5nIHdpdGggbm9uLWJtcCBjaGFyYWN0ZXJzLgpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvQml0bWFwSW1hZ2UuY3BwIGIvU291cmNlL1dlYkNv cmUvcGxhdGZvcm0vZ3JhcGhpY3MvQml0bWFwSW1hZ2UuY3BwCmluZGV4IDhiYzk3MjIuLjUzYTk4 MTQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0JpdG1hcElt YWdlLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9CaXRtYXBJbWFn ZS5jcHAKQEAgLTMxOSw4ICszMTksOSBAQCBOYXRpdmVJbWFnZVB0ciBCaXRtYXBJbWFnZTo6bmF0 aXZlSW1hZ2VGb3JDdXJyZW50RnJhbWUoKQogCiBib29sIEJpdG1hcEltYWdlOjpmcmFtZUhhc0Fs cGhhQXRJbmRleChzaXplX3QgaW5kZXgpCiB7CisgICAgLy8gV2hlbiBhIGZyYW1lIGhhcyBub3Qg ZmluaXNoZWQgZGVjb2RpbmcsIGFsd2F5cyBtYXJrIGl0IGFzIGhhdmluZyBhbHBoYS4KICAgICBp ZiAoIWVuc3VyZUZyYW1lSXNDYWNoZWQoaW5kZXgpKQotICAgICAgICByZXR1cm4gdHJ1ZTsgLy8g V2h5IGRvZXMgYW4gaW52YWxpZCBpbmRleCBtZWFuIGFscGhhPworICAgICAgICByZXR1cm4gdHJ1 ZTsKICAgICByZXR1cm4gbV9mcmFtZXNbaW5kZXhdLm1faGFzQWxwaGE7CiB9CiAKLS0gCjEuNy45 LjUKCg== 150704 2012-07-03 18:29:04 -0700 2012-07-03 22:19:44 -0700 patch v.3 0001-Add-a-comment-in-order-to-clarify-why.patch text/plain 2121 dongseong.hwang RnJvbSA4ZGI1ZDFjYzdlNDc1YTU4OWNlZGUzM2RmMjM4ZDcyMmYxN2M5YjZmIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBIdWFuZyBEb25nc3VuZyA8bHV4dGVsbGFAY29tcGFueTEwMC5u ZXQ+CkRhdGU6IFdlZCwgMjMgTWF5IDIwMTIgMTU6NTA6MjUgKzA5MDAKU3ViamVjdDogW1BBVENI XSBBZGQgYSBjb21tZW50IGluIG9yZGVyIHRvIGNsYXJpZnkgd2h5CiBCaXRtYXBJbWFnZTo6ZnJh bWVIYXNBbHBoYUF0SW5kZXggcmV0dXJucyB0cnVlIGFzIGRlZmF1bHQuCiBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTA0NDUKCi0tLQogU291cmNlL1dlYkNvcmUvQ2hh bmdlTG9nICAgICAgICAgICAgICAgICAgICAgICAgIHwgICAxNCArKysrKysrKysrKysrKwogU291 cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvQml0bWFwSW1hZ2UuY3BwIHwgICAgNCArKyst CiAyIGZpbGVzIGNoYW5nZWQsIDE3IGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkKCmRpZmYg LS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VM b2cKaW5kZXggMzU0MDdhZS4uODY5MDMwOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hh bmdlTG9nCisrKyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE5IEBACiAy MDEyLTA3LTAzICBIdWFuZyBEb25nc3VuZyAgPGx1eHRlbGxhQGNvbXBhbnkxMDAubmV0PgogCisg ICAgICAgIEFkZCBhIGNvbW1lbnQgaW4gb3JkZXIgdG8gY2xhcmlmeSB3aHkKKyAgICAgICAgQml0 bWFwSW1hZ2U6OmZyYW1lSGFzQWxwaGFBdEluZGV4IHJldHVybnMgdHJ1ZSBhcyBkZWZhdWx0Lgor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTA0NDUKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBObyBuZXcgdGVz dHMuIFRoaXMgcGF0Y2ggZG9lc24ndCBjaGFuZ2UgYmVoYXZpb3IuCisKKyAgICAgICAgKiBwbGF0 Zm9ybS9ncmFwaGljcy9CaXRtYXBJbWFnZS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpCaXRtYXBJ bWFnZTo6ZnJhbWVIYXNBbHBoYUF0SW5kZXgpOgorICAgICAgICAoV2ViQ29yZSk6CisKKzIwMTIt MDctMDMgIEh1YW5nIERvbmdzdW5nICA8bHV4dGVsbGFAY29tcGFueTEwMC5uZXQ+CisKICAgICAg ICAgU2V0IHRoZSBhY2Nlc3MgcXVhbGlmaWVyIG9mIHR3byBtZXRob2RzIHRvIHF1ZXJ5IGZyYW1l IHNwZWNpZmljIGluZm8gb2YKICAgICAgICAgQml0bWFwSW1hZ2UgdG8gcHJvdGVjdGVkLgogICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTA1MDUKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0JpdG1hcEltYWdlLmNwcCBi L1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0JpdG1hcEltYWdlLmNwcAppbmRleCA4 YmM5NzIyLi4yYzc1NDA0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9CaXRtYXBJbWFnZS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhp Y3MvQml0bWFwSW1hZ2UuY3BwCkBAIC0zMTksOCArMzE5LDEwIEBAIE5hdGl2ZUltYWdlUHRyIEJp dG1hcEltYWdlOjpuYXRpdmVJbWFnZUZvckN1cnJlbnRGcmFtZSgpCiAKIGJvb2wgQml0bWFwSW1h Z2U6OmZyYW1lSGFzQWxwaGFBdEluZGV4KHNpemVfdCBpbmRleCkKIHsKKyAgICAvLyBXaGVuIGEg ZnJhbWUgaGFzIG5vdCBmaW5pc2hlZCBkZWNvZGluZywgYWx3YXlzIG1hcmsgaXQgYXMgaGF2aW5n IGFscGhhLgorICAgIC8vIFNlZSBJbWFnZVNvdXJjZTo6ZnJhbWVoYXNBbHBoYUF0SW5kZXggZm9y IGV4cGxhbmF0aW9uIG9mIHdoeSBpbmNvbXBsZXRlIGltYWdlcyBjbGFpbSB0byBoYXZlIGFscGhh LgogICAgIGlmICghZW5zdXJlRnJhbWVJc0NhY2hlZChpbmRleCkpCi0gICAgICAgIHJldHVybiB0 cnVlOyAvLyBXaHkgZG9lcyBhbiBpbnZhbGlkIGluZGV4IG1lYW4gYWxwaGE/CisgICAgICAgIHJl dHVybiB0cnVlOwogICAgIHJldHVybiBtX2ZyYW1lc1tpbmRleF0ubV9oYXNBbHBoYTsKIH0KIAot LSAKMS43LjkuNQoK