New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 826878 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression

Blocked on:
issue 787039



Sign in to add a comment

Canvas function ToDataURL() does not work correctly if the webgl canvas has alpha values

Reported by hartl...@gmail.com, Mar 28 2018

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.181 Safari/537.36

Steps to reproduce the problem:
1. Create a Canvas object
2. Insert WebGL objects.
3. Insert WebGL objects over them that have alpha values of 0.5.
4. Export the canvas to an image using ToDataURL()
5. Display that image on the page.

What is the expected behavior?
An accurate scrape of the pixels within the WebGL canvas.

What went wrong?
The export is wrong.  The object with the alpha values have the wrong colors.  It appears the colors are being inverted.
In the attached picture, the upper frame is the WebGL canvas.  The flag has an alpha of about 0.3.  The lower picture is the image produced from ToDataURL().  You can see that the colors are badly distorted.  

Did this work before? Yes It works in 60.  Haven't tried others.

Does this work in other browsers? Yes

Chrome version: 65.0.3325.181  Channel: stable
OS Version: 10.0
Flash Version: 

It's important to us that WebGL images with alpha can be converted to images correctly.  We'll have to revert to 60 until this is fixed.
 
Capture.PNG
68.6 KB View Download
Labels: Needs-Triage-M65 Needs-Bisect
Cc: sindhu.chelamcherla@chromium.org
Labels: Triaged-ET Needs-Feedback
Thanks for filing the issue!!

@Reporter: Please provide sample URL/test file to test this issue further from TE end.

Comment 3 by hartl...@gmail.com, Mar 29 2018

Attached is the sample case.  It appears the issue occurs when working with textures.  I could not reproduce if textures were not used.
TestFiles_826878.zip
4.4 KB Download
Project Member

Comment 4 by sheriffbot@chromium.org, Mar 29 2018

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: Needs-Feedback
Unable to test this issue as we are seeing blank page without any images on chrome versions. Checked on M-60, M-65 and M-67 images are not seen in .html page.

Issue is working fine in firefox. Attaching screenshots for reference.

@Reporter: Please provide us a working URL to test this issue from TE end.

Thanks!
826878_M60.png
33.3 KB View Download
firefox.png
38.6 KB View Download
826878_M65.png
57.1 KB View Download

Comment 6 by hartl...@gmail.com, Mar 30 2018

Looks like there was a CORS issue retrieving images from the local file system.  The image is encoded in base64 within the file.
TestFiles_826878_2.zip
4.8 KB Download
Project Member

Comment 7 by sheriffbot@chromium.org, Mar 30 2018

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: zakerinasab@chromium.org
Labels: -Pri-2 -Needs-Bisect hasbisect-per-revision Target-66 Target-67 RegressedIn-65 M-65 FoundIn-66 FoundIn-67 Target-65 FoundIn-65 ReleaseBlock-Stable OS-Linux OS-Mac Pri-1
Owner: kbr@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on reported version 65.0.3325.181, on latest beta# 66.0.3359.66 and on latest canary 67.0.3386.0 using Windows 10, Ubuntu 14.04 & Mac 10.13.3.

Bisect Info:
=============
Good Build: 65.0.3285.0
Bad Build: 65.0.3286.0

You are probably looking for a change made after 521714 (known good), but no later than 521715 (first known bad).
CHANGELOG URL:
 https://chromium.googlesource.com/chromium/src/+log/510c597b5876820f33c678b5d04a5f71e42e26f6..6e0ab6ae54cc837abd96024dae5595a8d299e7b8

Reviewed-on: https://chromium-review.googlesource.com/780279

@ zakerinasab: Please confirm the issue and help in re-assigning if it is not related to your change, Adding ReleaseBlock-Stable for M-65 , feel free to remove it if not applicable.

As zakerinasab@ is OOO till April end assigning to reviewer kbr@. Please change if not the case.

Thanks!
Cc: pbomm...@chromium.org manoranj...@chromium.org gov...@chromium.org abdulsyed@chromium.org
Labels: M-66 M-67
Just a heads up, M66 Stable cut is on April 12th, 10 days away. This issue is marked as RB-Stable for 66. Please make sure to address this issue prior to stable cut. Thanks! 

Comment 11 by kbr@chromium.org, Apr 2 2018

Cc: fs...@chromium.org
Owner: junov@chromium.org
Confirmed with per-revision bisect (go/bisect-builds) that the above bisect is correct.

You are probably looking for a change made after 521714 (known good), but no later than 521715 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
  https://chromium.googlesource.com/chromium/src/+log/510c597b5876820f33c678b5d04a5f71e42e26f6..6e0ab6ae54cc837abd96024dae5595a8d299e7b8

Justin or Fernando, could one of you please take this? I'm not that familiar with the intent of the original CL. I can help produce a WebGL conformance test for this though.

Comment 12 by kbr@chromium.org, Apr 2 2018

Blockedon: 787039
Linking to  Issue 787039  in which this was introduced.

Thinking about this more, it's probably a difference in handling of superluminal values, which require "un-premultiplied alpha" handling.

Labels: -M-65
We're not planning any more M65 releases. Pls target fix for M66.
Friendly ping to get an update on this issue as it is marked as stable blocker & M66 Stable cut is on April 12th.

Thanks..! 

Zakerinasab is on leave and I was on vacation last week.  Taking a look now...
Status: Started (was: Assigned)
The problem is that WebGL has no safeguard against "illegal" alpha-premultiplied color values.  The code in the repro case is producing pixels that have color component values that are larger that the pixel's alpha value. This causes unmultiplied color component values to be greater than one, causing overflows in the image encoder.

Fix is on the way...

Reminder: Please note that M66 Stable is only 7 days away. This bug has been marked as ReleaseBlock Stable for M66. So please take a look and appropriately address this bug. 

Comment 20 by kbr@chromium.org, Apr 12 2018

Thanks Justin for diagnosing this. I'm surprised that this WebGL-rendered canvas composites correctly. I think it would be fine if we required that the premultipliedAlpha:false context creation attribute be specified in order to make this test case work. We should write a WebGL conformance test for this.

Labels: -M-66 -Target-65 -Target-66
Since this is present in 65, my recommendation is to wait until 67 to target this fix. 
Project Member

Comment 22 by bugdroid1@chromium.org, Apr 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dcffdac67a98797ffadf4aa14ce979ceffbb8f53

commit dcffdac67a98797ffadf4aa14ce979ceffbb8f53
Author: Justin Novosad <junov@chromium.org>
Date: Mon Apr 16 16:01:15 2018

Fix alpha-unpremultiply overflow in WebGL+toDataURL

This change fixes a regression that was introduced in
https://chromium-review.googlesource.com/c/chromium/src/+/780279
Fixing the issue by switching sensitive use cases back to using
SkImage::readPixels for safely performing the unpremul conversion

BUG= 826878 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ie4bc9ced4d7175ab308529f7dd7119f93378e34c
Reviewed-on: https://chromium-review.googlesource.com/1003320
Commit-Queue: Justin Novosad <junov@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550979}
[modify] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow-expected.html
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow.html
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/platform/mac-mac10.12/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/platform/mac/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.txt
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip.html
[modify] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/blink/renderer/platform/graphics/image_data_buffer.cc

Comment 23 by junov@chromium.org, Apr 16 2018

Labels: Merge-Request-66
Requesting permission to merge to M66
Cl listed at #22, also need a merge to M67. Pls request a merge. 
Project Member

Comment 25 by sheriffbot@chromium.org, Apr 16 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: We are only 0 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-66 Merge-Rejected-66
Why is this critical for 66? This seems to be present in 65 also, and 66 is going to stable tomorrow. 

Comment 27 by junov@chromium.org, Apr 16 2018

Labels: Merge-Request-67
@#26: I agree, not critical enough for 66 at this point. Sorry for the noise.
Requesting permission to merge to 67.
Project Member

Comment 28 by bugdroid1@chromium.org, Apr 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b096dcaa618b138bdf937fa12de11c8774ed1ffc

commit b096dcaa618b138bdf937fa12de11c8774ed1ffc
Author: Avi Drissman <avi@chromium.org>
Date: Mon Apr 16 18:42:46 2018

Revert "Fix alpha-unpremultiply overflow in WebGL+toDataURL"

This reverts commit dcffdac67a98797ffadf4aa14ce979ceffbb8f53.

Reason for revert:
Breaks fast/webgl/canvas-toDataURL-premul-overflow.html on
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty/43191
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20Leak/17758
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20(dbg)/11632
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.10/45887
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.11/31889
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.12%20(retina)/328

Original change's description:
> Fix alpha-unpremultiply overflow in WebGL+toDataURL
> 
> This change fixes a regression that was introduced in
> https://chromium-review.googlesource.com/c/chromium/src/+/780279
> Fixing the issue by switching sensitive use cases back to using
> SkImage::readPixels for safely performing the unpremul conversion
> 
> BUG= 826878 
> 
> Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Change-Id: Ie4bc9ced4d7175ab308529f7dd7119f93378e34c
> Reviewed-on: https://chromium-review.googlesource.com/1003320
> Commit-Queue: Justin Novosad <junov@chromium.org>
> Reviewed-by: Fernando Serboncini <fserb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#550979}

TBR=junov@chromium.org,fserb@chromium.org,xlai@chromium.org

Change-Id: Ib53e4ac4573894ff93424b50af6314a51fee217a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  826878 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/1014321
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551050}
[modify] https://crrev.com/b096dcaa618b138bdf937fa12de11c8774ed1ffc/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow-expected.html
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow.html
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/platform/mac-mac10.12/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/platform/mac/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.txt
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip.html
[modify] https://crrev.com/b096dcaa618b138bdf937fa12de11c8774ed1ffc/third_party/blink/renderer/platform/graphics/image_data_buffer.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Apr 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/97fcb105c54add62b68a66cee930d361340df128

commit 97fcb105c54add62b68a66cee930d361340df128
Author: Justin Novosad <junov@chromium.org>
Date: Mon Apr 16 20:24:38 2018

Reland "Fix alpha-unpremultiply overflow in WebGL+toDataURL"

This reverts commit b096dcaa618b138bdf937fa12de11c8774ed1ffc.

Reason for reland: This was not the right CL to revert (it was not even in the failure regression range).  The source of the regression is https://chromium-review.googlesource.com/c/chromium/src/+/1014140, which was just reverted so that this can reland.  The CL adds the test that was regressed by the other CL. Because these two CLs were in CQ at the same time, the problem only got caught on the main waterfall.

Original change's description:
> Revert "Fix alpha-unpremultiply overflow in WebGL+toDataURL"
> 
> This reverts commit dcffdac67a98797ffadf4aa14ce979ceffbb8f53.
> 
> Reason for revert:
> Breaks fast/webgl/canvas-toDataURL-premul-overflow.html on
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty/43191
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20Leak/17758
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20(dbg)/11632
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.10/45887
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.11/31889
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.12%20(retina)/328
> 
> Original change's description:
> > Fix alpha-unpremultiply overflow in WebGL+toDataURL
> > 
> > This change fixes a regression that was introduced in
> > https://chromium-review.googlesource.com/c/chromium/src/+/780279
> > Fixing the issue by switching sensitive use cases back to using
> > SkImage::readPixels for safely performing the unpremul conversion
> > 
> > BUG= 826878 
> > 
> > Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> > Change-Id: Ie4bc9ced4d7175ab308529f7dd7119f93378e34c
> > Reviewed-on: https://chromium-review.googlesource.com/1003320
> > Commit-Queue: Justin Novosad <junov@chromium.org>
> > Reviewed-by: Fernando Serboncini <fserb@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#550979}
> 
> TBR=junov@chromium.org,fserb@chromium.org,xlai@chromium.org
> 
> Change-Id: Ib53e4ac4573894ff93424b50af6314a51fee217a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  826878 
> Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Reviewed-on: https://chromium-review.googlesource.com/1014321
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Avi Drissman <avi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#551050}

TBR=avi@chromium.org,junov@chromium.org,fserb@chromium.org,xlai@chromium.org

Change-Id: I01758853007f982b81f99494be2be717c7ff6096
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  826878 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/1014423
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551096}
[modify] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow-expected.html
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow.html
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/platform/mac-mac10.12/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/platform/mac/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.txt
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip.html
[modify] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/blink/renderer/platform/graphics/image_data_buffer.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dcffdac67a98797ffadf4aa14ce979ceffbb8f53

commit dcffdac67a98797ffadf4aa14ce979ceffbb8f53
Author: Justin Novosad <junov@chromium.org>
Date: Mon Apr 16 16:01:15 2018

Fix alpha-unpremultiply overflow in WebGL+toDataURL

This change fixes a regression that was introduced in
https://chromium-review.googlesource.com/c/chromium/src/+/780279
Fixing the issue by switching sensitive use cases back to using
SkImage::readPixels for safely performing the unpremul conversion

BUG= 826878 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ie4bc9ced4d7175ab308529f7dd7119f93378e34c
Reviewed-on: https://chromium-review.googlesource.com/1003320
Commit-Queue: Justin Novosad <junov@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550979}
[modify] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow-expected.html
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow.html
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/platform/mac-mac10.12/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/platform/mac/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.txt
[add] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip.html
[modify] https://crrev.com/dcffdac67a98797ffadf4aa14ce979ceffbb8f53/third_party/blink/renderer/platform/graphics/image_data_buffer.cc

Project Member

Comment 31 by bugdroid1@chromium.org, Apr 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b096dcaa618b138bdf937fa12de11c8774ed1ffc

commit b096dcaa618b138bdf937fa12de11c8774ed1ffc
Author: Avi Drissman <avi@chromium.org>
Date: Mon Apr 16 18:42:46 2018

Revert "Fix alpha-unpremultiply overflow in WebGL+toDataURL"

This reverts commit dcffdac67a98797ffadf4aa14ce979ceffbb8f53.

Reason for revert:
Breaks fast/webgl/canvas-toDataURL-premul-overflow.html on
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty/43191
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20Leak/17758
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20(dbg)/11632
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.10/45887
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.11/31889
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.12%20(retina)/328

Original change's description:
> Fix alpha-unpremultiply overflow in WebGL+toDataURL
> 
> This change fixes a regression that was introduced in
> https://chromium-review.googlesource.com/c/chromium/src/+/780279
> Fixing the issue by switching sensitive use cases back to using
> SkImage::readPixels for safely performing the unpremul conversion
> 
> BUG= 826878 
> 
> Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Change-Id: Ie4bc9ced4d7175ab308529f7dd7119f93378e34c
> Reviewed-on: https://chromium-review.googlesource.com/1003320
> Commit-Queue: Justin Novosad <junov@chromium.org>
> Reviewed-by: Fernando Serboncini <fserb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#550979}

TBR=junov@chromium.org,fserb@chromium.org,xlai@chromium.org

Change-Id: Ib53e4ac4573894ff93424b50af6314a51fee217a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  826878 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/1014321
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551050}
[modify] https://crrev.com/b096dcaa618b138bdf937fa12de11c8774ed1ffc/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow-expected.html
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow.html
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/platform/mac-mac10.12/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/platform/mac/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.txt
[delete] https://crrev.com/ee2a9e659d31fe969bf3198495cd12674f31f46b/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip.html
[modify] https://crrev.com/b096dcaa618b138bdf937fa12de11c8774ed1ffc/third_party/blink/renderer/platform/graphics/image_data_buffer.cc

Project Member

Comment 32 by bugdroid1@chromium.org, Apr 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/97fcb105c54add62b68a66cee930d361340df128

commit 97fcb105c54add62b68a66cee930d361340df128
Author: Justin Novosad <junov@chromium.org>
Date: Mon Apr 16 20:24:38 2018

Reland "Fix alpha-unpremultiply overflow in WebGL+toDataURL"

This reverts commit b096dcaa618b138bdf937fa12de11c8774ed1ffc.

Reason for reland: This was not the right CL to revert (it was not even in the failure regression range).  The source of the regression is https://chromium-review.googlesource.com/c/chromium/src/+/1014140, which was just reverted so that this can reland.  The CL adds the test that was regressed by the other CL. Because these two CLs were in CQ at the same time, the problem only got caught on the main waterfall.

Original change's description:
> Revert "Fix alpha-unpremultiply overflow in WebGL+toDataURL"
> 
> This reverts commit dcffdac67a98797ffadf4aa14ce979ceffbb8f53.
> 
> Reason for revert:
> Breaks fast/webgl/canvas-toDataURL-premul-overflow.html on
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty/43191
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20Leak/17758
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20(dbg)/11632
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.10/45887
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.11/31889
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.12%20(retina)/328
> 
> Original change's description:
> > Fix alpha-unpremultiply overflow in WebGL+toDataURL
> > 
> > This change fixes a regression that was introduced in
> > https://chromium-review.googlesource.com/c/chromium/src/+/780279
> > Fixing the issue by switching sensitive use cases back to using
> > SkImage::readPixels for safely performing the unpremul conversion
> > 
> > BUG= 826878 
> > 
> > Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> > Change-Id: Ie4bc9ced4d7175ab308529f7dd7119f93378e34c
> > Reviewed-on: https://chromium-review.googlesource.com/1003320
> > Commit-Queue: Justin Novosad <junov@chromium.org>
> > Reviewed-by: Fernando Serboncini <fserb@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#550979}
> 
> TBR=junov@chromium.org,fserb@chromium.org,xlai@chromium.org
> 
> Change-Id: Ib53e4ac4573894ff93424b50af6314a51fee217a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  826878 
> Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Reviewed-on: https://chromium-review.googlesource.com/1014321
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Avi Drissman <avi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#551050}

TBR=avi@chromium.org,junov@chromium.org,fserb@chromium.org,xlai@chromium.org

Change-Id: I01758853007f982b81f99494be2be717c7ff6096
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  826878 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/1014423
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551096}
[modify] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow-expected.html
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow.html
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/platform/mac-mac10.12/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/platform/mac/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.txt
[add] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip.html
[modify] https://crrev.com/97fcb105c54add62b68a66cee930d361340df128/third_party/blink/renderer/platform/graphics/image_data_buffer.cc

Project Member

Comment 33 by sheriffbot@chromium.org, Apr 18 2018

Labels: -Merge-Request-67 Merge-Review-67
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
There are multiple CLs listed here. Pls clarify which CLs need merge to M67, how safe they are? Also as this was regressed in M65, can't this simply wait until M68?

Comment 35 by junov@chromium.org, Apr 19 2018

The last CL (#32) is the one we'd merge.  The code changes are modest: just file image_data_buffer.cc, the rest of the CL is tests. The original CL was reverted by mistake, not because the CL broke stuff. 

Risk is low, but impact is not widespread so perhaps merge is unwarranted
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge for Cl listed at #32 to M67 branch 3396 based on comment #35. Please merge ASAP. Thank you.
Project Member

Comment 37 by bugdroid1@chromium.org, Apr 19 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8b034e1a56a9f2a64d8184a284db881a2e9cd056

commit 8b034e1a56a9f2a64d8184a284db881a2e9cd056
Author: Justin Novosad <junov@chromium.org>
Date: Thu Apr 19 19:06:20 2018

Reland "Fix alpha-unpremultiply overflow in WebGL+toDataURL"

This reverts commit b096dcaa618b138bdf937fa12de11c8774ed1ffc.

Reason for reland: This was not the right CL to revert (it was not even in the failure regression range).  The source of the regression is https://chromium-review.googlesource.com/c/chromium/src/+/1014140, which was just reverted so that this can reland.  The CL adds the test that was regressed by the other CL. Because these two CLs were in CQ at the same time, the problem only got caught on the main waterfall.

Original change's description:
> Revert "Fix alpha-unpremultiply overflow in WebGL+toDataURL"
>
> This reverts commit dcffdac67a98797ffadf4aa14ce979ceffbb8f53.
>
> Reason for revert:
> Breaks fast/webgl/canvas-toDataURL-premul-overflow.html on
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty/43191
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20Leak/17758
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20(dbg)/11632
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.10/45887
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.11/31889
> https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.12%20(retina)/328
>
> Original change's description:
> > Fix alpha-unpremultiply overflow in WebGL+toDataURL
> >
> > This change fixes a regression that was introduced in
> > https://chromium-review.googlesource.com/c/chromium/src/+/780279
> > Fixing the issue by switching sensitive use cases back to using
> > SkImage::readPixels for safely performing the unpremul conversion
> >
> > BUG= 826878 
> >
> > Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> > Change-Id: Ie4bc9ced4d7175ab308529f7dd7119f93378e34c
> > Reviewed-on: https://chromium-review.googlesource.com/1003320
> > Commit-Queue: Justin Novosad <junov@chromium.org>
> > Reviewed-by: Fernando Serboncini <fserb@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#550979}
>
> TBR=junov@chromium.org,fserb@chromium.org,xlai@chromium.org
>
> Change-Id: Ib53e4ac4573894ff93424b50af6314a51fee217a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  826878 
> Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Reviewed-on: https://chromium-review.googlesource.com/1014321
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Avi Drissman <avi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#551050}

TBR=avi@chromium.org, fserb@chromium.org, junov@chromium.org, xlai@chromium.org

(cherry picked from commit 97fcb105c54add62b68a66cee930d361340df128)

Change-Id: I01758853007f982b81f99494be2be717c7ff6096
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  826878 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/1014423
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Justin Novosad <junov@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#551096}
Reviewed-on: https://chromium-review.googlesource.com/1020078
Cr-Commit-Position: refs/branch-heads/3396@{#141}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/8b034e1a56a9f2a64d8184a284db881a2e9cd056/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/8b034e1a56a9f2a64d8184a284db881a2e9cd056/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow-expected.html
[add] https://crrev.com/8b034e1a56a9f2a64d8184a284db881a2e9cd056/third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow.html
[add] https://crrev.com/8b034e1a56a9f2a64d8184a284db881a2e9cd056/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/8b034e1a56a9f2a64d8184a284db881a2e9cd056/third_party/WebKit/LayoutTests/platform/mac-mac10.12/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/8b034e1a56a9f2a64d8184a284db881a2e9cd056/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/8b034e1a56a9f2a64d8184a284db881a2e9cd056/third_party/WebKit/LayoutTests/platform/mac/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/8b034e1a56a9f2a64d8184a284db881a2e9cd056/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.png
[add] https://crrev.com/8b034e1a56a9f2a64d8184a284db881a2e9cd056/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip-expected.txt
[add] https://crrev.com/8b034e1a56a9f2a64d8184a284db881a2e9cd056/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip.html
[modify] https://crrev.com/8b034e1a56a9f2a64d8184a284db881a2e9cd056/third_party/blink/renderer/platform/graphics/image_data_buffer.cc

Comment 38 by junov@chromium.org, Apr 20 2018

Status: Fixed (was: Started)

Sign in to add a comment