color: Tests for fix accidental partial color conversion

color: Tests for fix accidental partial color conversion

Add tests for this issue.

BUG=694191
Review-Url: https://codereview.chromium.org/2705203003
Cr-Commit-Position: refs/heads/master@{#452236}
Committed: https://chromium.googlesource.com/chromium/src/+/86a6cab9976c0355b241d85c30655c2ae51d0005

[ccameron, 2017-02-21 21:49:37]

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

[commit-bot: I haz the power, 2017-02-21 21:50:55]

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

[ccameron, 2017-02-21 21:52:39]

Description was changed from

==========
color: Fix accidental partial color conversion

BUG=694191694191694191694191
==========

to

==========
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.

Add tests for this. The ICC profile that I got for testing this behavior
is somewhat gigantic (~3.1 MB). This only gets built for tests ... but
still may be larger-than-reasonable.

BUG=694191
==========

[ccameron, 2017-02-21 21:52:39]

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

[ccameron, 2017-02-21 21:54:44]

ptal

The test file that is added (it only sometimes shows up in cr) is 3.1MB (50k
lines long).

It will certainly be useful in many contexts (especially in WebKit layout tests)
... and it is only built in tests (unless I screwed something up) ... but it is
on the ginormous side.

[ccameron, 2017-02-21 22:04:25]

Description was changed from

==========
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.

Add tests for this. The ICC profile that I got for testing this behavior
is somewhat gigantic (~3.1 MB). This only gets built for tests ... but
still may be larger-than-reasonable.

BUG=694191
==========

to

==========
color: Tests for fix accidental partial color conversion

Add tests for this issue. The ICC profile that I got for testing this
behavior is somewhat gigantic (~3.1 MB). This only gets built for tests
... but still may be larger-than-reasonable.

BUG=694191
==========

[ccameron, 2017-02-21 22:07:01]

On 2017/02/21 21:54:44, ccameron wrote:
> ptal
> 
> The test file that is added (it only sometimes shows up in cr) is 3.1MB (50k
> lines long).
> 
> It will certainly be useful in many contexts (especially in WebKit layout
tests)
> ... and it is only built in tests (unless I screwed something up) ... but it
is
> on the ginormous side.

Actually, the file is being skipped by "git cl upload" b/c it is too large.

Let's just stick with https://codereview.chromium.org/2712533002/ for now.

[ccameron, 2017-02-22 20:54:31]

Description was changed from

==========
color: Tests for fix accidental partial color conversion

Add tests for this issue. The ICC profile that I got for testing this
behavior is somewhat gigantic (~3.1 MB). This only gets built for tests
... but still may be larger-than-reasonable.

BUG=694191
==========

to

==========
color: Tests for fix accidental partial color conversion

Add tests for this issue.

BUG=694191
==========

[ccameron, 2017-02-22 20:56:06]

ptal

Thanks for the smaller ICC profile -- that works!

[msarett1, 2017-02-22 20:58:46]

lgtm

https://codereview.chromium.org/2705203003/diff/50001/ui/gfx/icc_profile.cc
File ui/gfx/icc_profile.cc (right):

https://codereview.chromium.org/2705203003/diff/50001/ui/gfx/icc_profile.cc#n...
ui/gfx/icc_profile.cc:37: uint64_t next_unused_id = 10;
I don't really understand what's going on here.

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

Thanks!

https://codereview.chromium.org/2705203003/diff/50001/ui/gfx/icc_profile.cc
File ui/gfx/icc_profile.cc (right):

https://codereview.chromium.org/2705203003/diff/50001/ui/gfx/icc_profile.cc#n...
ui/gfx/icc_profile.cc:37: uint64_t next_unused_id = 10;
On 2017/02/22 20:58:46, msarett1 wrote:
> I don't really understand what's going on here.

This is sort of weird. It's ensuring that we don't use any IDs less than 10,
because they'll conflict with the hard-coded IDs.

It's to paper over the fact that, in Chrome, the IDs are always created in the
browser process, and so are always unique, but during layout tests, some of the
IDs are created in the renderer process (for testing), and we need to make sure
that they don't conflict with the IDs coming from the browser process.

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

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2017-02-22 21:04:35]

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 22:28:14]

CQ is committing da patch.

Bot data: {"patchset_id": 50001, "attempt_start_ts": 1487797414449460,
"parent_rev": "6b35359be7e542dec28d4ae27ea83f861f6cc34a", "commit_rev":
"86a6cab9976c0355b241d85c30655c2ae51d0005"}

[commit-bot: I haz the power, 2017-02-22 22:28:51]

Description was changed from

==========
color: Tests for fix accidental partial color conversion

Add tests for this issue.

BUG=694191
==========

to

==========
color: Tests for fix accidental partial color conversion

Add tests for this issue.

BUG=694191

Review-Url: https://codereview.chromium.org/2705203003
Cr-Commit-Position: refs/heads/master@{#452236}
Committed:
https://chromium.googlesource.com/chromium/src/+/86a6cab9976c0355b241d85c3065...
==========

[commit-bot: I haz the power, 2017-02-22 22:28:52]

Committed patchset #5 (id:50001) as
https://chromium.googlesource.com/chromium/src/+/86a6cab9976c0355b241d85c3065...