Image decode color: Push color profile from browser to renderer

Image decode color: Push color profile from browser to renderer

We decode images into the output device's color space.

This color space is computed in blink::Platform::screenColorProfile,
which is implemented as RendererBlinkPlatformImpl::screenColorProfile
in the renderer process. This function will either
- On Windows, issue a synchronous IPC to the browser process to query
  the color profile. This synchronous IPC is picked up in
  RenderMessageFilter::OnGetMonitorColorProfile, where it will read
  the color profile information from disk the first time it is called,
  and then always return that cached value.
- On Mac, use the CG monitor/color APIs to query the color profile from
  within the renderer process.
After this value is read the first time, it is cached in the renderer
process in a static local variable in blink::ImageDecoder::
qcmsOutputDeviceProfile.

This patch will has the browser push a color profile to the renderer
in ViewMsg_New, which is then pushed to the image decode code via the
new method ImageDecoder::setOutputDeviceColorProfile.

This patch also starts to clean up the gfx::ColorProfile API on Windows
by creating a separate function to update the cached profiles (which
will be reading from disk), and having the functions to read the color
profile only query the cache. There is much more clean-up and that will
be done here in future patches.

The up-sides to this patch are
- Removes a synchronous IPC to the browser on Windows
- Removes a use of CG monitor and color APIs from the renderer process
  (towards allowing tightening of the standbox).
- Makes the gfx::ColorProfile API slightly more uniform across all
  platforms (the query functions on Windows can now be called on the
  UI and IO thread, with the wrinkle that someone needs to do an
  update beforehand).

Some areas that still need cleanup include:
- The image decode color profile in Blink is still in a static local
  variable, and cannot be updated after being initially set.
- The image decode color profile is still not multi-monitor aware, and
  selects an arbitrary monitor's color profile for image decode.
- The process of calling UpdateDisplayColorProfileCache could be cleaned
  up.
  - This should be no more expensive than before in that we are still doing
    about one BrowserThread::PostBlockingPoolTask per renderer -- we're
    just doing it ahead of time now instead of in response to the IPC.
  - There is much to be done in cleaning up gfx::ColorProfile and
    meshing it with gfx::Display. This patch (pushing the color profile
    to the renderer) is a prerequisite to that cleanup.

Committed: https://crrev.com/6fc2fd3b6c99c97632b7fe43bdbf1fa12ad352ca
Cr-Commit-Position: refs/heads/master@{#402501}

[ccameron, 2016-03-15 05:43:33]

Patchset #3 (id:40001) has been deleted

[ccameron, 2016-03-15 05:43:39]

Patchset #2 (id:20001) has been deleted

[ccameron, 2016-03-15 05:43:45]

Patchset #1 (id:1) has been deleted

[ccameron, 2016-03-15 05:44:24]

Description was changed from

==========
Color end to end for testing
==========

to

==========
Image decode color: Push color profile from browser to renderer

We decode images into the output device's color space.

This color space is computed in blink::Platform::screenColorProfile,
which is implemented as RendererBlinkPlatformImpl::screenColorProfile
in the renderer process. This function will either
- On Windows, issue a synchronous IPC to the browser process to query
  the color profile. This synchronous IPC is picked up in
  RenderMessageFilter::OnGetMonitorColorProfile, where it will read
  the color profile information from disk the first time it is called,
  and then always return that cached value.
- On Mac, use the NSScreen API to query the color profile from within
  the renderer process.
After this value is read the first time, it is cached in the renderer
process in a static local variable in blink::ImageDecoder::
qcmsOutputDeviceProfile.

This patch will has the browser push a color profile to the renderer
in ViewMsg_New, which is then pushed to the image decode code via the
new method ImageDecoder::setOutputDeviceColorProfile.

This patch also starts to clean up the gfx::ColorProfile API on Windows
by creating a separate function to update the cached profiles (which
will be reading from disk), and having the functions to read the color
profile only query the cache. There is much more clean-up and that will
be done here in future patches.

The up-sides to this patch are
- Removes a synchronous IPC to the browser on Windows
- Removes a use of NSScreen APIs from the renderer process (towards
  allowing tightening of the standbox).
- Makes the gfx::ColorProfile API slightly more uniform across all
  platforms (the query functions on Windows can now be called on the
  UI and IO thread, with the wrinkle that someone needs to do an
  update beforehand).

Some areas that still need cleanup include:
- The image decode color profile in Blink is still in a static local
  variable, and cannot be updated after being initially set.
- The image decode color profile is still not multi-monitor aware, and
  selects an arbitrary monitor's color profile for image decode.
- The process of calling UpdateDisplayColorProfileCache could be cleaned
  up.
  - This should be no worse than before in that we are still doing about
    one BrowserThread::PostBlockingPoolTask per renderer -- we're just
    doing it ahead of time now instead of in response to the IPC.
  - There is much to be done in cleaning up gfx::ColorProfile and 
    meshing it with gfx::Display. This patch (pushing the color profile
    to the renderer) is a prerequisite to that cleanup.
==========

[ccameron, 2016-03-15 06:02:24]

https://codereview.chromium.org/1796293003/diff/60001/content/browser/rendere...
File content/browser/renderer_host/render_view_host_impl.cc (right):

https://codereview.chromium.org/1796293003/diff/60001/content/browser/rendere...
content/browser/renderer_host/render_view_host_impl.cc:342:
params.image_decode_color_profile = gfx::ColorProfile().profile();
Note that this default constructor actually computes the primary monitor's color
profile.

This matches previous behavior in RendererBlinkPlatformImpl::screenColorProfile
and RenderMessageFilter::OnGetMonitorColorProfile -- I'll change it to a
function and give it a more clear name in a future patch.

https://codereview.chromium.org/1796293003/diff/60001/content/renderer/render...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/1796293003/diff/60001/content/renderer/render...
content/renderer/render_view_impl.cc:742:
webview()->setDeviceColorProfile(params.image_decode_color_profile);
We should not consider this to be the permanent home of the
image_decode_color_profile.

I argue that this is appropriate *for now* because it reflects that the profile
is just once at initialization (unlike other IPCs like ViewMsg_Resize or
ViewMsg_ColorProfile, which can be sent multiple times).

When image decode color profile is dynamic in the renderer, ViewMsg_Resize or
ViewMsg_ColorProfile will be a better match for this functionality. But not now.

Of note is that because this is setting a static variable via initialization,
only the first call in this process will "count". While I wouldn't argue that to
be an ideal arrangement, I think it's the best we can do for the moment.

https://codereview.chromium.org/1796293003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/image-decoders/ImageDecoder.h (right):

https://codereview.chromium.org/1796293003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/image-decoders/ImageDecoder.h:255: 
Note that the optional colorProfile argument is used to initialize the profile
used for image decode.

This is not an intuitive pattern, but should only remain until we support
dynamic decode targets.

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

https://codereview.chromium.org/1796293003/diff/60001/ui/gfx/color_profile.h#...
ui/gfx/color_profile.h:25: // Creates a color profile for the system's primary
monitor.
This should be a function called GetPrimaryDisplayColorProfile(), not a default
constructor.

I will fix that in the upcoming refactor (for now, I'm leaving it as-is, modulo
allowing IO/UI thread access because Windows is now reading cached data).

https://codereview.chromium.org/1796293003/diff/60001/ui/gfx/color_profile.h#...
ui/gfx/color_profile.h:56: #if defined(OS_MACOSX)
Note that there is no reason why Mac should have a separate API here -- when we
merge this with gfx::Screen and gfx::Display, this difference can go away.

[ccameron, 2016-03-15 06:37:44]

Description was changed from

==========
Image decode color: Push color profile from browser to renderer

We decode images into the output device's color space.

This color space is computed in blink::Platform::screenColorProfile,
which is implemented as RendererBlinkPlatformImpl::screenColorProfile
in the renderer process. This function will either
- On Windows, issue a synchronous IPC to the browser process to query
  the color profile. This synchronous IPC is picked up in
  RenderMessageFilter::OnGetMonitorColorProfile, where it will read
  the color profile information from disk the first time it is called,
  and then always return that cached value.
- On Mac, use the NSScreen API to query the color profile from within
  the renderer process.
After this value is read the first time, it is cached in the renderer
process in a static local variable in blink::ImageDecoder::
qcmsOutputDeviceProfile.

This patch will has the browser push a color profile to the renderer
in ViewMsg_New, which is then pushed to the image decode code via the
new method ImageDecoder::setOutputDeviceColorProfile.

This patch also starts to clean up the gfx::ColorProfile API on Windows
by creating a separate function to update the cached profiles (which
will be reading from disk), and having the functions to read the color
profile only query the cache. There is much more clean-up and that will
be done here in future patches.

The up-sides to this patch are
- Removes a synchronous IPC to the browser on Windows
- Removes a use of NSScreen APIs from the renderer process (towards
  allowing tightening of the standbox).
- Makes the gfx::ColorProfile API slightly more uniform across all
  platforms (the query functions on Windows can now be called on the
  UI and IO thread, with the wrinkle that someone needs to do an
  update beforehand).

Some areas that still need cleanup include:
- The image decode color profile in Blink is still in a static local
  variable, and cannot be updated after being initially set.
- The image decode color profile is still not multi-monitor aware, and
  selects an arbitrary monitor's color profile for image decode.
- The process of calling UpdateDisplayColorProfileCache could be cleaned
  up.
  - This should be no worse than before in that we are still doing about
    one BrowserThread::PostBlockingPoolTask per renderer -- we're just
    doing it ahead of time now instead of in response to the IPC.
  - There is much to be done in cleaning up gfx::ColorProfile and 
    meshing it with gfx::Display. This patch (pushing the color profile
    to the renderer) is a prerequisite to that cleanup.
==========

to

==========
Image decode color: Push color profile from browser to renderer

We decode images into the output device's color space.

This color space is computed in blink::Platform::screenColorProfile,
which is implemented as RendererBlinkPlatformImpl::screenColorProfile
in the renderer process. This function will either
- On Windows, issue a synchronous IPC to the browser process to query
  the color profile. This synchronous IPC is picked up in
  RenderMessageFilter::OnGetMonitorColorProfile, where it will read
  the color profile information from disk the first time it is called,
  and then always return that cached value.
- On Mac, use the NSScreen API to query the color profile from within
  the renderer process.
After this value is read the first time, it is cached in the renderer
process in a static local variable in blink::ImageDecoder::
qcmsOutputDeviceProfile.

This patch will has the browser push a color profile to the renderer
in ViewMsg_New, which is then pushed to the image decode code via the
new method ImageDecoder::setOutputDeviceColorProfile.

This patch also starts to clean up the gfx::ColorProfile API on Windows
by creating a separate function to update the cached profiles (which
will be reading from disk), and having the functions to read the color
profile only query the cache. There is much more clean-up and that will
be done here in future patches.

The up-sides to this patch are
- Removes a synchronous IPC to the browser on Windows
- Removes a use of NSScreen APIs from the renderer process (towards
  allowing tightening of the standbox).
- Makes the gfx::ColorProfile API slightly more uniform across all
  platforms (the query functions on Windows can now be called on the
  UI and IO thread, with the wrinkle that someone needs to do an
  update beforehand).

Some areas that still need cleanup include:
- The image decode color profile in Blink is still in a static local
  variable, and cannot be updated after being initially set.
- The image decode color profile is still not multi-monitor aware, and
  selects an arbitrary monitor's color profile for image decode.
- The process of calling UpdateDisplayColorProfileCache could be cleaned
  up.
  - This should be no more expensive than before in that we are still doing
    about one BrowserThread::PostBlockingPoolTask per renderer -- we're
    just doing it ahead of time now instead of in response to the IPC.
  - There is much to be done in cleaning up gfx::ColorProfile and 
    meshing it with gfx::Display. This patch (pushing the color profile
    to the renderer) is a prerequisite to that cleanup.
==========

[Avi (use Gerrit), 2016-03-15 16:20:38]

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

[Avi (use Gerrit), 2016-03-15 16:20:38]

I am very much liking the direction you are going here. Thank you for the
detailed notes about what you see as the next few steps. LGTM

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

https://codereview.chromium.org/1796293003/diff/60001/ui/gfx/color_profile.h#...
ui/gfx/color_profile.h:56: #if defined(OS_MACOSX)
On 2016/03/15 06:02:24, ccameron wrote:
> Note that there is no reason why Mac should have a separate API here -- when
we
> merge this with gfx::Screen and gfx::Display, this difference can go away.

woot

https://codereview.chromium.org/1796293003/diff/60001/ui/gfx/color_profile_wi...
File ui/gfx/color_profile_win.cc (right):

https://codereview.chromium.org/1796293003/diff/60001/ui/gfx/color_profile_wi...
ui/gfx/color_profile_win.cc:8: #include <windows.h>
It's probably OK here, but in general leave the windows.h include to be first.
There are occasionally surprising results if you don't :(

(See src/tools/sort-headers.py and weep.)

[ccameron, 2016-03-15 17:02:37]

Description was changed from

==========
Image decode color: Push color profile from browser to renderer

We decode images into the output device's color space.

This color space is computed in blink::Platform::screenColorProfile,
which is implemented as RendererBlinkPlatformImpl::screenColorProfile
in the renderer process. This function will either
- On Windows, issue a synchronous IPC to the browser process to query
  the color profile. This synchronous IPC is picked up in
  RenderMessageFilter::OnGetMonitorColorProfile, where it will read
  the color profile information from disk the first time it is called,
  and then always return that cached value.
- On Mac, use the NSScreen API to query the color profile from within
  the renderer process.
After this value is read the first time, it is cached in the renderer
process in a static local variable in blink::ImageDecoder::
qcmsOutputDeviceProfile.

This patch will has the browser push a color profile to the renderer
in ViewMsg_New, which is then pushed to the image decode code via the
new method ImageDecoder::setOutputDeviceColorProfile.

This patch also starts to clean up the gfx::ColorProfile API on Windows
by creating a separate function to update the cached profiles (which
will be reading from disk), and having the functions to read the color
profile only query the cache. There is much more clean-up and that will
be done here in future patches.

The up-sides to this patch are
- Removes a synchronous IPC to the browser on Windows
- Removes a use of NSScreen APIs from the renderer process (towards
  allowing tightening of the standbox).
- Makes the gfx::ColorProfile API slightly more uniform across all
  platforms (the query functions on Windows can now be called on the
  UI and IO thread, with the wrinkle that someone needs to do an
  update beforehand).

Some areas that still need cleanup include:
- The image decode color profile in Blink is still in a static local
  variable, and cannot be updated after being initially set.
- The image decode color profile is still not multi-monitor aware, and
  selects an arbitrary monitor's color profile for image decode.
- The process of calling UpdateDisplayColorProfileCache could be cleaned
  up.
  - This should be no more expensive than before in that we are still doing
    about one BrowserThread::PostBlockingPoolTask per renderer -- we're
    just doing it ahead of time now instead of in response to the IPC.
  - There is much to be done in cleaning up gfx::ColorProfile and 
    meshing it with gfx::Display. This patch (pushing the color profile
    to the renderer) is a prerequisite to that cleanup.
==========

to

==========
Image decode color: Push color profile from browser to renderer

We decode images into the output device's color space.

This color space is computed in blink::Platform::screenColorProfile,
which is implemented as RendererBlinkPlatformImpl::screenColorProfile
in the renderer process. This function will either
- On Windows, issue a synchronous IPC to the browser process to query
  the color profile. This synchronous IPC is picked up in
  RenderMessageFilter::OnGetMonitorColorProfile, where it will read
  the color profile information from disk the first time it is called,
  and then always return that cached value.
- On Mac, use the CG monitor/color APIs to query the color profile from
  within the renderer process.
After this value is read the first time, it is cached in the renderer
process in a static local variable in blink::ImageDecoder::
qcmsOutputDeviceProfile.

This patch will has the browser push a color profile to the renderer
in ViewMsg_New, which is then pushed to the image decode code via the
new method ImageDecoder::setOutputDeviceColorProfile.

This patch also starts to clean up the gfx::ColorProfile API on Windows
by creating a separate function to update the cached profiles (which
will be reading from disk), and having the functions to read the color
profile only query the cache. There is much more clean-up and that will
be done here in future patches.

The up-sides to this patch are
- Removes a synchronous IPC to the browser on Windows
- Removes a use of CG monitor and color APIs from the renderer process
  (towards allowing tightening of the standbox).
- Makes the gfx::ColorProfile API slightly more uniform across all
  platforms (the query functions on Windows can now be called on the
  UI and IO thread, with the wrinkle that someone needs to do an
  update beforehand).

Some areas that still need cleanup include:
- The image decode color profile in Blink is still in a static local
  variable, and cannot be updated after being initially set.
- The image decode color profile is still not multi-monitor aware, and
  selects an arbitrary monitor's color profile for image decode.
- The process of calling UpdateDisplayColorProfileCache could be cleaned
  up.
  - This should be no more expensive than before in that we are still doing
    about one BrowserThread::PostBlockingPoolTask per renderer -- we're
    just doing it ahead of time now instead of in response to the IPC.
  - There is much to be done in cleaning up gfx::ColorProfile and 
    meshing it with gfx::Display. This patch (pushing the color profile
    to the renderer) is a prerequisite to that cleanup.
==========

[ccameron, 2016-03-15 17:10:08]

ccameron@chromium.org changed reviewers:
+ dcheng@chromium.org, noel@chromium.org, pdr@chromium.org

[ccameron, 2016-03-15 17:10:08]

dcheng, ptal at *_messages.h

This moves a parameter from a renderer-initiated sync IPC to a
browser-to-renderer IPC.

[dcheng, 2016-03-15 17:28:09]

https://codereview.chromium.org/1796293003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/image-decoders/ImageDecoder.h (right):

https://codereview.chromium.org/1796293003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/image-decoders/ImageDecoder.h:256: static
qcms_profile* qcmsOutputDeviceProfile(const ColorProfile* colorProfile = 0)
= nullptr

https://codereview.chromium.org/1796293003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1796293003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebViewImpl.cpp:3199: if (page())
Why not early return if page() is null? Doesn't that mean the WebView is going
away?

[ccameron, 2016-03-15 18:22:05]

Thanks -- updated.

https://codereview.chromium.org/1796293003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/image-decoders/ImageDecoder.h (right):

https://codereview.chromium.org/1796293003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/image-decoders/ImageDecoder.h:256: static
qcms_profile* qcmsOutputDeviceProfile(const ColorProfile* colorProfile = 0)
On 2016/03/15 17:28:08, dcheng wrote:
> = nullptr

Done.

https://codereview.chromium.org/1796293003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1796293003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebViewImpl.cpp:3199: if (page())
On 2016/03/15 17:28:08, dcheng wrote:
> Why not early return if page() is null? Doesn't that mean the WebView is going
> away?

Oh, if that's only-at-exit, then yes, this should be kept. Updated.

[dcheng, 2016-03-15 18:32:51]

lgtm

[pdr., 2016-03-16 03:45:18]

This patch points the code in a better direction. LGTM on the blinky bits (sorry
for the review delay).

[ccameron, 2016-03-16 05:45:51]

Thanks!

[ccameron, 2016-03-16 05:45:59]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2016-03-16 05:46:00]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org
Link to the patchset: https://codereview.chromium.org/1796293003/#ps120001
(title: "Incorporate review feedback")

[commit-bot: I haz the power, 2016-03-16 05:46:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1796293003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1796293003/120001

[Noel Gordon, 2016-03-16 06:07:38]

The CQ bit was unchecked by noel@chromium.org

[Noel Gordon, 2016-03-16 06:19:51]

Time-zones and all ...  Why are we removing the IPC pull model that shipped to
the field?  Is there some bug we are fixing?   No BUG= on the CL here so I can't
really tell.

In that pull model, only tabs with color profiled images request the profile,
right?  Now all tabs get profiles, why this change?  Also WebScreenInfo is an
unreliable sender of screen info (refer to http://crbug.com/357443), and we know
it.  Why are making shipping code dependent on it?

[ccameron, 2016-03-16 20:40:49]

Patchset #5 (id:140001) has been deleted

[ccameron, 2016-03-16 21:04:17]

Patchset #5 (id:160001) has been deleted

[ccameron, 2016-03-16 21:34:05]

Patchset #5 (id:180001) has been deleted

[ccameron, 2016-03-17 02:10:50]

On 2016/03/16 06:19:51, noel gordon wrote:
> Time-zones and all ...  Why are we removing the IPC pull model that shipped to
> the field?  Is there some bug we are fixing?   No BUG= on the CL here so I
can't
> really tell.

There are a handful of coherence performance and correctness issues with the
current IPC pull method.

The current IPC mechanism always queries the primary monitor's color space, and
it doesn't get any updates if the color profile changed or if the monitor is
changed. A browser push is a more effective at communicating this information
correctly.

It's also hard to keep a consistent view of color in the renderer because there
are two sources of color information (ViewMsg_ColorProfile and
RenderProcessHostMsg_GetMonitorColorProfile) which travel in different
directions are not kept in sync. One of these should go, and this patch is a
step in the direction of relying only on the ViewMsg_ColorProfile mechanism.

> In that pull model, only tabs with color profiled images request the profile,
> right?  Now all tabs get profiles, why this change?  Also WebScreenInfo is an
> unreliable sender of screen info (refer to http://crbug.com/357443), and we
know
> it.  Why are making shipping code dependent on it?

This patch does not involve WebScreenInfo.

[ccameron, 2016-05-03 18:19:08]

I plan to re-resolve and commit this soon.

[Noel Gordon, 2016-05-05 17:27:40]

With the pull model we ship to the field (the renderer IPC thing), do we have an
idea about how many web pages use it?

[ccameron, 2016-05-05 18:46:14]

On 2016/05/05 17:27:40, noel gordon wrote:
> With the pull model we ship to the field (the renderer IPC thing), do we have
an
> idea about how many web pages use it?

I don't think we have any stats on that, but, when we start doing color correct
rasterization, we will need 100% of the renderers to have the predicted target
color space communicated to them.

[Noel Gordon, 2016-06-01 05:41:51]

On 2016/05/05 18:46:14, ccameron wrote:
> On 2016/05/05 17:27:40, noel gordon wrote:
> > With the pull model we ship to the field (the renderer IPC thing), do we
have
> an
> > idea about how many web pages use it?
> 
> I don't think we have any stats on that, 

About 4% of web JPEG have a color profile. Translated to web pages, I estimate
about 2% of web pages use the pull IPC.

> but, when we start doing color correct
> rasterization, we will need 100% of the renderers to have the predicted target
> color space communicated to them.

Yes, I has assumed that work would be done behind the flag and we'd measure it
before shipping.

Anyhow, looks like all renderers (not behind the flag) in this patch on WIN
attempt to prime the profile cache, and that might show up in page load / perf
tests.

[Noel Gordon, 2016-06-01 06:08:25]

https://codereview.chromium.org/1796293003/diff/300001/ui/gfx/color_profile_w...
File ui/gfx/color_profile_win.cc (right):

https://codereview.chromium.org/1796293003/diff/300001/ui/gfx/color_profile_w...
ui/gfx/color_profile_win.cc:51: // pre-existing behavior is to read them, and
the fix for that will
Looking at code-diff left at line 91, I see

if (file_name != base::FilePath(L"sRGB Color Space Profile.icm")) {

so the pre-existing behavior was to skip reading them.  An empty profile should
be stored / returned in that case. The Blink-side renderer code will interpolate
the sRGB color profile as a result, this to save us sending them to renderers
since the sRGB profile is the system default on win.

I will be OOO for the next few months, so perhaps fix the above, and have you
other reviewers check it to move on to submit.

[ccameron, 2016-06-28 17:32:04]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2016-06-28 17:32:06]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org,
pdr@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/1796293003/#ps400001
(title: "leave empty check")

[commit-bot: I haz the power, 2016-06-28 17:32:58]

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-28 17:42:25]

Description was changed from

==========
Image decode color: Push color profile from browser to renderer

We decode images into the output device's color space.

This color space is computed in blink::Platform::screenColorProfile,
which is implemented as RendererBlinkPlatformImpl::screenColorProfile
in the renderer process. This function will either
- On Windows, issue a synchronous IPC to the browser process to query
  the color profile. This synchronous IPC is picked up in
  RenderMessageFilter::OnGetMonitorColorProfile, where it will read
  the color profile information from disk the first time it is called,
  and then always return that cached value.
- On Mac, use the CG monitor/color APIs to query the color profile from
  within the renderer process.
After this value is read the first time, it is cached in the renderer
process in a static local variable in blink::ImageDecoder::
qcmsOutputDeviceProfile.

This patch will has the browser push a color profile to the renderer
in ViewMsg_New, which is then pushed to the image decode code via the
new method ImageDecoder::setOutputDeviceColorProfile.

This patch also starts to clean up the gfx::ColorProfile API on Windows
by creating a separate function to update the cached profiles (which
will be reading from disk), and having the functions to read the color
profile only query the cache. There is much more clean-up and that will
be done here in future patches.

The up-sides to this patch are
- Removes a synchronous IPC to the browser on Windows
- Removes a use of CG monitor and color APIs from the renderer process
  (towards allowing tightening of the standbox).
- Makes the gfx::ColorProfile API slightly more uniform across all
  platforms (the query functions on Windows can now be called on the
  UI and IO thread, with the wrinkle that someone needs to do an
  update beforehand).

Some areas that still need cleanup include:
- The image decode color profile in Blink is still in a static local
  variable, and cannot be updated after being initially set.
- The image decode color profile is still not multi-monitor aware, and
  selects an arbitrary monitor's color profile for image decode.
- The process of calling UpdateDisplayColorProfileCache could be cleaned
  up.
  - This should be no more expensive than before in that we are still doing
    about one BrowserThread::PostBlockingPoolTask per renderer -- we're
    just doing it ahead of time now instead of in response to the IPC.
  - There is much to be done in cleaning up gfx::ColorProfile and 
    meshing it with gfx::Display. This patch (pushing the color profile
    to the renderer) is a prerequisite to that cleanup.
==========

to

==========
Image decode color: Push color profile from browser to renderer

We decode images into the output device's color space.

This color space is computed in blink::Platform::screenColorProfile,
which is implemented as RendererBlinkPlatformImpl::screenColorProfile
in the renderer process. This function will either
- On Windows, issue a synchronous IPC to the browser process to query
  the color profile. This synchronous IPC is picked up in
  RenderMessageFilter::OnGetMonitorColorProfile, where it will read
  the color profile information from disk the first time it is called,
  and then always return that cached value.
- On Mac, use the CG monitor/color APIs to query the color profile from
  within the renderer process.
After this value is read the first time, it is cached in the renderer
process in a static local variable in blink::ImageDecoder::
qcmsOutputDeviceProfile.

This patch will has the browser push a color profile to the renderer
in ViewMsg_New, which is then pushed to the image decode code via the
new method ImageDecoder::setOutputDeviceColorProfile.

This patch also starts to clean up the gfx::ColorProfile API on Windows
by creating a separate function to update the cached profiles (which
will be reading from disk), and having the functions to read the color
profile only query the cache. There is much more clean-up and that will
be done here in future patches.

The up-sides to this patch are
- Removes a synchronous IPC to the browser on Windows
- Removes a use of CG monitor and color APIs from the renderer process
  (towards allowing tightening of the standbox).
- Makes the gfx::ColorProfile API slightly more uniform across all
  platforms (the query functions on Windows can now be called on the
  UI and IO thread, with the wrinkle that someone needs to do an
  update beforehand).

Some areas that still need cleanup include:
- The image decode color profile in Blink is still in a static local
  variable, and cannot be updated after being initially set.
- The image decode color profile is still not multi-monitor aware, and
  selects an arbitrary monitor's color profile for image decode.
- The process of calling UpdateDisplayColorProfileCache could be cleaned
  up.
  - This should be no more expensive than before in that we are still doing
    about one BrowserThread::PostBlockingPoolTask per renderer -- we're
    just doing it ahead of time now instead of in response to the IPC.
  - There is much to be done in cleaning up gfx::ColorProfile and 
    meshing it with gfx::Display. This patch (pushing the color profile
    to the renderer) is a prerequisite to that cleanup.
==========

[commit-bot: I haz the power, 2016-06-28 17:42:28]

Committed patchset #15 (id:400001)

[commit-bot: I haz the power, 2016-06-28 17:45:33]

Description was changed from

==========
Image decode color: Push color profile from browser to renderer

We decode images into the output device's color space.

This color space is computed in blink::Platform::screenColorProfile,
which is implemented as RendererBlinkPlatformImpl::screenColorProfile
in the renderer process. This function will either
- On Windows, issue a synchronous IPC to the browser process to query
  the color profile. This synchronous IPC is picked up in
  RenderMessageFilter::OnGetMonitorColorProfile, where it will read
  the color profile information from disk the first time it is called,
  and then always return that cached value.
- On Mac, use the CG monitor/color APIs to query the color profile from
  within the renderer process.
After this value is read the first time, it is cached in the renderer
process in a static local variable in blink::ImageDecoder::
qcmsOutputDeviceProfile.

This patch will has the browser push a color profile to the renderer
in ViewMsg_New, which is then pushed to the image decode code via the
new method ImageDecoder::setOutputDeviceColorProfile.

This patch also starts to clean up the gfx::ColorProfile API on Windows
by creating a separate function to update the cached profiles (which
will be reading from disk), and having the functions to read the color
profile only query the cache. There is much more clean-up and that will
be done here in future patches.

The up-sides to this patch are
- Removes a synchronous IPC to the browser on Windows
- Removes a use of CG monitor and color APIs from the renderer process
  (towards allowing tightening of the standbox).
- Makes the gfx::ColorProfile API slightly more uniform across all
  platforms (the query functions on Windows can now be called on the
  UI and IO thread, with the wrinkle that someone needs to do an
  update beforehand).

Some areas that still need cleanup include:
- The image decode color profile in Blink is still in a static local
  variable, and cannot be updated after being initially set.
- The image decode color profile is still not multi-monitor aware, and
  selects an arbitrary monitor's color profile for image decode.
- The process of calling UpdateDisplayColorProfileCache could be cleaned
  up.
  - This should be no more expensive than before in that we are still doing
    about one BrowserThread::PostBlockingPoolTask per renderer -- we're
    just doing it ahead of time now instead of in response to the IPC.
  - There is much to be done in cleaning up gfx::ColorProfile and 
    meshing it with gfx::Display. This patch (pushing the color profile
    to the renderer) is a prerequisite to that cleanup.
==========

to

==========
Image decode color: Push color profile from browser to renderer

We decode images into the output device's color space.

This color space is computed in blink::Platform::screenColorProfile,
which is implemented as RendererBlinkPlatformImpl::screenColorProfile
in the renderer process. This function will either
- On Windows, issue a synchronous IPC to the browser process to query
  the color profile. This synchronous IPC is picked up in
  RenderMessageFilter::OnGetMonitorColorProfile, where it will read
  the color profile information from disk the first time it is called,
  and then always return that cached value.
- On Mac, use the CG monitor/color APIs to query the color profile from
  within the renderer process.
After this value is read the first time, it is cached in the renderer
process in a static local variable in blink::ImageDecoder::
qcmsOutputDeviceProfile.

This patch will has the browser push a color profile to the renderer
in ViewMsg_New, which is then pushed to the image decode code via the
new method ImageDecoder::setOutputDeviceColorProfile.

This patch also starts to clean up the gfx::ColorProfile API on Windows
by creating a separate function to update the cached profiles (which
will be reading from disk), and having the functions to read the color
profile only query the cache. There is much more clean-up and that will
be done here in future patches.

The up-sides to this patch are
- Removes a synchronous IPC to the browser on Windows
- Removes a use of CG monitor and color APIs from the renderer process
  (towards allowing tightening of the standbox).
- Makes the gfx::ColorProfile API slightly more uniform across all
  platforms (the query functions on Windows can now be called on the
  UI and IO thread, with the wrinkle that someone needs to do an
  update beforehand).

Some areas that still need cleanup include:
- The image decode color profile in Blink is still in a static local
  variable, and cannot be updated after being initially set.
- The image decode color profile is still not multi-monitor aware, and
  selects an arbitrary monitor's color profile for image decode.
- The process of calling UpdateDisplayColorProfileCache could be cleaned
  up.
  - This should be no more expensive than before in that we are still doing
    about one BrowserThread::PostBlockingPoolTask per renderer -- we're
    just doing it ahead of time now instead of in response to the IPC.
  - There is much to be done in cleaning up gfx::ColorProfile and
    meshing it with gfx::Display. This patch (pushing the color profile
    to the renderer) is a prerequisite to that cleanup.

Committed: https://crrev.com/6fc2fd3b6c99c97632b7fe43bdbf1fa12ad352ca
Cr-Commit-Position: refs/heads/master@{#402501}
==========

[commit-bot: I haz the power, 2016-06-28 17:45:35]

Patchset 15 (id:??) landed as
https://crrev.com/6fc2fd3b6c99c97632b7fe43bdbf1fa12ad352ca
Cr-Commit-Position: refs/heads/master@{#402501}