Color: Clean up color profile handling for layout tests

Color: Clean up color profile handling for layout tests

Allow layout tests to manually specify an output ICC profile
through the ContentRendererClient.

Match existing behavior on all platforms. This means:
- on Mac, using a generic RGB profile
- on Linux, not specifying a profile
- on Windows, leave this as TODO, since correct behavior is unclear

Remove layout_test_helper on Mac, since its only purpose was
to set the system color space to the now-specified color space.
Remove copyright exceptions for layout_test_helper as well.

BUG=44872
Committed: https://crrev.com/41495f2551c63bea769afd6cc70f1c3593f3537f
Cr-Commit-Position: refs/heads/master@{#416170}

[ccameron, 2016-08-09 01:03:32]

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

[commit-bot: I haz the power, 2016-08-09 01:04:07]

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

[ccameron, 2016-08-09 01:08:05]

Description was changed from

==========
Color: Manually override color profile for layout tests




BUG=44872
==========

to

==========
Color: Manually override color profile for layout tests

Don't change the system color profile when running layout tests
on Mac.

Manually specify an output ICC profile through the
ContentRendererClient.

BUG=44872
==========

[commit-bot: I haz the power, 2016-08-09 01:12:55]

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

[commit-bot: I haz the power, 2016-08-09 01:12:56]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[ccameron, 2016-08-09 02:07:07]

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

[commit-bot: I haz the power, 2016-08-09 02:07:46]

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

[ccameron, 2016-08-09 06:03:53]

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

[commit-bot: I haz the power, 2016-08-09 06:04:00]

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

[ccameron, 2016-08-09 06:13:24]

Description was changed from

==========
Color: Manually override color profile for layout tests

Don't change the system color profile when running layout tests
on Mac.

Manually specify an output ICC profile through the
ContentRendererClient.

BUG=44872
==========

to

==========
Color: Clean up color profile handling for layout tests

Allow layout tests to manually specify an output ICC profile
through the ContentRendererClient.

Match existing behavior on all platforms. This means:
- on Mac, using a generic RGB profile
- on Linux, not specifying a profile
- on Windows, leave this as TODO, since correct behavior is unclear

Remove layout_test_helper on Mac, since its only purpose was
to set the system color space to the now-specified color space.
Remove copyright exceptions for layout_test_helper as well.

BUG=44872
==========

[ccameron, 2016-08-09 06:13:24]

ccameron@chromium.org changed reviewers:
+ dpranke@chromium.org, phajdan.jr@chromium.org

[ccameron, 2016-08-09 07:11:08]

ptal

https://codereview.chromium.org/2226733003/diff/60001/content/shell/renderer/...
File content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
(right):

https://codereview.chromium.org/2226733003/diff/60001/content/shell/renderer/...
content/shell/renderer/layout_test/layout_test_content_renderer_client.cc:208:
LayoutTestContentRendererClient::GetImageDecodeColorProfile() {
An alternative TODO here would be to change all platforms to use a single color
profile.

That will require a rebaseline of a bazillion tests, but may be the right thing
to do.

[commit-bot: I haz the power, 2016-08-09 07:31:43]

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

[commit-bot: I haz the power, 2016-08-09 07:31:44]

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

[hubbe, 2016-08-09 16:57:40]

Wouldn't it be better to use the same profile on all platforms rather than
"match existing behavior", since existing behavior is essentially random?

[hubbe, 2016-08-09 16:59:53]

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

[hubbe, 2016-08-09 16:59:54]

https://codereview.chromium.org/2226733003/diff/60001/content/shell/renderer/...
File content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
(right):

https://codereview.chromium.org/2226733003/diff/60001/content/shell/renderer/...
content/shell/renderer/layout_test/layout_test_content_renderer_client.cc:208:
LayoutTestContentRendererClient::GetImageDecodeColorProfile() {
On 2016/08/09 07:11:08, ccameron wrote:
> An alternative TODO here would be to change all platforms to use a single
color
> profile.
> 
> That will require a rebaseline of a bazillion tests, but may be the right
thing
> to do.

Why is it that the code behaves differently when you have no icc profile or when
 you have an sRGB profile anyways?

[ccameron, 2016-08-09 17:06:45]

On 2016/08/09 16:59:54, hubbe wrote:
>
https://codereview.chromium.org/2226733003/diff/60001/content/shell/renderer/...
> File content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
> (right):
> 
>
https://codereview.chromium.org/2226733003/diff/60001/content/shell/renderer/...
> content/shell/renderer/layout_test/layout_test_content_renderer_client.cc:208:
> LayoutTestContentRendererClient::GetImageDecodeColorProfile() {
> On 2016/08/09 07:11:08, ccameron wrote:
> > An alternative TODO here would be to change all platforms to use a single
> color
> > profile.
> > 
> > That will require a rebaseline of a bazillion tests, but may be the right
> thing
> > to do.
> 
> Why is it that the code behaves differently when you have no icc profile or
when
>  you have an sRGB profile anyways?

In Mac-land, "Generic RGB" != sRGB.

WRT sRGB-versus-none, that gets at one of the more horrible things that is going
on, namely that it is expected that only the first call to set the output color
space will have an effect, see:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/image...

So far I've just preserved that behavior. Since we're leaning on Skia to do all
conversion in the future, and QCMS is being ripped out of the decoder, I'm
content to ignore that issue until it becomes irrelevant.

[Dirk Pranke, 2016-08-09 20:32:31]

lgtm

https://codereview.chromium.org/2226733003/diff/60001/content/shell/renderer/...
File content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
(right):

https://codereview.chromium.org/2226733003/diff/60001/content/shell/renderer/...
content/shell/renderer/layout_test/layout_test_content_renderer_client.cc:208:
LayoutTestContentRendererClient::GetImageDecodeColorProfile() {
On 2016/08/09 07:11:08, ccameron wrote:
> An alternative TODO here would be to change all platforms to use a single
color
> profile.
> 
> That will require a rebaseline of a bazillion tests, but may be the right
thing
> to do.

Yup, I'd say that's the right thing to do, so maybe update the TODO accordingly?

[ccameron, 2016-08-11 17:00:09]

ping phadjan.jr

[Paweł Hajdan Jr., 2016-08-17 13:49:12]

LGTM (make sure to specify what I'm supposed to review)

[hubbe, 2016-08-17 19:00:07]

lgtm

[ccameron, 2016-09-01 21:30:53]

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

[commit-bot: I haz the power, 2016-09-01 21:32:10]

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

[ccameron, 2016-09-01 22:15:10]

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

[ccameron, 2016-09-01 22:15:10]

Adding sievers@ for content/ stamp

[commit-bot: I haz the power, 2016-09-01 22:16:48]

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

[commit-bot: I haz the power, 2016-09-01 22:16:49]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ccameron, 2016-09-01 22:46:43]

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

[commit-bot: I haz the power, 2016-09-01 22:47:21]

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

[no sievers, 2016-09-01 23:01:52]

On 2016/09/01 22:15:10, ccameron (slow til Sept 1) wrote:
> Adding sievers@ for content/ stamp

lgtm

[ccameron, 2016-09-01 23:48:36]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2016-09-01 23:48:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, hubbe@chromium.org, phajdan.jr@chromium.org,
sievers@chromium.org
Link to the patchset: https://codereview.chromium.org/2226733003/#ps120001
(title: "Update TODOs")

[commit-bot: I haz the power, 2016-09-01 23:49:10]

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-02 02:37:48]

Description was changed from

==========
Color: Clean up color profile handling for layout tests

Allow layout tests to manually specify an output ICC profile
through the ContentRendererClient.

Match existing behavior on all platforms. This means:
- on Mac, using a generic RGB profile
- on Linux, not specifying a profile
- on Windows, leave this as TODO, since correct behavior is unclear

Remove layout_test_helper on Mac, since its only purpose was
to set the system color space to the now-specified color space.
Remove copyright exceptions for layout_test_helper as well.

BUG=44872
==========

to

==========
Color: Clean up color profile handling for layout tests

Allow layout tests to manually specify an output ICC profile
through the ContentRendererClient.

Match existing behavior on all platforms. This means:
- on Mac, using a generic RGB profile
- on Linux, not specifying a profile
- on Windows, leave this as TODO, since correct behavior is unclear

Remove layout_test_helper on Mac, since its only purpose was
to set the system color space to the now-specified color space.
Remove copyright exceptions for layout_test_helper as well.

BUG=44872
==========

[commit-bot: I haz the power, 2016-09-02 02:37:49]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-09-02 02:39:19]

Description was changed from

==========
Color: Clean up color profile handling for layout tests

Allow layout tests to manually specify an output ICC profile
through the ContentRendererClient.

Match existing behavior on all platforms. This means:
- on Mac, using a generic RGB profile
- on Linux, not specifying a profile
- on Windows, leave this as TODO, since correct behavior is unclear

Remove layout_test_helper on Mac, since its only purpose was
to set the system color space to the now-specified color space.
Remove copyright exceptions for layout_test_helper as well.

BUG=44872
==========

to

==========
Color: Clean up color profile handling for layout tests

Allow layout tests to manually specify an output ICC profile
through the ContentRendererClient.

Match existing behavior on all platforms. This means:
- on Mac, using a generic RGB profile
- on Linux, not specifying a profile
- on Windows, leave this as TODO, since correct behavior is unclear

Remove layout_test_helper on Mac, since its only purpose was
to set the system color space to the now-specified color space.
Remove copyright exceptions for layout_test_helper as well.

BUG=44872

Committed: https://crrev.com/41495f2551c63bea769afd6cc70f1c3593f3537f
Cr-Commit-Position: refs/heads/master@{#416170}
==========

[commit-bot: I haz the power, 2016-09-02 02:39:21]

Patchset 7 (id:??) landed as
https://crrev.com/41495f2551c63bea769afd6cc70f1c3593f3537f
Cr-Commit-Position: refs/heads/master@{#416170}