[base] Use absl::variant in base::Value

This change makes use of absl::variant inside of base::Value and
replaces the manually written tagged union.

Since adding an Abseil #include requires propagation of the
corresponding absl_include_config, several deps have been changed to
public_deps in order to avoid build errors.

TBR=battre

Bug: 646113
Change-Id: I781fb43aa0eb4caacbda7f745babd042755f199c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2332182
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799725}

base/values_unittest.cc

@@ -36,74 +36,23 @@ namespace base {
 // This test is only enabled when NDEBUG is defined. This way the test will not
 // fail in debug builds that sometimes contain larger versions of the standard
 // containers used inside base::Value.
-#if defined(NDEBUG)
-
 TEST(ValuesTest, SizeOfValue) {
-#define INNER_TYPES_LIST(X)              \
-  X(bool, bool_value_)                   \
-  X(int, int_value_)                     \
-  X(Value::DoubleStorage, double_value_) \
-  X(std::string, string_value_)          \
-  X(Value::BlobStorage, binary_value_)   \
-  X(Value::ListStorage, list_)           \
-  X(Value::DictStorage, dict_)
-
-#define INNER_FIELD_ALIGNMENT(type, value) alignof(type),
-
-  // The maximum alignment of each inner struct value field inside base::Value
-  size_t max_inner_value_alignment =
-      std::max({INNER_TYPES_LIST(INNER_FIELD_ALIGNMENT)});
-
-  // Check that base::Value has the smallest alignment possible. This would
-  // fail if the header would contain something that has a larger alignment
-  // than necessary.
-  EXPECT_EQ(max_inner_value_alignment, alignof(Value));
-
-  // Find the offset of each inner value. Which should normally not be
-  // larger than 4. Note that we use std::max(4, ...) because bool_value_
-  // could be stored just after the |bool_type_| field, with an offset of
-  // 1, and that would be ok.
-#define INNER_VALUE_START_OFFSET(type, value) offsetof(Value, value),
-
-  size_t min_inner_value_offset =
-      std::min({INNER_TYPES_LIST(INNER_VALUE_START_OFFSET)});
-
-  // Inner fields may contain pointers, which have an alignment of 8
-  // on most 64-bit platforms.
-  size_t expected_min_offset = alignof(void*);
-
-  EXPECT_EQ(expected_min_offset, min_inner_value_offset);
-
-  // Ensure that base::Value is not larger than necessary, i.e. that there is
-  // no un-necessary padding after the structs due to alignment constraints of
-  // one of the inner fields.
-#define INNER_STRUCT_END_OFFSET(type, value) \
-  offsetof(Value, value) + sizeof(type),
-
-  // The maximum size in bytes of each inner struct inside base::Value,
-  size_t max_inner_struct_end_offset =
-      std::max({INNER_TYPES_LIST(INNER_STRUCT_END_OFFSET)});
-
-  // The expected value size.
-  size_t expected_value_size =
-      bits::Align(max_inner_struct_end_offset, alignof(Value));
-
-  EXPECT_EQ(expected_value_size, sizeof(Value));
-  if (min_inner_value_offset != expected_min_offset ||
-      expected_value_size != sizeof(Value)) {
-    // The following are useful to understand what's wrong when the EXPECT_EQ()
-    // above actually fail.
-#define PRINT_INNER_FIELD_INFO(x, y)                           \
-  LOG(INFO) << #y " type=" #x " offset=" << offsetof(Value, y) \
-            << " size=" << sizeof(x) << " align=" << alignof(x);
-
-    LOG(INFO) << "Value size=" << sizeof(Value) << " align=" << alignof(Value);
-    INNER_TYPES_LIST(PRINT_INNER_FIELD_INFO)
-    LOG(INFO) << "max_inner_struct_end_offset=" << max_inner_struct_end_offset;
-  }
-}
+  static_assert(
+      std::max({alignof(size_t), alignof(bool), alignof(int),
+                alignof(Value::DoubleStorage), alignof(std::string),
+                alignof(Value::BlobStorage), alignof(Value::ListStorage),
+                alignof(Value::DictStorage)}) == alignof(Value),
+      "Value does not have smallest possible alignof");
 
-#endif  // NDEBUG
+  static_assert(
+      sizeof(size_t) +
+              std::max({sizeof(bool), sizeof(int), sizeof(Value::DoubleStorage),
+                        sizeof(std::string), sizeof(Value::BlobStorage),
+                        sizeof(Value::ListStorage),
+                        sizeof(Value::DictStorage)}) ==
+          sizeof(Value),
+      "Value does not have smallest possible sizeof");
+}
 
 TEST(ValuesTest, TestNothrow) {
   static_assert(std::is_nothrow_move_constructible<Value>::value,
@@ -240,6 +189,18 @@ TEST(ValuesTest, ConstructListFromStorage) {
   }
 }
 
+TEST(ValuesTest, HardenTests) {
+  Value value;
+  ASSERT_EQ(value.type(), Value::Type::NONE);
+  EXPECT_DEATH_IF_SUPPORTED(value.GetBool(), "");
+  EXPECT_DEATH_IF_SUPPORTED(value.GetInt(), "");
+  EXPECT_DEATH_IF_SUPPORTED(value.GetDouble(), "");
+  EXPECT_DEATH_IF_SUPPORTED(value.GetString(), "");
+  EXPECT_DEATH_IF_SUPPORTED(value.GetBlob(), "");
+  EXPECT_DEATH_IF_SUPPORTED(value.DictItems(), "");
+  EXPECT_DEATH_IF_SUPPORTED(value.GetList(), "");
+}
+
 // Group of tests for the copy constructors and copy-assigmnent. For equality
 // checks comparisons of the interesting fields are done instead of relying on
 // Equals being correct.
@@ -1474,8 +1435,8 @@ TEST(ValuesTest, ListRemoval) {
     ListValue list;
     list.Append(std::make_unique<Value>());
     EXPECT_EQ(1U, list.GetSize());
-    EXPECT_FALSE(list.Remove(std::numeric_limits<size_t>::max(),
-                             &removed_item));
+    EXPECT_FALSE(
+        list.Remove(std::numeric_limits<size_t>::max(), &removed_item));
     EXPECT_FALSE(list.Remove(1, &removed_item));
     EXPECT_TRUE(list.Remove(0, &removed_item));
     ASSERT_TRUE(removed_item);
@@ -2080,7 +2041,7 @@ TEST(ValuesTest, RemoveEmptyChildren) {
     root = root->DeepCopyWithoutEmptyChildren();
     EXPECT_EQ(3U, root->size());
 
-    ListValue* inner_value, *inner_value2;
+    ListValue *inner_value, *inner_value2;
     EXPECT_TRUE(root->GetList("list_with_empty_children", &inner_value));
     EXPECT_EQ(1U, inner_value->GetSize());  // Dictionary was pruned.
     EXPECT_TRUE(inner_value->GetList(0, &inner_value2));
@@ -2126,8 +2087,8 @@ TEST(ValuesTest, MergeDictionary) {
   EXPECT_TRUE(res_sub_dict->GetString("sub_base_key", &sub_base_key_value));
   EXPECT_EQ("sub_base_key_value_base", sub_base_key_value);  // Preserved.
   std::string sub_collide_key_value;
-  EXPECT_TRUE(res_sub_dict->GetString("sub_collide_key",
-                                      &sub_collide_key_value));
+  EXPECT_TRUE(
+      res_sub_dict->GetString("sub_collide_key", &sub_collide_key_value));
   EXPECT_EQ("sub_collide_key_value_merge", sub_collide_key_value);  // Replaced.
   std::string sub_merge_key_value;
   EXPECT_TRUE(res_sub_dict->GetString("sub_merge_key", &sub_merge_key_value));

chrome/chrome_cleaner/logging/BUILD.gn

@@ -37,7 +37,6 @@ source_set("common") {
   deps = [
     ":logging_definitions",
     ":scoped_timed_task_logger",
-    "//base",
     "//chrome/chrome_cleaner/constants:common_strings",
     "//chrome/chrome_cleaner/constants:version_header",
     "//chrome/chrome_cleaner/http",  # For safe_browsing_reporter
@@ -54,6 +53,7 @@ source_set("common") {
   ]
 
   public_deps = [
+    "//base",
     "//chrome/chrome_cleaner/logging/proto:shared_data_proto",
     "//net/traffic_annotation:traffic_annotation",
   ]
@@ -134,7 +134,6 @@ static_library("cleaner_logging") {
     ":api_keys",
     ":api_keys_header",
     ":common",
-    "//base",
     "//chrome/chrome_cleaner/chrome_utils:chrome_util_lib",
     "//chrome/chrome_cleaner/constants:chrome_cleanup_tool_branding_header",
     "//chrome/chrome_cleaner/constants:common_strings",
@@ -149,6 +148,8 @@ static_library("cleaner_logging") {
     "//chrome/chrome_cleaner/strings",
     "//components/chrome_cleaner/public/constants:constants",
   ]
+
+  public_deps = [ "//base" ]
 }
 
 static_library("reporter_logging") {
@@ -183,12 +184,13 @@ static_library("noop_logging") {
 
   deps = [
     ":common",
-    "//base",
     "//chrome/chrome_cleaner/os:common_os",
     "//chrome/chrome_cleaner/proto:shared_pup_enums_proto",
     "//chrome/chrome_cleaner/pup_data:pup_data_base",
     "//components/chrome_cleaner/public/constants:constants",
   ]
+
+  public_deps = [ "//base" ]
 }
 
 source_set("mock_logging_service") {

components/content_settings/core/common/BUILD.gn

@@ -28,10 +28,12 @@ static_library("common") {
 
   configs += [ "//build/config/compiler:wexit_time_destructors" ]
 
-  public_deps = [ ":features" ]
+  public_deps = [
+    ":features",
+    "//base",
+  ]
 
   deps = [
-    "//base",
     "//ipc",
     "//mojo/public/cpp/base",
     "//mojo/public/cpp/bindings:struct_traits",

components/prefs/BUILD.gn

@@ -50,10 +50,9 @@ component("prefs") {
 
   defines = [ "COMPONENTS_PREFS_IMPLEMENTATION" ]
 
-  deps = [
-    "//base",
-    "//base/util/values:values_util",
-  ]
+  deps = [ "//base/util/values:values_util" ]
+
+  public_deps = [ "//base" ]
 
   if (is_android) {
     sources += [

components/safe_browsing/core/common/BUILD.gn

@@ -13,13 +13,14 @@ static_library("safe_browsing_prefs") {
   ]
 
   deps = [
-    "//base:base",
     "//components/pref_registry:pref_registry",
     "//components/prefs",
     "//components/safe_browsing/core:features",
     "//components/safe_browsing/core/common:thread_utils",
     "//net:net",
   ]
+
+  public_deps = [ "//base" ]
 }
 
 source_set("safe_browsing_policy_handler") {

components/safe_browsing/core/db/BUILD.gn

@@ -127,9 +127,11 @@ static_library("v4_get_hash_protocol_manager") {
     "v4_get_hash_protocol_manager.cc",
     "v4_get_hash_protocol_manager.h",
   ]
-  public_deps = [ ":safebrowsing_proto" ]
-  deps = [
+  public_deps = [
+    ":safebrowsing_proto",
     ":util",
+  ]
+  deps = [
     ":v4_protocol_manager_util",
     "//base",
     "//components/safe_browsing/core:features",

content/public/browser/content_browser_client.cc

@@ -9,7 +9,7 @@
 // declarations instead of including more headers. If that is infeasible, adjust
 // the limit. For more info, see
 // https://chromium.googlesource.com/chromium/src/+/HEAD/docs/wmax_tokens.md
-#pragma clang max_tokens_here 860000
+#pragma clang max_tokens_here 880000
 
 #include <utility>
 

ios/chrome/test/earl_grey/BUILD.gn

@@ -206,12 +206,14 @@ source_set("eg_test_support+eg2") {
     "scoped_block_popups_pref.mm",
   ]
 
-  public_deps = [ ":chrome_egtest_bundle_main+eg2" ]
+  public_deps = [
+    ":chrome_egtest_bundle_main+eg2",
+    "//components/content_settings/core/common",
+  ]
 
   deps = [
     "//base",
     "//base/test:test_support",
-    "//components/content_settings/core/common:common",
     "//components/strings",
     "//components/sync/base",
     "//ios/chrome/app/strings",

net/BUILD.gn

@@ -1766,6 +1766,7 @@ source_set("net_public_deps") {
   public_deps = [
     ":buildflags",
     ":net_nqe_proto",
+    "//base",
     "//crypto",
     "//crypto:platform",
     "//net/third_party/quiche:net_quic_proto",