Allow absl::variant and convert ui/base/image_model.* to use it.

Bug: none Change-Id: I9fef002a0abf8ca87ecf044f8824448aa865feef Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2327456 Commit-Queue: Peter Kasting <pkasting@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Cr-Commit-Position: refs/heads/master@{#793680}
qweri0p · Jul 31, 2020 · a9cf1c5 · a9cf1c5
1 parent f41146c
commit a9cf1c5
Show file tree

Hide file tree

Showing 7 changed files with 40 additions and 53 deletions.
diff --git a/DEPS b/DEPS
@@ -4102,6 +4102,7 @@ include_rules = [
   # explicitly here.
   '-absl',
   '-third_party/abseil-cpp',
+  '+third_party/abseil-cpp/absl/types/variant.h',
 ]
 
 

diff --git a/styleguide/c++/c++11.html b/styleguide/c++/c++11.html
@@ -1,4 +1,4 @@
-<!DOCTYPE html>
+<!DOCTYPE html>
 <!--
 Copyright 2014 The Chromium Authors. All rights reserved.
 Use of this source code is governed by a BSD-style license that can be
@@ -290,7 +290,11 @@ <h2 id="allowlist_abseil"><a name="absl-allowlist"></a>Abseil Allowed Library Fe
 </tr>
 
 <tr>
-<td colspan="5" style="text-align:center">TBD</td>
+<td>Variant</td>
+<td><code>absl::variant</code></td>
+<td>Early adaptation of C++17 std::variant.</td>
+<td><a href="https://en.cppreference.com/w/cpp/utility/variant">std::variant</a></td>
+<td><a href="https://groups.google.com/a/chromium.org/g/cxx/c/DqvG-TpvMyU">Discussion thread</a></td>
 </tr>
 
 </tbody>
@@ -469,14 +473,6 @@ <h2 id="review_abseil"><a name="absl-review"></a>Abseil TBD Features</h2>
 <td>Overlaps with <code>Time</code> APIs in <code>base/</code>.</td>
 </tr>
 
-<tr>
-<td>Variant</td>
-<td><code>absl::variant</code></td>
-<td>Early adaptation of C++17 std::variant.</td>
-<td><a href="https://en.cppreference.com/w/cpp/utility/variant">std::variant</a></td>
-<td></td>
-</tr>
-
 </tbody>
 </table>
 

diff --git a/ui/base/BUILD.gn b/ui/base/BUILD.gn
@@ -428,6 +428,7 @@ jumbo_component("base") {
     ":ui_data_pack",
     "//base",
     "//skia",
+    "//third_party/abseil-cpp:absl",
     "//ui/gfx",
     "//ui/gfx/geometry",
   ]

diff --git a/ui/base/models/image_model.cc b/ui/base/models/image_model.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <tuple>
+
 #include "ui/base/models/image_model.h"
 
 namespace ui {
@@ -11,7 +13,7 @@ VectorIconModel::VectorIconModel() = default;
 VectorIconModel::VectorIconModel(const gfx::VectorIcon& vector_icon,
                                  int color_id,
                                  int icon_size)
-    : vector_icon_(&vector_icon), icon_size_(icon_size), color_id_(color_id) {}
+    : vector_icon_(&vector_icon), icon_size_(icon_size), color_(color_id) {}
 
 VectorIconModel::VectorIconModel(const gfx::VectorIcon& vector_icon,
                                  SkColor color,
@@ -29,8 +31,8 @@ VectorIconModel::VectorIconModel(VectorIconModel&&) = default;
 VectorIconModel& VectorIconModel::operator=(VectorIconModel&&) = default;
 
 bool VectorIconModel::operator==(const VectorIconModel& other) const {
-  return vector_icon_ == other.vector_icon_ && icon_size_ == other.icon_size_ &&
-         color_ == other.color_ && color_id_ == other.color_id_;
+  return std::tie(vector_icon_, icon_size_, color_) ==
+         std::tie(other.vector_icon_, other.icon_size_, other.color_);
 }
 
 bool VectorIconModel::operator!=(const VectorIconModel& other) const {
@@ -40,9 +42,9 @@ bool VectorIconModel::operator!=(const VectorIconModel& other) const {
 ImageModel::ImageModel() = default;
 
 ImageModel::ImageModel(const VectorIconModel& vector_icon_model)
-    : vector_icon_model_(vector_icon_model) {}
+    : icon_(vector_icon_model) {}
 
-ImageModel::ImageModel(const gfx::Image& image) : image_(image) {}
+ImageModel::ImageModel(const gfx::Image& image) : icon_(image) {}
 
 ImageModel::ImageModel(const gfx::ImageSkia& image_skia)
     : ImageModel(gfx::Image(image_skia)) {}
@@ -86,11 +88,13 @@ bool ImageModel::IsEmpty() const {
 }
 
 bool ImageModel::IsVectorIcon() const {
-  return vector_icon_model_ && !vector_icon_model_.value().is_empty();
+  return absl::holds_alternative<VectorIconModel>(icon_) &&
+         !absl::get<VectorIconModel>(icon_).is_empty();
 }
 
 bool ImageModel::IsImage() const {
-  return image_ && !image_.value().IsEmpty();
+  return absl::holds_alternative<gfx::Image>(icon_) &&
+         !absl::get<gfx::Image>(icon_).IsEmpty();
 }
 
 gfx::Size ImageModel::Size() const {
@@ -101,33 +105,18 @@ gfx::Size ImageModel::Size() const {
   return IsImage() ? GetImage().Size() : gfx::Size();
 }
 
-const VectorIconModel ImageModel::GetVectorIcon() const {
+VectorIconModel ImageModel::GetVectorIcon() const {
   DCHECK(IsVectorIcon());
-  return vector_icon_model_.value();
+  return absl::get<VectorIconModel>(icon_);
 }
 
-const gfx::Image ImageModel::GetImage() const {
+gfx::Image ImageModel::GetImage() const {
   DCHECK(IsImage());
-  return image_.value();
+  return absl::get<gfx::Image>(icon_);
 }
 
 bool ImageModel::operator==(const ImageModel& other) const {
-  if (IsEmpty() != other.IsEmpty())
-    return false;
-
-  if (IsEmpty())
-    return true;
-
-  if (IsVectorIcon() != other.IsVectorIcon())
-    return false;
-
-  if (IsImage()) {
-    return GetImage().AsImageSkia().BackedBySameObjectAs(
-        other.GetImage().AsImageSkia());
-  }
-
-  DCHECK(IsVectorIcon());
-  return GetVectorIcon() == other.GetVectorIcon();
+  return icon_ == other.icon_;
 }
 
 bool ImageModel::operator!=(const ImageModel& other) const {

diff --git a/ui/base/models/image_model.h b/ui/base/models/image_model.h
@@ -7,8 +7,9 @@
 
 #include "base/callback.h"
 #include "base/component_export.h"
-#include "base/optional.h"
+#include "third_party/abseil-cpp/absl/types/variant.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "ui/gfx/color_palette.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia.h"
 
@@ -57,17 +58,13 @@ class COMPONENT_EXPORT(UI_BASE) VectorIconModel {
 
   const gfx::VectorIcon* vector_icon() const { return vector_icon_; }
   int icon_size() const { return icon_size_; }
-  int color_id() const { return color_id_.value(); }
-  SkColor color() const { return color_.value(); }
-  bool has_color() const { return color_.has_value(); }
+  int color_id() const { return absl::get<int>(color_); }
+  SkColor color() const { return absl::get<SkColor>(color_); }
+  bool has_color() const { return absl::holds_alternative<SkColor>(color_); }
 
   const gfx::VectorIcon* vector_icon_ = nullptr;
   int icon_size_ = 0;
-  // Only one of the following will ever be assigned.
-  // TODO: Update to use std::variant or base:Variant once one of them is
-  // available to use.
-  base::Optional<int> color_id_;
-  base::Optional<SkColor> color_;
+  absl::variant<int, SkColor> color_ = gfx::kPlaceholderColor;
 };
 
 // ImageModel encapsulates either a gfx::Image or a VectorIconModel. Only one
@@ -97,8 +94,8 @@ class COMPONENT_EXPORT(UI_BASE) ImageModel {
   bool IsImage() const;
   gfx::Size Size() const;
   // Only valid if IsVectorIcon() or IsImage() return true, respectively.
-  const VectorIconModel GetVectorIcon() const;
-  const gfx::Image GetImage() const;
+  VectorIconModel GetVectorIcon() const;
+  gfx::Image GetImage() const;
 
   // Checks if both model yield equal images.
   bool operator==(const ImageModel& other) const;
@@ -109,11 +106,7 @@ class COMPONENT_EXPORT(UI_BASE) ImageModel {
   ImageModel(const gfx::ImageSkia& image_skia);
   ImageModel(const VectorIconModel& vector_icon_model);
 
-  // Only one of the following will ever be assigned.
-  // TODO: Update to use std::variant or base:Variant once one of them is
-  // available to use.
-  base::Optional<VectorIconModel> vector_icon_model_;
-  base::Optional<gfx::Image> image_;
+  absl::variant<VectorIconModel, gfx::Image> icon_;
 };
 
 }  // namespace ui

diff --git a/ui/gfx/image/image.cc b/ui/gfx/image/image.cc
@@ -371,6 +371,10 @@ Image& Image::operator=(Image&& other) noexcept = default;
 
 Image::~Image() {}
 
+bool Image::operator==(const Image& other) const {
+  return storage_ == other.storage_;
+}
+
 // static
 Image Image::CreateFrom1xBitmap(const SkBitmap& bitmap) {
   return Image(ImageSkia::CreateFrom1xBitmap(bitmap));

diff --git a/ui/gfx/image/image.h b/ui/gfx/image/image.h
@@ -93,6 +93,9 @@ class GFX_EXPORT Image {
   // representations.
   ~Image();
 
+  // True iff both images are backed by the same storage.
+  bool operator==(const Image& other) const;
+
   // Creates an image from the passed in 1x bitmap.
   // WARNING: The resulting image will be pixelated when painted on a high
   // density display.