Color: Don't duplicate ICC profile data

ui/gfx/color_space.cc

Index: ui/gfx/color_space.cc
diff --git a/ui/gfx/color_space.cc b/ui/gfx/color_space.cc
index 7650b1e3a9d274e45bbb538fb08041a19607d863..f2fdced3bb443359a01c9d44e3a8bb37c48fb32a 100644
--- a/ui/gfx/color_space.cc
+++ b/ui/gfx/color_space.cc
@@ -2,15 +2,89 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "build/build_config.h"
 #include "ui/gfx/color_space.h"
 
+#include <map>
+
+#include "base/synchronization/lock.h"
+
 namespace gfx {
 
 namespace {
 static const size_t kMinProfileLength = 128;
 static const size_t kMaxProfileLength = 4 * 1024 * 1024;
-}
+}  // namespace
+
+// The structure used to look up GlobalData structures.
+struct ColorSpace::Key {
+  Key(ColorSpace::Type type, const std::vector<char>& icc_profile)
+      : type(type), icc_profile(icc_profile) {}
+
+  bool operator<(const Key& other) const {
+    if (type < other.type)
+      return true;
+    if (type > other.type)
+      return false;
+    if (type != Type::ICC_PROFILE)
+      return false;
+
+    if (icc_profile.size() < other.icc_profile.size())
+      return true;
+    if (icc_profile.size() > other.icc_profile.size())
+      return false;
+    for (size_t i = 0; i < icc_profile.size(); ++i) {
+      if (icc_profile[i] < other.icc_profile[i])
+        return true;
+      if (icc_profile[i] > other.icc_profile[i])
+        return false;
+    }
+    return false;
+  }
+
+  ColorSpace::Type type;
+  const std::vector<char> icc_profile;
+};
+
+// Because this structure is shared across gfx::ColorSpace objects on
+// different threads, it needs to be thread-safe.
+class ColorSpace::GlobalData
+    : public base::RefCountedThreadSafe<ColorSpace::GlobalData> {
+ public:
+  static void Get(const Key& key, scoped_refptr<GlobalData>* value) {
+    base::AutoLock lock(map_lock_);
+    auto insert_result = map_.insert(std::make_pair(key, nullptr));
+    if (insert_result.second)
+      insert_result.first->second = new GlobalData(key, insert_result.first);
+    *value = make_scoped_refptr(insert_result.first->second);
+  }
+
+  const std::vector<char>& GetICCProfile() const { return icc_profile_; }
+
+ private:
+  friend class base::RefCountedThreadSafe<GlobalData>;
+
+  GlobalData(const Key& key, std::map<Key, GlobalData*>::iterator iterator)
+      : iterator_(iterator) {
+    // TODO: Compute the ICC profile for named color spaces.
+    if (key.type == Type::ICC_PROFILE)
+      icc_profile_ = key.icc_profile;
+  }
+  ~GlobalData() {
+    base::AutoLock lock(map_lock_);
+    map_.erase(iterator_);
+  }
+
+  std::vector<char> icc_profile_;
+
+  // In order to remove |this| from |map_| when its last reference goes away,
+  // keep in |iterator_| the corresponding iterator in |map_|.
+  std::map<Key, GlobalData*>::iterator iterator_;

[hubbe, 2016/07/12 05:12:40]

Seems like map_.erase(Key(icc_profile_)) would work just fine.

[ccameron, 2016/07/12 19:11:03]

True, ror now it's pretty small (we'd also need the type), but the next step
would be to throw in named color spaces, so the key would start replicating data
in the gfx::ColorSpace. But, I'm totally up for iterating on this.

+  static std::map<Key, GlobalData*> map_;
+  static base::Lock map_lock_;
+};
+
+std::map<ColorSpace::Key, ColorSpace::GlobalData*> ColorSpace::GlobalData::map_;
+base::Lock ColorSpace::GlobalData::map_lock_;

[Dan Beam, 2016/07/13 19:15:10]

i think these are adding static initializers:
https://build.chromium.org/p/chromium/builders/Linux%20x64/builds/22278

 
 ColorSpace::ColorSpace() = default;
 ColorSpace::ColorSpace(ColorSpace&& other) = default;
@@ -19,13 +93,26 @@ ColorSpace& ColorSpace::operator=(const ColorSpace& other) = default;
 ColorSpace::~ColorSpace() = default;
 
 bool ColorSpace::operator==(const ColorSpace& other) const {
-  return icc_profile_ == other.icc_profile_;
+  if (type_ == Type::ICC_PROFILE && other.type_ == Type::ICC_PROFILE)
+    return global_data_ == other.global_data_;
+  return type_ == other.type_;
 }
 
+bool ColorSpace::operator<(const ColorSpace& other) const {
+  // Note that this does a pointer-based comparision.
+  if (type_ == Type::ICC_PROFILE && other.type_ == Type::ICC_PROFILE)
+    return global_data_.get() < other.global_data_.get();
+  return type_ < other.type_;
+}
+
+// static
 ColorSpace ColorSpace::FromICCProfile(const std::vector<char>& icc_profile) {
   ColorSpace color_space;
-  if (IsValidProfileLength(icc_profile.size()))
-    color_space.icc_profile_ = icc_profile;
+  if (IsValidProfileLength(icc_profile.size())) {
+    color_space.type_ = Type::ICC_PROFILE;
+    Key key(Type::ICC_PROFILE, icc_profile);
+    GlobalData::Get(key, &color_space.global_data_);
+  }
   return color_space;
 }
 
@@ -36,6 +123,14 @@ ColorSpace ColorSpace::FromBestMonitor() {
 }
 #endif
 
+const std::vector<char>& ColorSpace::GetICCProfile() const {
+  if (!global_data_) {
+    Key key(type_, std::vector<char>());
+    GlobalData::Get(key, &global_data_);
+  }
+  return global_data_->GetICCProfile();
+}
+
 // static
 bool ColorSpace::IsValidProfileLength(size_t length) {
   return length >= kMinProfileLength && length <= kMaxProfileLength;