Revert of Color: Don't duplicate ICC profile data (patchset #3 id:40001 of...

Revert of Color: Don't duplicate ICC profile data (patchset #3 id:40001 of https://codereview.chromium.org/2140803002/ )

Reason for revert:
Broke static initializers step of sizes on Linux:
https://build.chromium.org/p/chromium/builders/Linux%20x64/builds/22278

Maybe making map_ a base::LazyInstance would solve your problem?

Original issue's description:
> Color: Don't duplicate ICC profile data
>
> Make gfx::ColorSpace have an internal pointer to a globally-unique
> structure for the color space, where its ICC profile can be stored.
>
> BUG=622133
>
> Committed: https://crrev.com/92294e78c762e993e8dcf538860abf65629f3320
> Cr-Commit-Position: refs/heads/master@{#405211}

TBR=hubbe@chromium.org,ccameron@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=622133

Review-Url: https://codereview.chromium.org/2148013002
Cr-Commit-Position: refs/heads/master@{#405221}

ui/gfx/color_space.cc

@@ -2,89 +2,15 @@
 // 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_;
-  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_;
+}
 
 ColorSpace::ColorSpace() = default;
 ColorSpace::ColorSpace(ColorSpace&& other) = default;
@@ -93,26 +19,13 @@ ColorSpace& ColorSpace::operator=(const ColorSpace& other) = default;
 ColorSpace::~ColorSpace() = default;
 
 bool ColorSpace::operator==(const ColorSpace& other) const {
-  if (type_ == Type::ICC_PROFILE && other.type_ == Type::ICC_PROFILE)
-    return global_data_ == other.global_data_;
-  return type_ == other.type_;
+  return icc_profile_ == other.icc_profile_;
 }
 
-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.type_ = Type::ICC_PROFILE;
-    Key key(Type::ICC_PROFILE, icc_profile);
-    GlobalData::Get(key, &color_space.global_data_);
-  }
+  if (IsValidProfileLength(icc_profile.size()))
+    color_space.icc_profile_ = icc_profile;
   return color_space;
 }
 
@@ -123,14 +36,6 @@ 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;

ui/gfx/color_space.h

@@ -8,7 +8,6 @@
 #include <vector>
 
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
 #include "build/build_config.h"
 #include "ui/gfx/gfx_export.h"
 
@@ -26,7 +25,6 @@ class GFX_EXPORT ColorSpace {
   ColorSpace& operator=(const ColorSpace& other);
   ~ColorSpace();
   bool operator==(const ColorSpace& other) const;
-  bool operator<(const ColorSpace& other) const;
 
   // Returns the color profile of the monitor that can best represent color.
   // This profile should be used for creating content that does not know on
@@ -37,7 +35,7 @@ class GFX_EXPORT ColorSpace {
   static ColorSpace FromCGColorSpace(CGColorSpaceRef cg_color_space);
 #endif
 
-  const std::vector<char>& GetICCProfile() const;
+  const std::vector<char>& GetICCProfile() const { return icc_profile_; }
 
 #if defined(OS_WIN)
   // This will read monitor ICC profiles from disk and cache the results for the
@@ -49,20 +47,7 @@ class GFX_EXPORT ColorSpace {
   static bool IsValidProfileLength(size_t length);
 
  private:
-  struct Key;
-  class GlobalData;
-  friend struct Key;
-  friend class GlobalData;
-  enum class Type {
-    UNDEFINED,
-    ICC_PROFILE,
-  };
-  Type type_ = Type::UNDEFINED;
-
-  // GlobalData stores large or expensive-to-compute data about a color space
-  // (e.g, ICC profile). This structure is shared by all identical ColorSpace
-  // objects in the process. It is lazily initialized for named color spaces.
-  mutable scoped_refptr<GlobalData> global_data_;
+  std::vector<char> icc_profile_;
 };
 
 }  // namespace gfx

ui/gfx/color_space_win.cc

@@ -59,10 +59,9 @@ bool ColorSpace::CachedProfilesNeedUpdate() {
 void ColorSpace::UpdateCachedProfilesOnBackgroundThread() {
   std::vector<char> icc_profile;
   ReadBestMonitorICCProfile(&icc_profile);
-  gfx::ColorSpace color_space = FromICCProfile(icc_profile);
 
   base::AutoLock lock(g_best_monitor_color_space_lock.Get());
-  g_best_monitor_color_space.Get() = color_space;
+  g_best_monitor_color_space.Get().icc_profile_ = icc_profile;
   g_has_initialized_best_monitor_color_space = true;
 }
 

ui/gfx/color_space_x11.cc

@@ -17,6 +17,7 @@ namespace gfx {
 
 // static
 ColorSpace ColorSpace::FromBestMonitor() {
+  ColorSpace color_space;
   Atom property = XInternAtom(GetXDisplay(), "_ICC_PROFILE", true);
   if (property != None) {
     Atom prop_type = None;
@@ -29,13 +30,11 @@ ColorSpace ColorSpace::FromBestMonitor() {
             0x1FFFFFFF /* MAXINT32 / 4 */, False, AnyPropertyType, &prop_type,
             &prop_format, &nitems, &nbytes,
             reinterpret_cast<unsigned char**>(&property_data)) == Success) {
-      std::vector<char> icc_profile;
-      icc_profile.assign(property_data, property_data + nitems);
+      color_space.icc_profile_.assign(property_data, property_data + nitems);
       XFree(property_data);
-      return FromICCProfile(icc_profile);
     }
   }
-  return ColorSpace();
+  return color_space;
 }
 
 }  // namespace gfx