ui/gfx/color_profile: Add GetDisplayColorProfile for an NSWindow

ui/gfx/color_profile: Add GetDisplayColorProfile for an NSWindow

Add a GetDisplayColorProfile variant on Mac to return the display
color profile associated with a native window (NSWindow).

Add unit tests for on-screen, partially-on-screen, off-screen and
empty and null windows.

Note an sRGB color profile is a common case, especially on win32,
so GetDisplayColorProfile returns true and an empty color profile
in that case. Add a header file comment about that.

TEST=gfx_unittests --gtest_filter="ColorProfileTest*Window"
BUG=368694
Committed: https://crrev.com/fc43a71f7e178e7a1bd6c97b3841984949c3e381
Cr-Commit-Position: refs/heads/master@{#313601}

[Noel Gordon, 2015-01-17 14:05:48]

Patchset #1 (id:1) has been deleted

[Noel Gordon, 2015-01-19 13:29:44]

noel@chromium.org changed reviewers:
+ rsesek@chromium.org

[Robert Sesek, 2015-01-20 17:03:39]

Is this just a convenience function? Can't this also be accomplished using
[NSWindow frame] and the existing GetDisplayColorProfile() ?

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

https://codereview.chromium.org/849523003/diff/20001/ui/gfx/color_profile.cc#...
ui/gfx/color_profile.cc:9: #if defined(OS_WIN) || defined(OS_MACOSX)
What are these redeclarations for? They're already in the header file.

[Noel Gordon, 2015-01-22 16:26:57]

> Is this just a convenience function? Can't this also be
> accomplished using [NSWindow frame] and the existing
> GetDisplayColorProfile() ?

Nope, can't be accomplished using [NSWindow frame] and the exiting routine sorry
to say [1] -- unless you'd prefer the wrong color profile being returned. My
preference is the right one.

[1] https://code.google.com/p/chromium/issues/detail?id=368694#c39

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

https://codereview.chromium.org/849523003/diff/20001/ui/gfx/color_profile.cc#...
ui/gfx/color_profile.cc:9: #if defined(OS_WIN) || defined(OS_MACOSX)
On 2015/01/20 17:03:39, Robert Sesek wrote:
> What are these redeclarations for? They're already in the header file.

Mainly for doumentation it seems.

[Robert Sesek, 2015-01-23 14:30:47]

On 2015/01/22 16:26:57, noel gordon wrote:
> > Is this just a convenience function? Can't this also be
> > accomplished using [NSWindow frame] and the existing
> > GetDisplayColorProfile() ?
> 
> Nope, can't be accomplished using [NSWindow frame] and the exiting routine
sorry
> to say [1] -- unless you'd prefer the wrong color profile being returned. My
> preference is the right one.
> 
> [1] https://code.google.com/p/chromium/issues/detail?id=368694#c39
> 

Ah, that's unfortunate. By any chance did you also test on 10.10, which I
believe made a change to how partially-on-screen windows are handled?

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

https://codereview.chromium.org/849523003/diff/20001/ui/gfx/color_profile.cc#...
ui/gfx/color_profile.cc:9: #if defined(OS_WIN) || defined(OS_MACOSX)
On 2015/01/22 16:26:57, noel gordon wrote:
> On 2015/01/20 17:03:39, Robert Sesek wrote:
> > What are these redeclarations for? They're already in the header file.
> 
> Mainly for doumentation it seems.

I don't follow. I.e., I think these should be removed since the header serves
this purpose.

[Noel Gordon, 2015-01-24 01:11:08]

On 2015/01/23 14:30:47, Robert Sesek wrote:
> On 2015/01/22 16:26:57, noel gordon wrote:
> > > Is this just a convenience function? Can't this also be
> > > accomplished using [NSWindow frame] and the existing
> > > GetDisplayColorProfile() ?
> > 
> > Nope, can't be accomplished using [NSWindow frame] and the exiting routine
> sorry
> > to say [1] -- unless you'd prefer the wrong color profile being returned. My
> > preference is the right one.
> > 
> > [1] https://code.google.com/p/chromium/issues/detail?id=368694#c39
> > 
> 
> Ah, that's unfortunate.

Yes, unfortunate.  Also think about in terms of DPI changes for example.  gfx/ui
uses maximum overlap for DPI too in my understanding, so no suprise then if its
result disagrees with the OSX result.  Not fixing that here though.

> By any chance did you also test on 10.10, which I
> believe made a change to how partially-on-screen windows are handled?

Puppet has not given me 10.10 yet, sorry.  Once it does, I'll look at and
address any color profile then.  Any reference to how 10.10 might have changed
partial windows would be appreciated.  Think the tests added here would be run
on 10.10 bots though, we'll see. 
 
> https://codereview.chromium.org/849523003/diff/20001/ui/gfx/color_profile.cc
> File ui/gfx/color_profile.cc (right):
> 
>
https://codereview.chromium.org/849523003/diff/20001/ui/gfx/color_profile.cc#...
> ui/gfx/color_profile.cc:9: #if defined(OS_WIN) || defined(OS_MACOSX)
> On 2015/01/22 16:26:57, noel gordon wrote:
> > On 2015/01/20 17:03:39, Robert Sesek wrote:
> > > What are these redeclarations for? They're already in the header file.
> > 
> > Mainly for doumentation it seems.
> 
> I don't follow. I.e., I think these should be removed since the header serves
> this purpose.

I can address that in a follow-up.  We're at 7 days here already.

[Noel Gordon, 2015-01-24 14:27:59]

On 2015/01/24 01:11:08, noel gordon wrote:
> Think the tests added here would be run on 10.10 bots though, we'll see.

ahem ignore that :)  We don't have 10.10 bots yet best I can tell.

[Robert Sesek, 2015-01-26 16:48:06]

On 2015/01/24 01:11:08, noel gordon wrote:
> On 2015/01/23 14:30:47, Robert Sesek wrote:
> > On 2015/01/22 16:26:57, noel gordon wrote:
> > > > Is this just a convenience function? Can't this also be
> > > > accomplished using [NSWindow frame] and the existing
> > > > GetDisplayColorProfile() ?
> > > 
> > > Nope, can't be accomplished using [NSWindow frame] and the exiting routine
> > sorry
> > > to say [1] -- unless you'd prefer the wrong color profile being returned.
My
> > > preference is the right one.
> > > 
> > > [1] https://code.google.com/p/chromium/issues/detail?id=368694#c39
> > > 
> > 
> > Ah, that's unfortunate.
> 
> Yes, unfortunate.  Also think about in terms of DPI changes for example. 
gfx/ui
> uses maximum overlap for DPI too in my understanding, so no suprise then if
its
> result disagrees with the OSX result.  Not fixing that here though.
> 
> > By any chance did you also test on 10.10, which I
> > believe made a change to how partially-on-screen windows are handled?
> 
> Puppet has not given me 10.10 yet, sorry.  Once it does, I'll look at and
> address any color profile then.  Any reference to how 10.10 might have changed
> partial windows would be appreciated.  Think the tests added here would be run
> on 10.10 bots though, we'll see. 

I believe it was mentioned in one of the WWDC 2014 talks, but my notes don't say
which one. They made changes because of how fullscreen and multiple desktops now
work, IIRC.

> > https://codereview.chromium.org/849523003/diff/20001/ui/gfx/color_profile.cc
> > File ui/gfx/color_profile.cc (right):
> > 
> >
>
https://codereview.chromium.org/849523003/diff/20001/ui/gfx/color_profile.cc#...
> > ui/gfx/color_profile.cc:9: #if defined(OS_WIN) || defined(OS_MACOSX)
> > On 2015/01/22 16:26:57, noel gordon wrote:
> > > On 2015/01/20 17:03:39, Robert Sesek wrote:
> > > > What are these redeclarations for? They're already in the header file.
> > > 
> > > Mainly for doumentation it seems.
> > 
> > I don't follow. I.e., I think these should be removed since the header
serves
> > this purpose.
> 
> I can address that in a follow-up.  We're at 7 days here already.

Why add code that needs to be removed in an immediate follow-up? (FYI you sent
this message 4 days after review started, which is IMO acceptable given the
timezone difference. But now we're going to spend another round on this).

[Noel Gordon, 2015-01-27 06:01:43]

On 2015/01/26 16:48:06, Robert Sesek wrote:

> Why add code that needs to be removed in an immediate follow-up? (FYI you sent
> this message 4 days after review started, which is IMO acceptable given the
> timezone difference. But now we're going to spend another round on this).

Since we're going to need another round, right ripped them out.

[Noel Gordon, 2015-01-27 06:07:38]

On 2015/01/26 16:48:06, Robert Sesek wrote:
> > Noel wrote:
> > Puppet has not given me 10.10 yet, sorry.  Once it does, I'll look at and
> > address any color profile then.  Any reference to how 10.10 might have
changed
> > partial windows would be appreciated.  Think the tests added here would be
run
> > on 10.10 bots though, we'll see. 
> 
> I believe it was mentioned in one of the WWDC 2014 talks, but my notes don't
say
> which one. They made changes because of how fullscreen and multiple desktops
now
> work, IIRC.

http://arstechnica.com/apple/2013/10/os-x-10-9/11/#multiple-displays

[Robert Sesek, 2015-01-27 20:38:30]

LGTM

On 2015/01/27 06:07:38, noel gordon wrote:
> On 2015/01/26 16:48:06, Robert Sesek wrote:
> > > Noel wrote:
> > > Puppet has not given me 10.10 yet, sorry.  Once it does, I'll look at and
> > > address any color profile then.  Any reference to how 10.10 might have
> changed
> > > partial windows would be appreciated.  Think the tests added here would be
> run
> > > on 10.10 bots though, we'll see. 
> > 
> > I believe it was mentioned in one of the WWDC 2014 talks, but my notes don't
> say
> > which one. They made changes because of how fullscreen and multiple desktops
> now
> > work, IIRC.
> 
> http://arstechnica.com/apple/2013/10/os-x-10-9/11/#multiple-displays

Ah it's in my 2013 WWDC notes, so you're right, 10.9. From the video "What's New
in Cocoa", windows can not span screens anymore.

[Noel Gordon, 2015-01-27 23:53:13]

On 2015/01/27 20:38:30, Robert Sesek wrote:

> > http://arstechnica.com/apple/2013/10/os-x-10-9/11/#multiple-displays
> 
> Ah it's in my 2013 WWDC notes, so you're right, 10.9. From the video "What's
New
> in Cocoa", windows can not span screens anymore.

Yeah. If you disable full screen apps, windows can span screens. When full
screen apps are enabled, no window spanning.

[Noel Gordon, 2015-01-27 23:53:25]

The CQ bit was checked by noel@chromium.org

[commit-bot: I haz the power, 2015-01-27 23:54:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/849523003/40001

[commit-bot: I haz the power, 2015-01-28 00:02:24]

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

[commit-bot: I haz the power, 2015-01-28 00:02:26]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Noel Gordon, 2015-01-28 00:04:05]

+sky for OWNERS

[Noel Gordon, 2015-01-28 00:04:22]

noel@chromium.org changed reviewers:
+ sky@chromium.org

[sky, 2015-01-28 20:54:49]

sky@chromium.org changed reviewers:
+ thakis@chromium.org
- sky@chromium.org

[sky, 2015-01-28 20:54:49]

sky->thakis (he is an owner and knows mac)

[Nico, 2015-01-28 21:08:27]

lgtm (somewhat stampy, since rsesek reviewed already)

[Noel Gordon, 2015-01-28 22:49:09]

The CQ bit was checked by noel@chromium.org

[commit-bot: I haz the power, 2015-01-28 22:50:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/849523003/40001

[commit-bot: I haz the power, 2015-01-28 22:55:01]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-01-28 22:57:35]

Patchset 2 (id:??) landed as
https://crrev.com/fc43a71f7e178e7a1bd6c97b3841984949c3e381
Cr-Commit-Position: refs/heads/master@{#313601}