Clean up gfx::ColorProfile

Clean up gfx::ColorProfile

Remove unused methods for querying the color profile of a particular
display. Something similar to these will be added in the future.

Add an explicit "update the cache read from disk" method on Windows.

BUG=622133
Committed: https://crrev.com/3f0de31ea0e8651a39ffe77d214d28a565d9ca1c
Cr-Commit-Position: refs/heads/master@{#402058}

[ccameron, 2016-06-22 21:31:32]

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

[ccameron, 2016-06-22 21:31:33]

This is some preliminary cleanup towards finally landing this patch
https://codereview.chromium.org/1796293003/

This patch deletes functionality for getting the color profile of a particular
monitor. These functions have not been called in production for years (and in
fact had been deleted before).

We'll need similar functionality soon, but the interface for that functionality
is different on different platforms (different arguments, different thread
behaviors), so it will need to be re-written anyway.

[enne (OOO), 2016-06-22 21:44:24]

This all looks pretty reasonable to me.

What's the general plan with respect to this code, re: "something similar to
these will be added in the future"?

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

https://codereview.chromium.org/2092533002/diff/1/ui/gfx/color_profile.cc#new...
ui/gfx/color_profile.cc:29: bool ColorProfile::InvalidColorProfileLength(size_t
length) {
nit: I know this existed before, but maybe call this IsValidColorProfileLength
to be more clear about what it does?

https://codereview.chromium.org/2092533002/diff/1/ui/gfx/color_profile.h
File ui/gfx/color_profile.h (right):

https://codereview.chromium.org/2092533002/diff/1/ui/gfx/color_profile.h#newc...
ui/gfx/color_profile.h:38: static ColorProfile GetFromDeepestMonitor();
What does "deepest" in this context mean?

[ccameron, 2016-06-22 22:33:31]

> What's the general plan with respect to this code, re: "something similar to
> these will be added in the future"?

WRT multi-monitor, I'm going to want to integrate this into display::Display, so
that you can get this information for a monitor in the same place that you get
everything else from a monitor. From there I'll be plumbing this through
ui::Compositor into GLRenderer, to get appropriate color correction in a shader.

The rough plans are at
https://docs.google.com/document/d/18Nt3XJFtKEMTRExyb_R_y_K4U-3tPIS3GksOQ9S2-...
which we're still revising/iterating on.

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

https://codereview.chromium.org/2092533002/diff/1/ui/gfx/color_profile.cc#new...
ui/gfx/color_profile.cc:29: bool ColorProfile::InvalidColorProfileLength(size_t
length) {
On 2016/06/22 21:44:24, enne wrote:
> nit: I know this existed before, but maybe call this IsValidColorProfileLength
> to be more clear about what it does?

Done.

https://codereview.chromium.org/2092533002/diff/1/ui/gfx/color_profile.h
File ui/gfx/color_profile.h (right):

https://codereview.chromium.org/2092533002/diff/1/ui/gfx/color_profile.h#newc...
ui/gfx/color_profile.h:38: static ColorProfile GetFromDeepestMonitor();
On 2016/06/22 21:44:24, enne wrote:
> What does "deepest" in this context mean?

The name came from the NSDisplay API (I think deepest means widest range).
Changed to GetFromBestMonitor().

[enne (OOO), 2016-06-22 22:39:35]

lgtm

On 2016/06/22 at 22:33:31, ccameron wrote:
> > What's the general plan with respect to this code, re: "something similar to
> > these will be added in the future"?
> 
> WRT multi-monitor, I'm going to want to integrate this into display::Display,
so that you can get this information for a monitor in the same place that you
get everything else from a monitor. From there I'll be plumbing this through
ui::Compositor into GLRenderer, to get appropriate color correction in a shader.

Ok, thanks.  Asking displays for their color profile makes more sense than the
previous code that was using bounds.  Just wasn't sure where that particular bit
was going to go.

Also, maybe you mean plumbing through cc::Display to GLRenderer? ui::Compositor
doesn't have a DirectRenderer because surfaces.

[ccameron, 2016-06-22 23:44:10]

On 2016/06/22 22:39:35, enne wrote:
> lgtm
> 
> On 2016/06/22 at 22:33:31, ccameron wrote:
> > > What's the general plan with respect to this code, re: "something similar
to
> > > these will be added in the future"?
> > 
> > WRT multi-monitor, I'm going to want to integrate this into
display::Display,
> so that you can get this information for a monitor in the same place that you
> get everything else from a monitor. From there I'll be plumbing this through
> ui::Compositor into GLRenderer, to get appropriate color correction in a
shader.
> 
> Ok, thanks.  Asking displays for their color profile makes more sense than the
> previous code that was using bounds.  Just wasn't sure where that particular
bit
> was going to go.
> 
> Also, maybe you mean plumbing through cc::Display to GLRenderer?
ui::Compositor
> doesn't have a DirectRenderer because surfaces.

Mmh, you're right ... I need to go look at how things connect to things more
carefully. But yes, to cc::Display to GLRenderer.

[no sievers, 2016-06-23 17:03:22]

lgtm

https://codereview.chromium.org/2092533002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/render_message_filter.cc (right):

https://codereview.chromium.org/2092533002/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_message_filter.cc:387: // TODO(ccameron):
Remove this synchronous IPC and push the color profile
can you put a bug number?

[ccameron, 2016-06-25 07:20:44]

Thanks!

https://codereview.chromium.org/2092533002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/render_message_filter.cc (right):

https://codereview.chromium.org/2092533002/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_message_filter.cc:387: // TODO(ccameron):
Remove this synchronous IPC and push the color profile
On 2016/06/23 17:03:22, sievers wrote:
> can you put a bug number?

Done.

[ccameron, 2016-06-25 07:20:49]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2016-06-25 07:20:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
sievers@chromium.org, enne@chromium.org
Link to the patchset: https://codereview.chromium.org/2092533002/#ps60001
(title: "git cl try")

[commit-bot: I haz the power, 2016-06-25 07:20:57]

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-06-25 08:11:16]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-25 08:13:07]

Description was changed from

==========
Clean up gfx::ColorProfile

Remove unused methods for querying the color profile of a particular
display. Something similar to these will be added in the future.

Add an explicit "update the cache read from disk" method on Windows.

BUG=622133
==========

to

==========
Clean up gfx::ColorProfile

Remove unused methods for querying the color profile of a particular
display. Something similar to these will be added in the future.

Add an explicit "update the cache read from disk" method on Windows.

BUG=622133

Committed: https://crrev.com/3f0de31ea0e8651a39ffe77d214d28a565d9ca1c
Cr-Commit-Position: refs/heads/master@{#402058}
==========

[commit-bot: I haz the power, 2016-06-25 08:13:08]

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