color: Fix accidental partial color conversion

color: Fix accidental partial color conversion

For color spaces that can't be represented by an matrix plus analytic
transfer function, we need to ensure that we don't accidentally enable
color conversion when the destination is undefined.

Tests may be disruptive and are in a separate patch.

BUG=694191

[ccameron, 2017-02-21 22:02:40]

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

[ccameron, 2017-02-21 22:03:34]

ccameron@chromium.org changed reviewers:
+ hubbe@chromium.org

[ccameron, 2017-02-21 22:03:34]

I decided to separate out the tests and the patch, since the fix is wanted for
tonight's build, and the tests are disruptive.

[commit-bot: I haz the power, 2017-02-21 22:05:11]

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

[hubbe, 2017-02-21 22:46:49]

https://codereview.chromium.org/2712533002/diff/1/ui/gfx/color_transform.cc
File ui/gfx/color_transform.cc (right):

https://codereview.chromium.org/2712533002/diff/1/ui/gfx/color_transform.cc#n...
ui/gfx/color_transform.cc:735: // If the dst profile is not set, then do only
YUV to RGB conversion, so
I'm afraid I don't get this.
When does this happen?

[commit-bot: I haz the power, 2017-02-22 00:07:59]

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

[commit-bot: I haz the power, 2017-02-22 00:07:59]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[ccameron, 2017-02-22 00:08:29]

https://codereview.chromium.org/2712533002/diff/1/ui/gfx/color_transform.cc
File ui/gfx/color_transform.cc (right):

https://codereview.chromium.org/2712533002/diff/1/ui/gfx/color_transform.cc#n...
ui/gfx/color_transform.cc:735: // If the dst profile is not set, then do only
YUV to RGB conversion, so
On 2017/02/21 22:46:49, hubbe wrote:
> I'm afraid I don't get this.
> When does this happen?

This happens whenever media::kVideoColorManagement is not true and color
conversion is not enabled.

In that case we have ColorTransform do the YUV->RGB conversion for us (instead
of doing it manually in GLRenderer).

[hubbe, 2017-02-22 00:10:05]

lgtm

https://codereview.chromium.org/2712533002/diff/1/ui/gfx/color_transform.cc
File ui/gfx/color_transform.cc (right):

https://codereview.chromium.org/2712533002/diff/1/ui/gfx/color_transform.cc#n...
ui/gfx/color_transform.cc:735: // If the dst profile is not set, then do only
YUV to RGB conversion, so
On 2017/02/22 00:08:29, ccameron wrote:
> On 2017/02/21 22:46:49, hubbe wrote:
> > I'm afraid I don't get this.
> > When does this happen?
> 
> This happens whenever media::kVideoColorManagement is not true and color
> conversion is not enabled.
> 
> In that case we have ColorTransform do the YUV->RGB conversion for us (instead
> of doing it manually in GLRenderer).

Can we add some more meat to the comment to describe this?

[ccameron, 2017-02-22 00:30:36]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2017-02-22 00:30:36]

The patchset sent to the CQ was uploaded after l-g-t-m from hubbe@chromium.org
Link to the patchset: https://codereview.chromium.org/2712533002/#ps40001
(title: "More comments")

[ccameron, 2017-02-22 00:30:49]

https://codereview.chromium.org/2712533002/diff/1/ui/gfx/color_transform.cc
File ui/gfx/color_transform.cc (right):

https://codereview.chromium.org/2712533002/diff/1/ui/gfx/color_transform.cc#n...
ui/gfx/color_transform.cc:735: // If the dst profile is not set, then do only
YUV to RGB conversion, so
On 2017/02/22 00:10:05, hubbe wrote:
> On 2017/02/22 00:08:29, ccameron wrote:
> > On 2017/02/21 22:46:49, hubbe wrote:
> > > I'm afraid I don't get this.
> > > When does this happen?
> > 
> > This happens whenever media::kVideoColorManagement is not true and color
> > conversion is not enabled.
> > 
> > In that case we have ColorTransform do the YUV->RGB conversion for us
(instead
> > of doing it manually in GLRenderer).
> 
> Can we add some more meat to the comment to describe this?

Done.

[commit-bot: I haz the power, 2017-02-22 00:31:41]

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, 2017-02-22 02:37:33]

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

[commit-bot: I haz the power, 2017-02-22 02:37:34]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)