Pass SkPaint instead of its alpha and mode in WebMediaPlayer::paint

Pass SkPaint instead of its alpha and mode in WebMediaPlayer::paint

At this moment, the WebMediaPlayer::paint takes the alpha and SkXferMode
of the paint, and drop all the other properties of the SkPaint. We should
pass the whole SkPaint as the argument instead of passing only two of its
properties. Doing this will avoid other properties such as filter quality
gets dropped

BUG=640214, 456529
TBR=qinmin@chromium.org, esprehn@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/0ebd94d53de1de4631466ae8497c2515c1d0ecd7
Cr-Commit-Position: refs/heads/master@{#416942}

[xidachen, 2016-08-24 15:56:52]

Description was changed from

==========
Pass SkPaint instead of its alpha and mode in WebMediaPlayer::paint

At this moment, the WebMediaPlayer::paint takes the alpha and SkXferMode
of the paint, and drop all the other properties of the SkPaint. We should
pass the whole SkPaint as the argument instead of passing only two of its
properties. Doing this will avoid other properties such as filter quality
gets dropped

BUG=640214, 456529
==========

to

==========
Pass SkPaint instead of its alpha and mode in WebMediaPlayer::paint

At this moment, the WebMediaPlayer::paint takes the alpha and SkXferMode
of the paint, and drop all the other properties of the SkPaint. We should
pass the whole SkPaint as the argument instead of passing only two of its
properties. Doing this will avoid other properties such as filter quality
gets dropped

BUG=640214, 456529
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[xidachen, 2016-08-25 18:49:17]

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

[xidachen, 2016-08-25 18:49:17]

PTAL

junov@: does the expected result for canvas-composite-video-shadow make sense?
Specially the row "copy"?

[Justin Novosad, 2016-08-25 19:18:19]

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:335: const SkPaint* paint,
since the code is not mean to support nullptr for paint, you should use &
instead of *

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:346: SkPaint freshPaint;
this is a funny name.  What do you mean by fresh?

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:439: paint.setAlpha(0xFF);
not necessary

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:440:
paint.setFilterQuality(kLow_SkFilterQuality);
not necessary

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer_unittest.cc (right):

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer_unittest.cc:210:
paint.setXfermodeMode(SkXfermode::kSrc_Mode);
You changed SrcOver to Src. SrcOver is the default, so you don't even need this
line.

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer_unittest.cc:211: paint.setAlpha(0xFF);
This is a no-op. 0xFF is the default

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer_unittest.cc:212:
paint.setFilterQuality(kLow_SkFilterQuality);
This is also a no-op

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer_unittest.cc:548:
paint.setXfermodeMode(SkXfermode::kSrc_Mode);
You change this from SrcOver to Src. Accident?

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer_unittest.cc:550:
paint.setFilterQuality(kLow_SkFilterQuality);
These setting are the defaults; You could just use a default SkPaint.

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:705: # The failure in the next
line is due to virtual/gpu bots using osmesa
Explain that it is becaus under osmesa, skia's filter quality does not work
correctly. State that these tests are expected to pass when we switch to
swiftshader.

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:706: crbug.com/640214 [ Linux
Mac Win ]
virtual/gpu/fast/canvas/canvas-drawImage-video-imageSmoothingEnabled.html [
Failure ]
For failure expectation, do not associate the test failure with the crbug of the
CL that introduced the failure.  Instead, associate it with a new crbug that is
about resolving the failure.  Basically, we should never end up in a state where
a line in TestExpectations is associated with a closed issue.

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/canvas/canvas-drawImage-video-imageSmoothingEnabled.html
(right):

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/canvas-drawImage-video-imageSmoothingEnabled.html:46:
}, 'createImageBitmap from a HTMLVideoElement with resize option.');
Wrong text here.

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebMediaPlayer.h (right):

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebMediaPlayer.h:40: #include
"third_party/skia/include/core/SkPaint.h"
No need to include.  Just forward-declare.

[xidachen, 2016-08-26 02:21:39]

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:346: SkPaint freshPaint;
On 2016/08/25 19:18:19, Justin Novosad wrote:
> this is a funny name.  What do you mean by fresh?

It is basically explaining that we are creating a new skpaint instead of using
the one that is passed in. Should I name it "newPaint"?

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:439: paint.setAlpha(0xFF);
On 2016/08/25 19:18:18, Justin Novosad wrote:
> not necessary

Done.

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:440:
paint.setFilterQuality(kLow_SkFilterQuality);
On 2016/08/25 19:18:18, Justin Novosad wrote:
> not necessary

But without this line, the default filter quality is kNone.

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer_unittest.cc (right):

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer_unittest.cc:210:
paint.setXfermodeMode(SkXfermode::kSrc_Mode);
On 2016/08/25 19:18:19, Justin Novosad wrote:
> You changed SrcOver to Src. SrcOver is the default, so you don't even need
this
> line.

Done.

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer_unittest.cc:211: paint.setAlpha(0xFF);
On 2016/08/25 19:18:19, Justin Novosad wrote:
> This is a no-op. 0xFF is the default

Done.

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer_unittest.cc:212:
paint.setFilterQuality(kLow_SkFilterQuality);
On 2016/08/25 19:18:19, Justin Novosad wrote:
> This is also a no-op

I believe the default filter quality is kNone.

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer_unittest.cc:548:
paint.setXfermodeMode(SkXfermode::kSrc_Mode);
On 2016/08/25 19:18:19, Justin Novosad wrote:
> You change this from SrcOver to Src. Accident?

Done.

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:705: # The failure in the next
line is due to virtual/gpu bots using osmesa
On 2016/08/25 19:18:19, Justin Novosad wrote:
> Explain that it is becaus under osmesa, skia's filter quality does not work
> correctly. State that these tests are expected to pass when we switch to
> swiftshader.

Done.

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:706: crbug.com/640214 [ Linux
Mac Win ]
virtual/gpu/fast/canvas/canvas-drawImage-video-imageSmoothingEnabled.html [
Failure ]
On 2016/08/25 19:18:19, Justin Novosad wrote:
> For failure expectation, do not associate the test failure with the crbug of
the
> CL that introduced the failure.  Instead, associate it with a new crbug that
is
> about resolving the failure.  Basically, we should never end up in a state
where
> a line in TestExpectations is associated with a closed issue.

Done.

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/canvas/canvas-drawImage-video-imageSmoothingEnabled.html
(right):

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/canvas-drawImage-video-imageSmoothingEnabled.html:46:
}, 'createImageBitmap from a HTMLVideoElement with resize option.');
On 2016/08/25 19:18:19, Justin Novosad wrote:
> Wrong text here.

Done.

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebMediaPlayer.h (right):

https://codereview.chromium.org/2276033002/diff/60001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebMediaPlayer.h:40: #include
"third_party/skia/include/core/SkPaint.h"
On 2016/08/25 19:18:19, Justin Novosad wrote:
> No need to include.  Just forward-declare.

Done.

[Justin Novosad, 2016-08-26 14:25:25]

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:346: SkPaint freshPaint;
On 2016/08/26 02:21:39, xidachen wrote:
> On 2016/08/25 19:18:19, Justin Novosad wrote:
> > this is a funny name.  What do you mean by fresh?
> 
> It is basically explaining that we are creating a new skpaint instead of using
> the one that is passed in. Should I name it "newPaint"?

Think of the person reading this code trying to understand it.  There is a
reason you are not using "paint" directly.  You only want to use part of the
settings in "paint".  This code sets up a paint and changes it further later on,
which is also bad for readability.  I would encourage you to us two paint
objects.  Call the first one  blackWithAlphaPaint, and create it inside the "if"
statement that uses it. The second one could be videoPaint, or something like
that to imply that it was construct with specifically the properties needed for
the video blit.

[xidachen, 2016-08-26 14:44:12]

On 2016/08/26 14:25:25, Justin Novosad wrote:
>
https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
> File media/renderers/skcanvas_video_renderer.cc (right):
> 
>
https://codereview.chromium.org/2276033002/diff/60001/media/renderers/skcanva...
> media/renderers/skcanvas_video_renderer.cc:346: SkPaint freshPaint;
> On 2016/08/26 02:21:39, xidachen wrote:
> > On 2016/08/25 19:18:19, Justin Novosad wrote:
> > > this is a funny name.  What do you mean by fresh?
> > 
> > It is basically explaining that we are creating a new skpaint instead of
using
> > the one that is passed in. Should I name it "newPaint"?
> 
> Think of the person reading this code trying to understand it.  There is a
> reason you are not using "paint" directly.  You only want to use part of the
> settings in "paint".  This code sets up a paint and changes it further later
on,
> which is also bad for readability.  I would encourage you to us two paint
> objects.  Call the first one  blackWithAlphaPaint, and create it inside the
"if"
> statement that uses it. The second one could be videoPaint, or something like
> that to imply that it was construct with specifically the properties needed
for
> the video blit.

Thanks for the suggestions. I have updated the naming for the paint.

One last concern:

could you take a look at the "canvas-composite-video-shadow-expected.png" in
this CL? I have a feeling that the "copy" row isn't correct because it doesn't
show any shadow there.

[Justin Novosad, 2016-08-31 20:49:22]

> 
> could you take a look at the "canvas-composite-video-shadow-expected.png" in
> this CL? I have a feeling that the "copy" row isn't correct because it doesn't
> show any shadow there.

Not showing a shodow in "copy" mode is the spec compliant behavior.  The way
copy mode is spec'ed, the primitive is drawn onto an infinite plane, that is
then composited with a "src" operator, which means that every pixel in the
current clip is over written.  The way shadows are spec'ed, bot the shadow and
the foreground are rendered as separate passes and are composited independently
of each other. Therefore, drawing the foreground effectively overwrites the
shadow.

lgtm

[xidachen, 2016-09-06 12:19:48]

ping ajuma@: could you take a look at cc changes, which are relatively small.

[ajuma, 2016-09-06 13:15:34]

On 2016/09/06 12:19:48, xidachen wrote:
> ping ajuma@: could you take a look at cc changes, which are relatively small.

I don't see any changes in cc/, what am I missing? :)

[xidachen, 2016-09-06 13:25:16]

On 2016/09/06 13:15:34, ajuma wrote:
> On 2016/09/06 12:19:48, xidachen wrote:
> > ping ajuma@: could you take a look at cc changes, which are relatively
small.
> 
> I don't see any changes in cc/, what am I missing? :)

Oh, sorry, my bad.

[xidachen, 2016-09-06 13:27:21]

xidachen@chromium.org changed reviewers:
+ chcunningham@chromium.org, esprehn@chromium.org
- ajuma@chromium.org

[xidachen, 2016-09-06 13:27:21]

esprehn@: content/renderer/, for Blink API.

chcunningham@: media/

PTAL. The changes are mostly plumbing.

[chcunningham, 2016-09-06 23:07:33]

LGTM % nits

https://codereview.chromium.org/2276033002/diff/100001/content/renderer/media...
File content/renderer/media/html_video_element_capturer_source.cc (right):

https://codereview.chromium.org/2276033002/diff/100001/content/renderer/media...
content/renderer/media/html_video_element_capturer_source.cc:140:
paint.setFilterQuality(kLow_SkFilterQuality);
Are you missing a call to setAlpha(0xFF)? Does this line replace that call?

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer.cc (left):

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:435: Paint(video_frame, canvas,
gfx::RectF(video_frame->visible_rect()), 0xff,
ditto - safe to not set alpha?

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:365: if
(!SkXfermode::AsMode(paint.getXfermode(), &mode))
weird - the getter and setter are different types?

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer_unittest.cc (right):

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer_unittest.cc:209: SkPaint paint;
set alpha? set mode?

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer_unittest.cc:545: SkPaint paint;
set alpha/mode?

[xidachen, 2016-09-07 12:28:44]

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

[commit-bot: I haz the power, 2016-09-07 12:28:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 12:38:31]

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

[commit-bot: I haz the power, 2016-09-07 12:38:32]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[xidachen, 2016-09-07 13:23:08]

https://codereview.chromium.org/2276033002/diff/100001/content/renderer/media...
File content/renderer/media/html_video_element_capturer_source.cc (right):

https://codereview.chromium.org/2276033002/diff/100001/content/renderer/media...
content/renderer/media/html_video_element_capturer_source.cc:140:
paint.setFilterQuality(kLow_SkFilterQuality);
On 2016/09/06 23:07:33, chcunningham wrote:
> Are you missing a call to setAlpha(0xFF)? Does this line replace that call?

The 0xFF is by default when the SkPaint is defined. As you can see in this line:
https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkPaint.cpp?sq...

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer.cc (left):

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:435: Paint(video_frame, canvas,
gfx::RectF(video_frame->visible_rect()), 0xff,
On 2016/09/06 23:07:33, chcunningham wrote:
> ditto - safe to not set alpha?

0xff automatically set in the constructor of SkPaint.

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:365: if
(!SkXfermode::AsMode(paint.getXfermode(), &mode))
On 2016/09/06 23:07:33, chcunningham wrote:
> weird - the getter and setter are different types?

Yes, getXfermode() returns a SkXfermode*. The class SkXfermode contains a enum
named Mode, so next we are calling setXfermodeMode not setXfermode.

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer_unittest.cc (right):

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer_unittest.cc:209: SkPaint paint;
On 2016/09/06 23:07:33, chcunningham wrote:
> set alpha? set mode?

The mode is by default kSrcOver_Mode. So in Skpaint's constructor, there is a
sk_sp<SkXfermode> which is initialized to be nullptr, and nullptr is treated as
kSrcOver_Mode.

https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkXfermode.cpp...

[xidachen, 2016-09-07 13:23:19]

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

[commit-bot: I haz the power, 2016-09-07 13:23:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 13:40:11]

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

[commit-bot: I haz the power, 2016-09-07 13:40:12]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[xidachen, 2016-09-07 14:13:37]

Description was changed from

==========
Pass SkPaint instead of its alpha and mode in WebMediaPlayer::paint

At this moment, the WebMediaPlayer::paint takes the alpha and SkXferMode
of the paint, and drop all the other properties of the SkPaint. We should
pass the whole SkPaint as the argument instead of passing only two of its
properties. Doing this will avoid other properties such as filter quality
gets dropped

BUG=640214, 456529
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Pass SkPaint instead of its alpha and mode in WebMediaPlayer::paint

At this moment, the WebMediaPlayer::paint takes the alpha and SkXferMode
of the paint, and drop all the other properties of the SkPaint. We should
pass the whole SkPaint as the argument instead of passing only two of its
properties. Doing this will avoid other properties such as filter quality
gets dropped

BUG=640214, 456529
TBR=qinmin@chromium.org, esprehn@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[xidachen, 2016-09-07 14:13:37]

xidachen@chromium.org changed reviewers:
- esprehn@chromium.org

[xidachen, 2016-09-07 14:21:32]

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

[commit-bot: I haz the power, 2016-09-07 14:21:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xidachen, 2016-09-07 15:37:18]

The CQ bit was unchecked by xidachen@chromium.org

[xidachen, 2016-09-07 15:39:04]

The CQ bit was checked by xidachen@chromium.org

[xidachen, 2016-09-07 15:39:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
chcunningham@chromium.org, junov@chromium.org
Link to the patchset: https://codereview.chromium.org/2276033002/#ps160001
(title: "android should compile")

[commit-bot: I haz the power, 2016-09-07 15:39:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 15:47:42]

Description was changed from

==========
Pass SkPaint instead of its alpha and mode in WebMediaPlayer::paint

At this moment, the WebMediaPlayer::paint takes the alpha and SkXferMode
of the paint, and drop all the other properties of the SkPaint. We should
pass the whole SkPaint as the argument instead of passing only two of its
properties. Doing this will avoid other properties such as filter quality
gets dropped

BUG=640214, 456529
TBR=qinmin@chromium.org, esprehn@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Pass SkPaint instead of its alpha and mode in WebMediaPlayer::paint

At this moment, the WebMediaPlayer::paint takes the alpha and SkXferMode
of the paint, and drop all the other properties of the SkPaint. We should
pass the whole SkPaint as the argument instead of passing only two of its
properties. Doing this will avoid other properties such as filter quality
gets dropped

BUG=640214, 456529
TBR=qinmin@chromium.org, esprehn@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[commit-bot: I haz the power, 2016-09-07 15:47:43]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-09-07 15:49:51]

Description was changed from

==========
Pass SkPaint instead of its alpha and mode in WebMediaPlayer::paint

At this moment, the WebMediaPlayer::paint takes the alpha and SkXferMode
of the paint, and drop all the other properties of the SkPaint. We should
pass the whole SkPaint as the argument instead of passing only two of its
properties. Doing this will avoid other properties such as filter quality
gets dropped

BUG=640214, 456529
TBR=qinmin@chromium.org, esprehn@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Pass SkPaint instead of its alpha and mode in WebMediaPlayer::paint

At this moment, the WebMediaPlayer::paint takes the alpha and SkXferMode
of the paint, and drop all the other properties of the SkPaint. We should
pass the whole SkPaint as the argument instead of passing only two of its
properties. Doing this will avoid other properties such as filter quality
gets dropped

BUG=640214, 456529
TBR=qinmin@chromium.org, esprehn@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/0ebd94d53de1de4631466ae8497c2515c1d0ecd7
Cr-Commit-Position: refs/heads/master@{#416942}
==========

[commit-bot: I haz the power, 2016-09-07 15:49:53]

Patchset 9 (id:??) landed as
https://crrev.com/0ebd94d53de1de4631466ae8497c2515c1d0ecd7
Cr-Commit-Position: refs/heads/master@{#416942}

[chcunningham, 2016-09-07 16:01:13]

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer_unittest.cc (right):

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer_unittest.cc:245: paint.setAlpha(0xFF);
I see, I was thrown off by this one. I guess its probably not needed?

[xidachen, 2016-09-07 16:10:53]

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer_unittest.cc (right):

https://codereview.chromium.org/2276033002/diff/100001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer_unittest.cc:245: paint.setAlpha(0xFF);
On 2016/09/07 16:01:12, chcunningham wrote:
> I see, I was thrown off by this one. I guess its probably not needed?

Oh, damn! I totally missed it. Yes, this line is not needed. Now that I have
committed this CL, I will have a clean up for it.