Fix bug for createImageBitmap(HTMLImageElement, premultiplyAlpha=false)

Fix bug for createImageBitmap(HTMLImageElement, premultiplyAlpha=false)

When creating an ImageBitmap from an HTMLImageElement, it calls the
cropImage() function in ImageBitmap class, and it should not get into
the block of using ImageDecoder, because the SkImage that can be obtained
from image->imageForCurrentFrame() is already un-premultiplied
in the case of HTMLImageElement.

Locally I have WebGL conformance and conformance2 tests that pass
with this change but fail without this change.

BUG=591263
Committed: https://crrev.com/2160ae0126d1adc88ec8abe5f97291119cb42d68
Cr-Commit-Position: refs/heads/master@{#378864}

[xidachen, 2016-03-01 21:41:43]

The CQ bit was checked by xidachen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-01 21:44:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1756593002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1756593002/20001

[xidachen, 2016-03-01 21:46:43]

Description was changed from

==========
Fix bug for createImageBitmap(HTMLImageElement, premultiplyAlpha=false)

When creating an ImageBitmap from an HTMLImageElement, it calls the
cropImage() function in ImageBitmap class, and it should not get into
the block of ImageDecoder, instead it should return a subregion of the
source directly.
==========

to

==========
Fix bug for createImageBitmap(HTMLImageElement, premultiplyAlpha=false)

When creating an ImageBitmap from an HTMLImageElement, it calls the
cropImage() function in ImageBitmap class, and it should not get into
the block of ImageDecoder, instead it should return a subregion of the
source directly.

Locally I have WebGL conformance and conformance2 tests that passes
with this change but fails without this change.
==========

[xidachen, 2016-03-01 21:46:56]

Description was changed from

==========
Fix bug for createImageBitmap(HTMLImageElement, premultiplyAlpha=false)

When creating an ImageBitmap from an HTMLImageElement, it calls the
cropImage() function in ImageBitmap class, and it should not get into
the block of ImageDecoder, instead it should return a subregion of the
source directly.

Locally I have WebGL conformance and conformance2 tests that passes
with this change but fails without this change.
==========

to

==========
Fix bug for createImageBitmap(HTMLImageElement, premultiplyAlpha=false)

When creating an ImageBitmap from an HTMLImageElement, it calls the
cropImage() function in ImageBitmap class, and it should not get into
the block of ImageDecoder, instead it should return a subregion of the
source directly.

Locally I have WebGL conformance and conformance2 tests that pass
with this change but fail without this change.
==========

[commit-bot: I haz the power, 2016-03-01 23:34:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-01 23:34:04]

Dry run: This issue passed the CQ dry run.

[xidachen, 2016-03-02 02:18:52]

Description was changed from

==========
Fix bug for createImageBitmap(HTMLImageElement, premultiplyAlpha=false)

When creating an ImageBitmap from an HTMLImageElement, it calls the
cropImage() function in ImageBitmap class, and it should not get into
the block of ImageDecoder, instead it should return a subregion of the
source directly.

Locally I have WebGL conformance and conformance2 tests that pass
with this change but fail without this change.
==========

to

==========
Fix bug for createImageBitmap(HTMLImageElement, premultiplyAlpha=false)

When creating an ImageBitmap from an HTMLImageElement, it calls the
cropImage() function in ImageBitmap class, and it should not get into
the block of using ImageDecoder, because the SkImage that can be obtained
from image->imageForCurrentFrame() is already un-premultiplied 
in the case of HTMLImageElement.

Locally I have WebGL conformance and conformance2 tests that pass
with this change but fail without this change.

BUG=591263
==========

[xidachen, 2016-03-02 02:19:29]

xidachen@chromium.org changed reviewers:
+ junov@chromium.org

[xidachen, 2016-03-02 02:19:29]

PTAL.

[Justin Novosad, 2016-03-02 17:19:25]

https://codereview.chromium.org/1756593002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/ImageBitmap.cpp (right):

https://codereview.chromium.org/1756593002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/ImageBitmap.cpp:168: m_image =
cropImage(image->cachedImage()->image(), cropRect, flipY, m_isPremultiplied,
false);
Poor readability.  You have two options:
1) Change the bool argument to use an enum to make litteral arguments at the
call site more readable.
2) Use an intermediate variable: 
bool isBitmapPremultiplied = false
cropImage(..., isBitmapPremultiplied);

1) is generally preferred especially for public methods, but might be overkill
in this case.

[xidachen, 2016-03-02 18:00:57]

https://codereview.chromium.org/1756593002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/ImageBitmap.cpp (right):

https://codereview.chromium.org/1756593002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/ImageBitmap.cpp:168: m_image =
cropImage(image->cachedImage()->image(), cropRect, flipY, m_isPremultiplied,
false);
On 2016/03/02 17:19:25, Justin Novosad wrote:
> Poor readability.  You have two options:
> 1) Change the bool argument to use an enum to make litteral arguments at the
> call site more readable.
> 2) Use an intermediate variable: 
> bool isBitmapPremultiplied = false
> cropImage(..., isBitmapPremultiplied);
> 
> 1) is generally preferred especially for public methods, but might be overkill
> in this case.

Agreed, using enum makes more sense IMO.

[Justin Novosad, 2016-03-02 19:40:51]

On 2016/03/02 18:00:57, xidachen wrote:
>
https://codereview.chromium.org/1756593002/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/frame/ImageBitmap.cpp (right):
> 
>
https://codereview.chromium.org/1756593002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/frame/ImageBitmap.cpp:168: m_image =
> cropImage(image->cachedImage()->image(), cropRect, flipY, m_isPremultiplied,
> false);
> On 2016/03/02 17:19:25, Justin Novosad wrote:
> > Poor readability.  You have two options:
> > 1) Change the bool argument to use an enum to make litteral arguments at the
> > call site more readable.
> > 2) Use an intermediate variable: 
> > bool isBitmapPremultiplied = false
> > cropImage(..., isBitmapPremultiplied);
> > 
> > 1) is generally preferred especially for public methods, but might be
overkill
> > in this case.
> 
> Agreed, using enum makes more sense IMO.

Hah, and we already had one for this!

lgtm

[xidachen, 2016-03-02 19:42:00]

The CQ bit was checked by xidachen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-02 19:42:41]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1756593002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1756593002/60001

[xidachen, 2016-03-02 20:54:27]

The CQ bit was unchecked by xidachen@chromium.org

[xidachen, 2016-03-02 20:54:51]

The CQ bit was checked by xidachen@chromium.org

[commit-bot: I haz the power, 2016-03-02 20:55:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1756593002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1756593002/60001

[commit-bot: I haz the power, 2016-03-02 22:46:07]

Description was changed from

==========
Fix bug for createImageBitmap(HTMLImageElement, premultiplyAlpha=false)

When creating an ImageBitmap from an HTMLImageElement, it calls the
cropImage() function in ImageBitmap class, and it should not get into
the block of using ImageDecoder, because the SkImage that can be obtained
from image->imageForCurrentFrame() is already un-premultiplied 
in the case of HTMLImageElement.

Locally I have WebGL conformance and conformance2 tests that pass
with this change but fail without this change.

BUG=591263
==========

to

==========
Fix bug for createImageBitmap(HTMLImageElement, premultiplyAlpha=false)

When creating an ImageBitmap from an HTMLImageElement, it calls the
cropImage() function in ImageBitmap class, and it should not get into
the block of using ImageDecoder, because the SkImage that can be obtained
from image->imageForCurrentFrame() is already un-premultiplied 
in the case of HTMLImageElement.

Locally I have WebGL conformance and conformance2 tests that pass
with this change but fail without this change.

BUG=591263
==========

[commit-bot: I haz the power, 2016-03-02 22:46:09]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-02 22:47:20]

Description was changed from

==========
Fix bug for createImageBitmap(HTMLImageElement, premultiplyAlpha=false)

When creating an ImageBitmap from an HTMLImageElement, it calls the
cropImage() function in ImageBitmap class, and it should not get into
the block of using ImageDecoder, because the SkImage that can be obtained
from image->imageForCurrentFrame() is already un-premultiplied 
in the case of HTMLImageElement.

Locally I have WebGL conformance and conformance2 tests that pass
with this change but fail without this change.

BUG=591263
==========

to

==========
Fix bug for createImageBitmap(HTMLImageElement, premultiplyAlpha=false)

When creating an ImageBitmap from an HTMLImageElement, it calls the
cropImage() function in ImageBitmap class, and it should not get into
the block of using ImageDecoder, because the SkImage that can be obtained
from image->imageForCurrentFrame() is already un-premultiplied
in the case of HTMLImageElement.

Locally I have WebGL conformance and conformance2 tests that pass
with this change but fail without this change.

BUG=591263

Committed: https://crrev.com/2160ae0126d1adc88ec8abe5f97291119cb42d68
Cr-Commit-Position: refs/heads/master@{#378864}
==========

[commit-bot: I haz the power, 2016-03-02 22:47:21]

Patchset 4 (id:??) landed as
https://crrev.com/2160ae0126d1adc88ec8abe5f97291119cb42d68
Cr-Commit-Position: refs/heads/master@{#378864}