mirror of
				https://bitbucket.org/chromiumembedded/cef
				synced 2025-06-05 21:39:12 +02:00 
			
		
		
		
	
		
			
				
	
	
		
			260 lines
		
	
	
		
			9.0 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
			
		
		
	
	
			260 lines
		
	
	
		
			9.0 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
| diff --git base/values.cc base/values.cc
 | |
| index 6f3a9e2cd8a2..d35972814034 100644
 | |
| --- base/values.cc
 | |
| +++ base/values.cc
 | |
| @@ -22,20 +22,6 @@
 | |
|  
 | |
|  namespace base {
 | |
|  
 | |
| -// base::Value must be standard layout to guarantee that writing to
 | |
| -// |bool_type_| then reading |type_| is defined behaviour. See:
 | |
| -//
 | |
| -// [class.union]:
 | |
| -//   If a standard-layout union contains several standard-layout structs that
 | |
| -//   share a common initial sequence (9.2), and if an object of this
 | |
| -//   standard-layout union type contains one of the standard-layout structs,
 | |
| -//   it is permitted to inspect the common initial sequence of any of
 | |
| -//   standard-layout struct members;
 | |
| -//
 | |
| -static_assert(std::is_standard_layout<Value>::value,
 | |
| -              "base::Value should be a standard-layout C++ class in order "
 | |
| -              "to avoid undefined behaviour in its implementation!");
 | |
| -
 | |
|  namespace {
 | |
|  
 | |
|  const char* const kTypeNames[] = {"null",   "boolean", "integer",    "double",
 | |
| @@ -150,15 +136,15 @@ Value::Value(Type type) : type_(type) {
 | |
|  }
 | |
|  
 | |
|  Value::Value(bool in_bool)
 | |
| -    : bool_type_(Type::BOOLEAN),
 | |
| +    : type_(Type::BOOLEAN),
 | |
|        bool_value_(in_bool) {}
 | |
|  
 | |
|  Value::Value(int in_int)
 | |
| -    : int_type_(Type::INTEGER),
 | |
| +    : type_(Type::INTEGER),
 | |
|        int_value_(in_int) {}
 | |
|  
 | |
|  Value::Value(double in_double)
 | |
| -    : double_type_(Type::DOUBLE),
 | |
| +    : type_(Type::DOUBLE),
 | |
|        double_value_(in_double) {
 | |
|    if (!std::isfinite(double_value_)) {
 | |
|      NOTREACHED() << "Non-finite (i.e. NaN or positive/negative infinity) "
 | |
| @@ -172,7 +158,7 @@ Value::Value(const char* in_string) : Value(std::string(in_string)) {}
 | |
|  Value::Value(StringPiece in_string) : Value(std::string(in_string)) {}
 | |
|  
 | |
|  Value::Value(std::string&& in_string) noexcept
 | |
| -    : string_type_(Type::STRING),
 | |
| +    : type_(Type::STRING),
 | |
|        string_value_(std::move(in_string)) {
 | |
|    DCHECK(IsStringUTF8(string_value_));
 | |
|  }
 | |
| @@ -182,19 +168,19 @@ Value::Value(const char16* in_string16) : Value(StringPiece16(in_string16)) {}
 | |
|  Value::Value(StringPiece16 in_string16) : Value(UTF16ToUTF8(in_string16)) {}
 | |
|  
 | |
|  Value::Value(const std::vector<char>& in_blob)
 | |
| -    : binary_type_(Type::BINARY),
 | |
| +    : type_(Type::BINARY),
 | |
|        binary_value_(in_blob.begin(), in_blob.end()) {}
 | |
|  
 | |
|  Value::Value(base::span<const uint8_t> in_blob)
 | |
| -    : binary_type_(Type::BINARY),
 | |
| +    : type_(Type::BINARY),
 | |
|        binary_value_(in_blob.begin(), in_blob.end()) {}
 | |
|  
 | |
|  Value::Value(BlobStorage&& in_blob) noexcept
 | |
| -    : binary_type_(Type::BINARY),
 | |
| +    : type_(Type::BINARY),
 | |
|        binary_value_(std::move(in_blob)) {}
 | |
|  
 | |
|  Value::Value(const DictStorage& in_dict)
 | |
| -    : dict_type_(Type::DICTIONARY), dict_() {
 | |
| +    : type_(Type::DICTIONARY), dict_() {
 | |
|    dict_.reserve(in_dict.size());
 | |
|    for (const auto& it : in_dict) {
 | |
|      dict_.try_emplace(dict_.end(), it.first,
 | |
| @@ -203,17 +189,17 @@ Value::Value(const DictStorage& in_dict)
 | |
|  }
 | |
|  
 | |
|  Value::Value(DictStorage&& in_dict) noexcept
 | |
| -    : dict_type_(Type::DICTIONARY),
 | |
| +    : type_(Type::DICTIONARY),
 | |
|        dict_(std::move(in_dict)) {}
 | |
|  
 | |
| -Value::Value(const ListStorage& in_list) : list_type_(Type::LIST), list_() {
 | |
| +Value::Value(const ListStorage& in_list) : type_(Type::LIST), list_() {
 | |
|    list_.reserve(in_list.size());
 | |
|    for (const auto& val : in_list)
 | |
|      list_.emplace_back(val.Clone());
 | |
|  }
 | |
|  
 | |
|  Value::Value(ListStorage&& in_list) noexcept
 | |
| -    : list_type_(Type::LIST),
 | |
| +    : type_(Type::LIST),
 | |
|        list_(std::move(in_list)) {}
 | |
|  
 | |
|  Value& Value::operator=(Value&& that) noexcept {
 | |
| diff --git base/values.h base/values.h
 | |
| index 7bc355ee586d..441967681a4b 100644
 | |
| --- base/values.h
 | |
| +++ base/values.h
 | |
| @@ -390,78 +390,18 @@ class BASE_EXPORT Value {
 | |
|    size_t EstimateMemoryUsage() const;
 | |
|  
 | |
|   protected:
 | |
| -  // Technical note:
 | |
| -  // The naive way to implement a tagged union leads to wasted bytes
 | |
| -  // in the object on CPUs like ARM ones, which impose an 8-byte alignment
 | |
| -  // for double values. I.e. if one does something like:
 | |
| -  //
 | |
| -  //    struct TaggedValue {
 | |
| -  //      int type_;                    // size = 1, align = 4
 | |
| -  //      union {
 | |
| -  //        bool bool_value_;           // size = 1, align = 1
 | |
| -  //        int int_value_;             // size = 4, align = 4
 | |
| -  //        double double_value_;       // size = 8, align = 8
 | |
| -  //        std::string string_value_;  // size = 12, align = 4  (32-bit)
 | |
| -  //      };
 | |
| -  //    };
 | |
| -  //
 | |
| -  // The end result is that the union will have an alignment of 8, and a size
 | |
| -  // of 16, due to 4 extra padding bytes following |string_value_| to respect
 | |
| -  // the alignment requirement.
 | |
| -  //
 | |
| -  // As a consequence, the struct TaggedValue will have a size of 24 bytes,
 | |
| -  // due to the size of the union (16), the size of |type_| (4) and 4 bytes
 | |
| -  // of padding between |type_| and the union to respect its alignment.
 | |
| -  //
 | |
| -  // This means 8 bytes of unused memory per instance on 32-bit ARM!
 | |
| -  //
 | |
| -  // To reclaim these, a union of structs is used instead, in order to ensure
 | |
| -  // that |double_value_| below is always located at an offset that is a
 | |
| -  // multiple of 8, relative to the start of the overall data structure.
 | |
| -  //
 | |
| -  // Each struct must declare its own |type_| field, which must have a different
 | |
| -  // name, to appease the C++ compiler.
 | |
| -  //
 | |
| -  // Using this technique sizeof(base::Value) == 16 on 32-bit ARM instead
 | |
| -  // of 24, without losing any information. Results are unchanged for x86,
 | |
| -  // x86_64 and arm64 (16, 32 and 32 bytes respectively).
 | |
| +   // TODO(crbug.com/646113): Make these private once DictionaryValue and
 | |
| +   // ListValue are properly inlined.
 | |
| +   Type type_;
 | |
| +
 | |
|    union {
 | |
| -    struct {
 | |
| -      // TODO(crbug.com/646113): Make these private once DictionaryValue and
 | |
| -      // ListValue are properly inlined.
 | |
| -      Type type_ : 8;
 | |
| -    };
 | |
| -    struct {
 | |
| -      Type bool_type_ : 8;
 | |
| -      bool bool_value_;
 | |
| -    };
 | |
| -    struct {
 | |
| -      Type int_type_ : 8;
 | |
| -      int int_value_;
 | |
| -    };
 | |
| -    struct {
 | |
| -      Type double_type_ : 8;
 | |
| -      // Subtle: On architectures that require it, the compiler will ensure
 | |
| -      // that |double_value_|'s offset is a multiple of 8 (e.g. 32-bit ARM).
 | |
| -      // See technical note above to understand why it is important.
 | |
| -      double double_value_;
 | |
| -    };
 | |
| -    struct {
 | |
| -      Type string_type_ : 8;
 | |
| -      std::string string_value_;
 | |
| -    };
 | |
| -    struct {
 | |
| -      Type binary_type_ : 8;
 | |
| -      BlobStorage binary_value_;
 | |
| -    };
 | |
| -    struct {
 | |
| -      Type dict_type_ : 8;
 | |
| -      DictStorage dict_;
 | |
| -    };
 | |
| -    struct {
 | |
| -      Type list_type_ : 8;
 | |
| -      ListStorage list_;
 | |
| -    };
 | |
| +    bool bool_value_;
 | |
| +    int int_value_;
 | |
| +    double double_value_;
 | |
| +    std::string string_value_;
 | |
| +    BlobStorage binary_value_;
 | |
| +    DictStorage dict_;
 | |
| +    ListStorage list_;
 | |
|    };
 | |
|  
 | |
|   private:
 | |
| diff --git base/values_unittest.cc base/values_unittest.cc
 | |
| index 2907dc066843..3a65e2d6ff2b 100644
 | |
| --- base/values_unittest.cc
 | |
| +++ base/values_unittest.cc
 | |
| @@ -6,7 +6,6 @@
 | |
|  
 | |
|  #include <stddef.h>
 | |
|  
 | |
| -#include <algorithm>
 | |
|  #include <functional>
 | |
|  #include <limits>
 | |
|  #include <memory>
 | |
| @@ -16,7 +15,6 @@
 | |
|  #include <vector>
 | |
|  
 | |
|  #include "base/containers/adapters.h"
 | |
| -#include "base/logging.h"
 | |
|  #include "base/strings/string16.h"
 | |
|  #include "base/strings/string_piece.h"
 | |
|  #include "base/strings/utf_string_conversions.h"
 | |
| @@ -26,46 +24,6 @@
 | |
|  
 | |
|  namespace base {
 | |
|  
 | |
| -// Test is currently incorrect on Windows x86.
 | |
| -#if !defined(OS_WIN) || !defined(ARCH_CPU_X86)
 | |
| -TEST(ValuesTest, SizeOfValue) {
 | |
| -  // Ensure that base::Value is as small as possible, i.e. that there is
 | |
| -  // no wasted space after the inner value due to alignment constraints.
 | |
| -  // Distinguish between the 'header' that includes |type_| and and the inner
 | |
| -  // value that follows it, which can be a bool, int, double, string, blob, list
 | |
| -  // or dict.
 | |
| -#define INNER_TYPES_LIST(X)            \
 | |
| -  X(bool, bool_value_)                 \
 | |
| -  X(int, int_value_)                   \
 | |
| -  X(double, double_value_)             \
 | |
| -  X(std::string, string_value_)        \
 | |
| -  X(Value::BlobStorage, binary_value_) \
 | |
| -  X(Value::ListStorage, list_)         \
 | |
| -  X(Value::DictStorage, dict_)
 | |
| -
 | |
| -#define INNER_STRUCT_LIMIT(type, value) offsetof(Value, value) + sizeof(type),
 | |
| -
 | |
| -  // Return the maximum size in bytes of each inner struct inside base::Value
 | |
| -  size_t max_inner_struct_limit =
 | |
| -      std::max({INNER_TYPES_LIST(INNER_STRUCT_LIMIT)});
 | |
| -
 | |
| -  // Ensure that base::Value is not larger than necessary, i.e. that there is
 | |
| -  // no un-necessary padding afte the structs due to alignment constraints of
 | |
| -  // one of the inner fields.
 | |
| -  EXPECT_EQ(max_inner_struct_limit, sizeof(Value));
 | |
| -  if (max_inner_struct_limit != sizeof(Value)) {
 | |
| -    // The following are useful to understand what's wrong when the EXPECT_EQ()
 | |
| -    // above actually fails.
 | |
| -#define PRINT_INNER_FIELD_INFO(x, y) \
 | |
| -  LOG(INFO) << #y " type=" #x " 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_limit=" << max_inner_struct_limit;
 | |
| -  }
 | |
| -}
 | |
| -#endif
 | |
| -
 | |
|  TEST(ValuesTest, TestNothrow) {
 | |
|    static_assert(std::is_nothrow_move_constructible<Value>::value,
 | |
|                  "IsNothrowMoveConstructible");
 |