Adds monitor ICC profile support for win and mac

ui/gfx/color_profile_win.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/gfx/color_profile.h"
+
+#include <stdio.h>
+#include <windows.h>
+
+#include "base/file_util.h"
+#include "base/sys_byteorder.h"
+
+// Max profile size is 4M.
+#define MAX_PROFILE_LENGTH (4 * 1000 * 1000)

[Noel Gordon, 2012/06/21 16:58:33]

I don't see how this #define is particular to win32. How about we move this into
"ui/gfx/color_profile.h".

While you're are at, #define MIN_PROFILE_LENGTH (128) in the same file.

[tpayne, 2012/07/02 19:22:32]

Done.

+
+namespace {
+
+// ICC profiles store the profile length in network byte order in the first
+// bytes of the data.
+size_t ReadLengthFromString(const std::string& data) {

[Noel Gordon, 2012/06/21 16:58:33]

Nice comment, but this function has really confused matters below in
ReadProfile() and introduced bugs.  It's not helping and I'd remove it.

[tpayne, 2012/07/02 19:22:32]

Done.

+  uint32 length;
+
+  if (data.size() < sizeof(length))
+    return 0;
+
+  memcpy(&length, data.data(), sizeof(length));
+
+  return base::NetToHost32(length);
+}
+
+}  // namespace
+
+namespace gfx {
+
+void ColorProfile::ReadProfile(gfx::NativeViewId parent_window,
+                               std::vector<char>* profile) {
+  HDC screen_dc = GetDC(NULL);
+  DWORD path_len = MAX_PATH;
+  WCHAR path[MAX_PATH];
+
+  BOOL res = GetICMProfile(screen_dc,
+                           &path_len,
+                           reinterpret_cast<LPWSTR>(&path));
+  ReleaseDC(NULL, screen_dc);
+  if (!res)
+    return;
+  std::string profileData;
+  if (!file_util::ReadFileToString(FilePath(path), &profileData))
+    return;
+  if (profileData.size() > MAX_PROFILE_LENGTH)
+    return;

[Noel Gordon, 2012/06/21 16:58:33]

I'd add an additional clause here:

if (profileData.size() < MIN_PROFILE_LENGTH)
  return;

[tpayne, 2012/07/02 19:22:32]

Done.

+  size_t length = ReadLengthFromString(profileData);
+  if (length == 0 || length + sizeof(uint32) > profileData.size())

[Noel Gordon, 2012/06/21 16:58:33]

So maybe length + sizeof(uint32) could be less than than profile Data.size() and
you'd accept it.  Reading the length off the profile header seems fraught and
best left to the qcms color profile library in my opinion, and ...

[tpayne, 2012/07/02 19:22:32]

Done.

+    return;
+  const char* dataBuffer = profileData.data() + sizeof(uint32);

[Noel Gordon, 2012/06/21 16:58:33]

... as your comment @ line 18 said, the length is in the profile header.  Here
you skip dataBuffer past the length (4 bytes) and then write |length| bytes via
profile->assign().  Yikes!

If you don't provide the length within the returned color profile data, how
would the qcms library read it?  It sure wants to.  The ICC profile header is
128 bytes, length included I believe.  (Please go check).

And I'd hope that either qcms_profile_from_memory() or qcms_profile_is_bogus()
in WebCore fail with the data you are returning to it here. (Again, please
check).  Not sure how you tested this code.

[tpayne, 2012/07/02 19:22:32]

I haven't figured out a reliable way of testing, mostly because my win debugging
fu is inadequate. I ran through the win_rel and win_layout_rel bots and I ran
locally, but I was not able to verify if the profile was being used. Clearly, it
wasn't so I need a better testing approach.

+  profile->assign(dataBuffer, dataBuffer + length);
+}
+
+}  // namespace gfx
+