Remove need of ICC profiles in the renderer process

Change zero-copy to specify IOSurface color profiles using
SetColorSpaceMetadataCHROMIUM rather than calling SetColorSpace
on the GpuMemoryBuffer directly.

The SetColorSpaceMetadataCHROMIUM path ends up calling
gl::GLImageIOSurface::SetColorSpace, which uses the
gfx::DisplayICCProfiles structure to ensure that we use low power
profiles when possible.

This removes the need for
- gfx::ICCProfile::FromCacheMac
- Sending gfx::ICCProfiles from the browser to the renderer in
  the content::ScreenInfo
  - Note that content::ScreenInfo::icc_profile was not ever
    actually read
  - Sending it over IPC would result it being added to the cache
    read by gfx::ICCProfile::FromCacheMac
- mojo and IPC support of gfx::ICCProfile

Bug: 869570
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I814496a5fffc8da38b4b59d7d2cc055dc47cd52b
Reviewed-on: https://chromium-review.googlesource.com/1159233
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580066}

cc/raster/zero_copy_raster_buffer_provider.cc

@@ -141,6 +141,11 @@ class ZeroCopyRasterBufferImpl : public RasterBuffer {
                                     backing_->image_id);
       gl->BindTexImage2DCHROMIUM(backing_->texture_target, backing_->image_id);
     }
+    if (resource_color_space_.IsValid()) {
+      gl->SetColorSpaceMetadataCHROMIUM(
+          backing_->texture_id,
+          reinterpret_cast<GLColorSpace>(&resource_color_space_));
+    }
     gl->BindTexture(backing_->texture_target, 0);
 
     backing_->mailbox_sync_token =
@@ -162,10 +167,10 @@ class ZeroCopyRasterBufferImpl : public RasterBuffer {
       gpu_memory_buffer_ = gpu_memory_buffer_manager_->CreateGpuMemoryBuffer(
           resource_size_, viz::BufferFormat(resource_format_), kBufferUsage,
           gpu::kNullSurfaceHandle);
-      // GpuMemoryBuffer allocation can fail (https://crbug.com/554541).
+      // Note that GpuMemoryBuffer allocation can fail.
+      // https://crbug.com/554541
       if (!gpu_memory_buffer_)
         return;
-      gpu_memory_buffer_->SetColorSpace(resource_color_space_);
     }
 
     DCHECK_EQ(1u, gfx::NumberOfPlanesForBufferFormat(

content/browser/renderer_host/display_util.cc

@@ -21,10 +21,6 @@ void DisplayUtil::DisplayToScreenInfo(ScreenInfo* screen_info,
   screen_info->available_rect = display.work_area();
   screen_info->device_scale_factor = display.device_scale_factor();
   screen_info->color_space = display.color_space();
-#if defined(OS_MACOSX)
-  screen_info->icc_profile =
-      gfx::ICCProfile::FromCacheMac(display.color_space());
-#endif
   screen_info->depth = display.color_depth();
   screen_info->depth_per_component = display.depth_per_component();
   screen_info->is_monochrome = display.is_monochrome();

content/common/frame_messages.h

@@ -352,9 +352,6 @@ IPC_STRUCT_TRAITS_END()
 IPC_STRUCT_TRAITS_BEGIN(content::ScreenInfo)
   IPC_STRUCT_TRAITS_MEMBER(device_scale_factor)
   IPC_STRUCT_TRAITS_MEMBER(color_space)
-#if defined(OS_MACOSX)
-  IPC_STRUCT_TRAITS_MEMBER(icc_profile)
-#endif
   IPC_STRUCT_TRAITS_MEMBER(depth)
   IPC_STRUCT_TRAITS_MEMBER(depth_per_component)
   IPC_STRUCT_TRAITS_MEMBER(is_monochrome)

content/common/renderer.mojom

@@ -13,7 +13,6 @@ import "services/network/public/mojom/network_types.mojom";
 import "services/service_manager/public/mojom/interface_provider.mojom";
 import "services/service_manager/public/mojom/service.mojom";
 import "ui/gfx/geometry/mojo/geometry.mojom";
-import "ui/gfx/mojo/icc_profile.mojom";
 
 struct CreateViewParams {
   // Renderer-wide preferences.

content/public/common/screen_info.h

@@ -28,15 +28,6 @@ struct CONTENT_EXPORT ScreenInfo {
     // The color space of the output display.
     gfx::ColorSpace color_space = gfx::ColorSpace::CreateSRGB();
 
-#if defined(OS_MACOSX)
-    // The ICC profile from which |color_space| was derived, if any. This is
-    // used only on macOS, to ensure that the color profile set on an IOSurface
-    // exactly match that of the display, when possible (because that has
-    // significant power implications).
-    // https://crbug.com/766736#c1
-    gfx::ICCProfile icc_profile;
-#endif
-
     // The screen depth in bits per pixel
     uint32_t depth = 0;
 

ui/gfx/icc_profile.cc

@@ -22,22 +22,6 @@ namespace {
 
 static const size_t kMaxCachedICCProfiles = 16;
 
-// An MRU cache mapping ColorSpace objects to the ICCProfile that created them.
-// This cache is necessary only on macOS, for power consumption reasons. In
-// particular:
-//  * IOSurfaces specify their output color space by raw ICC profile data.
-//  * If the IOSurface ICC profile does not exactly match the output monitor's
-//    ICC profile, there is a substantial power cost.
-//  * This structure allows us to retrieve the exact ICC profile data that
-//    produced a given ColorSpace.
-using SpaceToProfileCacheBase = base::MRUCache<ColorSpace, ICCProfile>;
-class SpaceToProfileCache : public SpaceToProfileCacheBase {
- public:
-  SpaceToProfileCache() : SpaceToProfileCacheBase(kMaxCachedICCProfiles) {}
-};
-base::LazyInstance<SpaceToProfileCache>::Leaky g_space_to_profile_cache_mac =
-    LAZY_INSTANCE_INITIALIZER;
-
 // An MRU cache mapping data to ICCProfile objects, to avoid re-parsing
 // profiles every time they are read.
 using DataToProfileCacheBase = base::MRUCache<std::vector<char>, ICCProfile>;
@@ -63,8 +47,7 @@ base::LazyInstance<IdToProfileCache>::Leaky g_id_to_profile_cache =
 // The next id to assign to a color profile.
 uint64_t g_next_unused_id = 1;
 
-// Lock that must be held to access |g_space_to_profile_cache_mac| and
-// |g_next_unused_id|.
+// Lock that must be held to access |g_next_unused_id|.
 base::LazyInstance<base::Lock>::Leaky g_icc_profile_lock =
     LAZY_INSTANCE_INITIALIZER;
 
@@ -181,8 +164,6 @@ ICCProfile ICCProfile::FromDataWithId(const void* data_as_void,
 
   // Insert the profile into all caches.
   ColorSpace color_space = icc_profile.GetColorSpace();
-  if (color_space.IsValid())
-    g_space_to_profile_cache_mac.Get().Put(color_space, icc_profile);
   if (icc_profile.internals_->id_)
     g_id_to_profile_cache.Get().Put(icc_profile.internals_->id_, icc_profile);
   g_data_to_profile_cache.Get().Put(icc_profile.internals_->data_, icc_profile);
@@ -241,19 +222,6 @@ ICCProfile ICCProfile::FromParametricColorSpace(const ColorSpace& color_space) {
   return FromDataWithId(data->data(), data->size(), 0);
 }
 
-// static
-ICCProfile ICCProfile::FromCacheMac(const ColorSpace& color_space) {
-  base::AutoLock lock(g_icc_profile_lock.Get());
-  auto found_by_space = g_space_to_profile_cache_mac.Get().Get(color_space);
-  if (found_by_space != g_space_to_profile_cache_mac.Get().end())
-    return found_by_space->second;
-
-  if (color_space.icc_profile_id_) {
-    DLOG(ERROR) << "Failed to find id-based ColorSpace in ICCProfile cache";
-  }
-  return ICCProfile();
-}
-
 // static
 sk_sp<SkColorSpace> ICCProfile::GetSkColorSpaceFromId(uint64_t id) {
   base::AutoLock lock(g_icc_profile_lock.Get());

ui/gfx/icc_profile.h

@@ -46,11 +46,6 @@ class COLOR_SPACE_EXPORT ICCProfile {
   static ICCProfile FromParametricColorSpace(
       const gfx::ColorSpace& color_space);
 
-  // Return the ICCProfile that was used to create the specified ColorSpace, if
-  // available. Otherwise, return an invalid profile. This is used on macOS for
-  // power reasons.
-  static ICCProfile FromCacheMac(const gfx::ColorSpace& color_space);
-
   // Return a ColorSpace that references this ICCProfile. ColorTransforms
   // created using this ColorSpace will match this ICCProfile precisely.
   ColorSpace GetColorSpace() const;

ui/gfx/icc_profile_unittest.cc

@@ -49,16 +49,6 @@ TEST(ICCProfile, Equality) {
   EXPECT_FALSE(spin_space == adobe_space);
   EXPECT_TRUE(spin_space != adobe_space);
 
-  ICCProfile temp = ICCProfile::FromCacheMac(spin_space);
-  EXPECT_TRUE(temp.IsValid());
-  EXPECT_TRUE(spin_profile == temp);
-  EXPECT_FALSE(spin_profile != temp);
-
-  temp = ICCProfile::FromCacheMac(adobe_space);
-  EXPECT_TRUE(temp.IsValid());
-  EXPECT_FALSE(spin_profile == temp);
-  EXPECT_TRUE(spin_profile != temp);
-
   EXPECT_TRUE(!!spin_space.ToSkColorSpace());
   EXPECT_TRUE(!!adobe_space.ToSkColorSpace());
   EXPECT_FALSE(SkColorSpace::Equals(
@@ -74,12 +64,8 @@ TEST(ICCProfile, ParametricVersusExactInaccurate) {
   ICCProfile multi_tr_fn = ICCProfileForTestingNoAnalyticTrFn();
   EXPECT_TRUE(multi_tr_fn.GetColorSpace().IsParametricAccurate());
 
-  ICCProfile profile;
-  profile = ICCProfile::FromCacheMac(multi_tr_fn.GetColorSpace());
-  EXPECT_TRUE(profile.IsValid());
-  EXPECT_EQ(profile, multi_tr_fn);
-
   // We are capable of generating a parametric approximation.
+  ICCProfile profile;
   profile = ICCProfile::FromParametricColorSpace(multi_tr_fn.GetColorSpace());
   EXPECT_TRUE(profile.IsValid());
   EXPECT_NE(profile, multi_tr_fn);
@@ -92,10 +78,6 @@ TEST(ICCProfile, ParametricVersusExactOvershoot) {
   EXPECT_TRUE(overshoot.GetColorSpace().IsParametricAccurate());
 
   ICCProfile profile;
-  profile = ICCProfile::FromCacheMac(overshoot.GetColorSpace());
-  EXPECT_TRUE(profile.IsValid());
-  EXPECT_EQ(profile, overshoot);
-
   profile = ICCProfile::FromParametricColorSpace(overshoot.GetColorSpace());
   EXPECT_TRUE(profile.IsValid());
   EXPECT_NE(profile, overshoot);
@@ -107,10 +89,6 @@ TEST(ICCProfile, ParametricVersusExactAdobe) {
   EXPECT_TRUE(accurate.GetColorSpace().IsParametricAccurate());
 
   ICCProfile profile;
-  profile = ICCProfile::FromCacheMac(accurate.GetColorSpace());
-  EXPECT_TRUE(profile.IsValid());
-  EXPECT_EQ(profile, accurate);
-
   profile = ICCProfile::FromParametricColorSpace(accurate.GetColorSpace());
   EXPECT_TRUE(profile.IsValid());
   EXPECT_NE(profile, accurate);

ui/gfx/ipc/color/gfx_param_traits.cc

@@ -60,37 +60,6 @@ void ParamTraits<gfx::ColorSpace>::Log(const gfx::ColorSpace& p,
   l->append("<gfx::ColorSpace>");
 }
 
-void ParamTraits<gfx::ICCProfile>::Write(base::Pickle* m,
-                                         const gfx::ICCProfile& p) {
-  if (p.internals_) {
-    WriteParam(m, p.internals_->id_);
-    WriteParam(m, p.internals_->data_);
-  } else {
-    uint64_t id = 0;
-    std::vector<char> data;
-    WriteParam(m, id);
-    WriteParam(m, data);
-  }
-}
-
-bool ParamTraits<gfx::ICCProfile>::Read(const base::Pickle* m,
-                                        base::PickleIterator* iter,
-                                        gfx::ICCProfile* r) {
-  uint64_t id;
-  std::vector<char> data;
-  if (!ReadParam(m, iter, &id))
-    return false;
-  if (!ReadParam(m, iter, &data))
-    return false;
-  *r = gfx::ICCProfile::FromDataWithId(data.data(), data.size(), id);
-  return true;
-}
-
-void ParamTraits<gfx::ICCProfile>::Log(const gfx::ICCProfile& p,
-                                       std::string* l) {
-  l->append("<gfx::ICCProfile>");
-}
-
 }  // namespace IPC
 
 // Generate param traits write methods.

ui/gfx/ipc/color/gfx_param_traits.h

@@ -28,16 +28,6 @@ struct GFX_IPC_COLOR_EXPORT ParamTraits<gfx::ColorSpace> {
   static void Log(const param_type& p, std::string* l);
 };
 
-template <>
-struct GFX_IPC_COLOR_EXPORT ParamTraits<gfx::ICCProfile> {
-  typedef gfx::ICCProfile param_type;
-  static void Write(base::Pickle* m, const param_type& p);
-  static bool Read(const base::Pickle* m,
-                   base::PickleIterator* iter,
-                   param_type* r);
-  static void Log(const param_type& p, std::string* l);
-};
-
 }  // namespace IPC
 
 #endif  // UI_GFX_IPC_COLOR_GFX_PARAM_TRAITS_H_

ui/gfx/mac/display_icc_profiles.cc

@@ -70,7 +70,7 @@ void DisplayICCProfiles::UpdateIfNeeded() {
     // don't store its data (we will assign the best parametric fit to
     // IOSurfaces, and rely on the system compositor to do conversion to the
     // display profile).
-    if (color_space.IsParametricAccurate())
+    if (color_space.IsValid() && color_space.IsParametricAccurate())
       map_[color_space] = icc_data;
   }
 }

ui/gfx/mac/io_surface.cc

@@ -210,46 +210,44 @@ IOSurfaceRef CreateIOSurface(const gfx::Size& size,
 
 void IOSurfaceSetColorSpace(IOSurfaceRef io_surface,
                             const ColorSpace& color_space) {
-  // Retrieve the ICC profile data that created this profile, if it exists.
-  ICCProfile icc_profile = ICCProfile::FromCacheMac(color_space);
+  // Special-case sRGB.
+  if (color_space == ColorSpace::CreateSRGB()) {
+    base::ScopedCFTypeRef<CFDataRef> srgb_icc(
+        CGColorSpaceCopyICCProfile(base::mac::GetSRGBColorSpace()));
+    IOSurfaceSetValue(io_surface, CFSTR("IOSurfaceColorSpace"), srgb_icc);
+    return;
+  }
 
-  // If that fails, generate parametric data.
-  if (!icc_profile.IsValid()) {
-    if (color_space == ColorSpace::CreateSRGB()) {
-      base::ScopedCFTypeRef<CFDataRef> srgb_icc(
-          CGColorSpaceCopyICCProfile(base::mac::GetSRGBColorSpace()));
-      IOSurfaceSetValue(io_surface, CFSTR("IOSurfaceColorSpace"), srgb_icc);
+  // Special-case BT2020_NCL.
+  if (__builtin_available(macos 10.12, *)) {
+    const ColorSpace kBt2020(
+        ColorSpace::PrimaryID::BT2020, ColorSpace::TransferID::SMPTEST2084,
+        ColorSpace::MatrixID::BT2020_NCL, ColorSpace::RangeID::LIMITED);
+    if (color_space == kBt2020) {
+      base::ScopedCFTypeRef<CGColorSpaceRef> cg_color_space(
+          CGColorSpaceCreateWithName(kCGColorSpaceITUR_2020));
+      DCHECK(cg_color_space);
+
+      base::ScopedCFTypeRef<CFDataRef> cf_data_icc_profile(
+          CGColorSpaceCopyICCData(cg_color_space));
+      DCHECK(cf_data_icc_profile);
+      IOSurfaceSetValue(io_surface, CFSTR("IOSurfaceColorSpace"),
+                        cf_data_icc_profile);
       return;
     }
-    icc_profile =
-        ICCProfile::FromParametricColorSpace(color_space.GetAsFullRangeRGB());
   }
+
+  // Generate an ICCProfile from the parametric color space.
+  ICCProfile icc_profile =
+      ICCProfile::FromParametricColorSpace(color_space.GetAsFullRangeRGB());
   if (!icc_profile.IsValid()) {
-    if (__builtin_available(macos 10.12, *)) {
-      static const ColorSpace kBt2020(ColorSpace::PrimaryID::BT2020,
-                                      ColorSpace::TransferID::SMPTEST2084,
-                                      ColorSpace::MatrixID::BT2020_NCL,
-                                      ColorSpace::RangeID::LIMITED);
-      if (color_space == kBt2020) {
-        base::ScopedCFTypeRef<CGColorSpaceRef> cg_color_space(
-            CGColorSpaceCreateWithName(kCGColorSpaceITUR_2020));
-        DCHECK(cg_color_space);
-
-        base::ScopedCFTypeRef<CFDataRef> cf_data_icc_profile(
-            CGColorSpaceCopyICCData(cg_color_space));
-        DCHECK(cf_data_icc_profile);
-        IOSurfaceSetValue(io_surface, CFSTR("IOSurfaceColorSpace"),
-                          cf_data_icc_profile);
-        return;
-      }
-    }
     DLOG(ERROR) << "Failed to set color space for IOSurface: no ICC profile: "
                 << color_space.ToString();
     return;
   }
-  std::vector<char> icc_profile_data = icc_profile.GetData();
 
   // Package it as a CFDataRef and send it to the IOSurface.
+  std::vector<char> icc_profile_data = icc_profile.GetData();
   base::ScopedCFTypeRef<CFDataRef> cf_data_icc_profile(CFDataCreate(
       nullptr, reinterpret_cast<const UInt8*>(icc_profile_data.data()),
       icc_profile_data.size()));

ui/gfx/mojo/BUILD.gn

@@ -12,7 +12,6 @@ mojom("mojo") {
     "color_space.mojom",
     "font_render_params.mojom",
     "gpu_fence_handle.mojom",
-    "icc_profile.mojom",
     "overlay_transform.mojom",
     "presentation_feedback.mojom",
     "selection_bound.mojom",

ui/gfx/mojo/icc_profile.mojom

-// Copyright 2016 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.
-
-module gfx.mojom;
-
-import "ui/gfx/mojo/color_space.mojom";
-
-[Native]
-struct ICCProfile;

ui/gfx/mojo/icc_profile.typemap

-# Copyright 2016 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.
-
-mojom = "//ui/gfx/mojo/icc_profile.mojom"
-public_headers = [ "//ui/gfx/icc_profile.h" ]
-traits_headers = [ "//ui/gfx/ipc/color/gfx_param_traits.h" ]
-public_deps = [
-  "//ipc",
-  "//ui/gfx",
-]
-deps = [
-  "//ui/gfx/ipc/color",
-]
-type_mappings = [ "gfx.mojom.ICCProfile=gfx::ICCProfile" ]

ui/gfx/typemaps.gni

@@ -11,7 +11,6 @@ typemaps = [
   "//ui/gfx/mojo/color_space.typemap",
   "//ui/gfx/mojo/font_render_params.typemap",
   "//ui/gfx/mojo/gpu_fence_handle.typemap",
-  "//ui/gfx/mojo/icc_profile.typemap",
   "//ui/gfx/mojo/overlay_transform.typemap",
   "//ui/gfx/mojo/presentation_feedback.typemap",
   "//ui/gfx/mojo/selection_bound.typemap",