Owned class-properties using std::unique_ptr and optionally cascading properties.

This is intended to support the work in this CL: cl/2982960

Bug: None
Change-Id: Ifc0ea84b5d32cb9d55d0fdbbb029290567d38072
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3016342
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#901150}

ui/aura/window.cc

@@ -871,7 +871,8 @@ void Window::SetNativeWindowProperty(const char* key, void* value) {
 }
 
 void* Window::GetNativeWindowProperty(const char* key) const {
-  return reinterpret_cast<void*>(GetPropertyInternal(key, 0));
+  return reinterpret_cast<void*>(GetPropertyInternal(key, 0,
+                                                     /*search_parent=*/false));
 }
 
 void Window::OnDeviceScaleFactorChanged(float old_device_scale_factor,

ui/base/class_property.cc

@@ -7,6 +7,8 @@
 #include <algorithm>
 #include <utility>
 
+#include "base/notreached.h"
+
 namespace ui {
 
 PropertyHandler::PropertyHandler() = default;
@@ -28,7 +30,7 @@ int64_t PropertyHandler::SetPropertyInternal(const void* key,
                                              PropertyDeallocator deallocator,
                                              int64_t value,
                                              int64_t default_value) {
-  int64_t old = GetPropertyInternal(key, default_value);
+  int64_t old = GetPropertyInternal(key, default_value, false);
   if (value == default_value) {
     prop_map_.erase(key);
   } else {
@@ -53,12 +55,30 @@ void PropertyHandler::ClearProperties() {
   prop_map_.clear();
 }
 
+PropertyHandler* PropertyHandler::GetParentHandler() const {
+  // If you plan on using cascading properties, you must override this method
+  // to return the "parent" handler. If you want to use cascading properties in
+  // scenarios where there isn't a notion of a parent, just override this method
+  // and return null.
+  NOTREACHED();
+  return nullptr;
+}
+
 int64_t PropertyHandler::GetPropertyInternal(const void* key,
-                                             int64_t default_value) const {
+                                             int64_t default_value,
-  auto iter = prop_map_.find(key);
+                                             bool search_parent) const {
-  if (iter == prop_map_.end())
+  const PropertyHandler* handler = this;
-    return default_value;
+  while (handler) {
-  return iter->second.value;
+    auto iter = handler->prop_map_.find(key);
+    if (iter == handler->prop_map_.end()) {
+      if (!search_parent)
+        break;
+      handler = handler->GetParentHandler();
+      continue;
+    }
+    return iter->second.value;
+  }
+  return default_value;
 }
 
 std::set<const void*> PropertyHandler::GetAllPropertyKeys() const {

ui/base/class_property.h

@@ -45,6 +45,18 @@
 // If the properties are used outside the file where they are defined
 // their accessor methods should also be declared in a suitable header
 // using DECLARE_EXPORTED_UI_CLASS_PROPERTY_TYPE(FOO_EXPORT, MyType)
+//
+// Cascading properties:
+//
+// Use the DEFINE_CASCADING_XXX macros to create a class property type that
+// will automatically search up an instance hierarchy for the first defined
+// property. This only affects the GetProperty() call. SetProperty() will
+// still explicitly set the value on the given instance. This is useful for
+// hierarchies of instances which a single set property can effect a whole sub-
+// tree of instances.
+//
+// In order to use this feature, you must override GetParentHandler() on the
+// class that inherits PropertyHandler.
 
 namespace ui {
 
@@ -55,6 +67,7 @@ template<typename T>
 struct ClassProperty {
   T default_value;
   const char* name;
+  bool cascading = false;
   PropertyDeallocator deallocator;
 };
 
@@ -79,7 +92,9 @@ class COMPONENT_EXPORT(UI_BASE) PropertyHandler {
   // Sets the |value| of the given class |property|. Setting to the default
   // value (e.g., NULL) removes the property. The lifetime of objects set as
   // values of unowned properties is managed by the caller (owned properties are
-  // freed when they are overwritten or cleared).
+  // freed when they are overwritten or cleared).  NOTE: This should NOT be
+  // for passing a raw pointer for owned properties. Prefer the std::unique_ptr
+  // version below.
   template<typename T>
   void SetProperty(const ClassProperty<T>* property, T value);
 
@@ -97,8 +112,16 @@ class COMPONENT_EXPORT(UI_BASE) PropertyHandler {
   template <typename T>
   void SetProperty(const ClassProperty<T*>* property, T&& value);
 
+  // Sets the |value| of the given class |property|, which must be an owned
+  // property and of pointer type. Use std::make_unique<> or base::WrapUnique to
+  // ensure proper ownership transfer.
+  template <typename T>
+  T* SetProperty(const ClassProperty<T*>* property, std::unique_ptr<T> value);
+
   // Returns the value of the given class |property|.  Returns the
   // property-specific default value if the property was not previously set.
+  // The return value is the raw pointer useful for accessing the value
+  // contents.
   template<typename T>
   T GetProperty(const ClassProperty<T>* property) const;
 
@@ -115,6 +138,10 @@ class COMPONENT_EXPORT(UI_BASE) PropertyHandler {
 
   virtual void AfterPropertyChange(const void* key, int64_t old_value) {}
   void ClearProperties();
+  // Override this function when inheriting this class on a class or classes
+  // in which instances are arranged in a parent-child relationship and
+  // the intent is to use cascading properties.
+  virtual PropertyHandler* GetParentHandler() const;
 
   // Called by the public {Set,Get,Clear}Property functions.
   int64_t SetPropertyInternal(const void* key,
@@ -122,7 +149,13 @@ class COMPONENT_EXPORT(UI_BASE) PropertyHandler {
                               PropertyDeallocator deallocator,
                               int64_t value,
                               int64_t default_value);
-  int64_t GetPropertyInternal(const void* key, int64_t default_value) const;
+  // |search_parent| is required here for the setters to be able to look up the
+  // current value of property only on the current instance without searching
+  // the parent handler. This value is sent with the AfterPropertyChange()
+  // notification.
+  int64_t GetPropertyInternal(const void* key,
+                              int64_t default_value,
+                              bool search_parent) const;
 
  private:
   // Value struct to keep the name and deallocator for this property.
@@ -177,11 +210,13 @@ class COMPONENT_EXPORT(UI_BASE) PropertyHelper {
       (*property->deallocator)(old);
     }
   }
-  template<typename T>
+  template <typename T>
   static T Get(const ::ui::PropertyHandler* handler,
-               const ::ui::ClassProperty<T>* property) {
+               const ::ui::ClassProperty<T>* property,
+               bool allow_cascade) {
     return ClassPropertyCaster<T>::FromInt64(handler->GetPropertyInternal(
-        property, ClassPropertyCaster<T>::ToInt64(property->default_value)));
+        property, ClassPropertyCaster<T>::ToInt64(property->default_value),
+        property->cascading && allow_cascade));
   }
   template<typename T>
   static void Clear(::ui::PropertyHandler* handler,
@@ -202,13 +237,13 @@ template <typename T>
 void PropertyHandler::SetProperty(const ClassProperty<T*>* property,
                                   const T& value) {
   // Prevent additional heap allocation if possible.
-  T* const old = GetProperty(property);
+  T* const old = subtle::PropertyHelper::Get<T*>(this, property, false);
   if (old) {
     T temp(*old);
     *old = value;
     AfterPropertyChange(property, reinterpret_cast<int64_t>(&temp));
   } else {
-    SetProperty(property, new T(value));
+    SetProperty(property, std::make_unique<T>(value));
   }
 }
 
@@ -216,16 +251,26 @@ template <typename T>
 void PropertyHandler::SetProperty(const ClassProperty<T*>* property,
                                   T&& value) {
   // Prevent additional heap allocation if possible.
-  T* const old = GetProperty(property);
+  T* const old = subtle::PropertyHelper::Get<T*>(this, property, false);
   if (old) {
     T temp(std::move(*old));
     *old = std::forward<T>(value);
     AfterPropertyChange(property, reinterpret_cast<int64_t>(&temp));
   } else {
-    SetProperty(property, new T(std::forward<T>(value)));
+    SetProperty(property, std::make_unique<T>(std::forward<T>(value)));
   }
 }
 
+template <typename T>
+T* PropertyHandler::SetProperty(const ClassProperty<T*>* property,
+                                std::unique_ptr<T> value) {
+  // This form only works for 'owned' properties.
+  DCHECK(property->deallocator);
+  T* value_ptr = value.get();
+  subtle::PropertyHelper::Set<T*>(this, property, value.release());
+  return value_ptr;
+}
+
 }  // namespace ui
 
 // Macros to declare the property getter/setter template functions.
@@ -253,7 +298,7 @@ void PropertyHandler::SetProperty(const ClassProperty<T*>* property,
   template <>                                                                \
   EXPORT T                                                                   \
   PropertyHandler::GetProperty(const ClassProperty<T>* property) const {     \
-    return subtle::PropertyHelper::Get<T>(this, property);                   \
+    return subtle::PropertyHelper::Get<T>(this, property, true);             \
   }                                                                          \
   template <>                                                                \
   EXPORT void PropertyHandler::ClearProperty(                                \
@@ -267,18 +312,36 @@ void PropertyHandler::SetProperty(const ClassProperty<T*>* property,
 
 #define DEFINE_UI_CLASS_PROPERTY_KEY(TYPE, NAME, DEFAULT)                    \
   static_assert(sizeof(TYPE) <= sizeof(int64_t), "property type too large"); \
-  const ::ui::ClassProperty<TYPE> NAME##_Value = {DEFAULT, #NAME, nullptr};  \
+  const ::ui::ClassProperty<TYPE> NAME##_Value = {DEFAULT, #NAME, false,     \
+                                                  nullptr};                  \
   const ::ui::ClassProperty<TYPE>* const NAME = &NAME##_Value;
 
-#define DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(TYPE, NAME, DEFAULT)         \
+#define DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(TYPE, NAME, DEFAULT)           \
-  namespace {                                                           \
+  namespace {                                                             \
-  void Deallocator##NAME(int64_t p) {                                   \
+  void Deallocator##NAME(int64_t p) {                                     \
-    enum { type_must_be_complete = sizeof(TYPE) };                      \
+    enum { type_must_be_complete = sizeof(TYPE) };                        \
-    delete ::ui::ClassPropertyCaster<TYPE*>::FromInt64(p);              \
+    delete ::ui::ClassPropertyCaster<TYPE*>::FromInt64(p);                \
-  }                                                                     \
+  }                                                                       \
-  const ::ui::ClassProperty<TYPE*> NAME##_Value = {DEFAULT, #NAME,      \
+  const ::ui::ClassProperty<TYPE*> NAME##_Value = {DEFAULT, #NAME, false, \
-                                                   &Deallocator##NAME}; \
+                                                   &Deallocator##NAME};   \
-  } /* namespace */                                                     \
+  } /* namespace */                                                       \
+  const ::ui::ClassProperty<TYPE*>* const NAME = &NAME##_Value;
+
+#define DEFINE_CASCADING_UI_CLASS_PROPERTY_KEY(TYPE, NAME, DEFAULT)          \
+  static_assert(sizeof(TYPE) <= sizeof(int64_t), "property type too large"); \
+  const ::ui::ClassProperty<TYPE> NAME##_Value = {DEFAULT, #NAME, true,      \
+                                                  nullptr};                  \
+  const ::ui::ClassProperty<TYPE>* const NAME = &NAME##_Value;
+
+#define DEFINE_CASCADING_OWNED_UI_CLASS_PROPERTY_KEY(TYPE, NAME, DEFAULT) \
+  namespace {                                                             \
+  void Deallocator##NAME(int64_t p) {                                     \
+    enum { type_must_be_complete = sizeof(TYPE) };                        \
+    delete ::ui::ClassPropertyCaster<TYPE*>::FromInt64(p);                \
+  }                                                                       \
+  const ::ui::ClassProperty<TYPE*> NAME##_Value = {DEFAULT, #NAME, true,  \
+                                                   &Deallocator##NAME};   \
+  } /* namespace */                                                       \
   const ::ui::ClassProperty<TYPE*>* const NAME = &NAME##_Value;
 
 #endif  // UI_BASE_CLASS_PROPERTY_H_

ui/base/class_property_unittest.cc

@@ -22,7 +22,9 @@ namespace {
 
 class TestProperty {
  public:
-  TestProperty() {}
+  TestProperty() = default;
+  TestProperty(const TestProperty&) = delete;
+  TestProperty& operator=(const TestProperty&) = delete;
   ~TestProperty() {
     last_deleted_ = this;
   }
@@ -30,7 +32,18 @@ class TestProperty {
 
  private:
   static void* last_deleted_;
-  DISALLOW_COPY_AND_ASSIGN(TestProperty);
+};
+
+class TestCascadingProperty {
+ public:
+  explicit TestCascadingProperty(ui::PropertyHandler* handler)
+      : handler_(handler) {}
+  ~TestCascadingProperty() = default;
+
+  ui::PropertyHandler* handler() { return handler_; }
+
+ private:
+  ui::PropertyHandler* handler_;
 };
 
 void* TestProperty::last_deleted_ = nullptr;
@@ -66,11 +79,15 @@ DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(TestProperty, kOwnedKey, nullptr)
 DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(AssignableTestProperty,
                                    kAssignableKey,
                                    nullptr)
+DEFINE_CASCADING_OWNED_UI_CLASS_PROPERTY_KEY(TestCascadingProperty,
+                                             kCascadingOwnedKey,
+                                             nullptr)
 
 }  // namespace
 
 DEFINE_UI_CLASS_PROPERTY_TYPE(TestProperty*)
 DEFINE_UI_CLASS_PROPERTY_TYPE(AssignableTestProperty*)
+DEFINE_UI_CLASS_PROPERTY_TYPE(TestCascadingProperty*)
 
 namespace ui {
 namespace test {
@@ -79,15 +96,20 @@ namespace {
 
 class TestPropertyHandler : public PropertyHandler {
  public:
+  TestPropertyHandler() = default;
+  explicit TestPropertyHandler(TestPropertyHandler* parent) : parent_(parent) {}
+  ~TestPropertyHandler() override = default;
   int num_events() const { return num_events_; }
 
  protected:
   void AfterPropertyChange(const void* key, int64_t old_value) override {
     ++num_events_;
   }
+  PropertyHandler* GetParentHandler() const override { return parent_; }
 
  private:
   int num_events_ = 0;
+  TestPropertyHandler* parent_ = nullptr;
 };
 
 const int kDefaultIntValue = -2;
@@ -133,24 +155,23 @@ TEST(PropertyTest, OwnedProperty) {
   {
     PropertyHandler h;
 
-    EXPECT_EQ(NULL, h.GetProperty(kOwnedKey));
+    EXPECT_EQ(nullptr, h.GetProperty(kOwnedKey));
     void* last_deleted = TestProperty::last_deleted();
-    TestProperty* p1 = new TestProperty();
+    TestProperty* p1 =
-    h.SetProperty(kOwnedKey, p1);
+        h.SetProperty(kOwnedKey, std::make_unique<TestProperty>());
     EXPECT_EQ(p1, h.GetProperty(kOwnedKey));
     EXPECT_EQ(last_deleted, TestProperty::last_deleted());
 
-    TestProperty* p2 = new TestProperty();
+    TestProperty* p2 =
-    h.SetProperty(kOwnedKey, p2);
+        h.SetProperty(kOwnedKey, std::make_unique<TestProperty>());
     EXPECT_EQ(p2, h.GetProperty(kOwnedKey));
     EXPECT_EQ(p1, TestProperty::last_deleted());
 
     h.ClearProperty(kOwnedKey);
-    EXPECT_EQ(NULL, h.GetProperty(kOwnedKey));
+    EXPECT_EQ(nullptr, h.GetProperty(kOwnedKey));
     EXPECT_EQ(p2, TestProperty::last_deleted());
 
-    p3 = new TestProperty();
+    p3 = h.SetProperty(kOwnedKey, std::make_unique<TestProperty>());
-    h.SetProperty(kOwnedKey, p3);
     EXPECT_EQ(p3, h.GetProperty(kOwnedKey));
     EXPECT_EQ(p2, TestProperty::last_deleted());
   }
@@ -162,8 +183,8 @@ TEST(PropertyTest, AcquireAllPropertiesFrom) {
   std::unique_ptr<PropertyHandler> src = std::make_unique<PropertyHandler>();
   void* last_deleted = TestProperty::last_deleted();
   EXPECT_FALSE(src->GetProperty(kOwnedKey));
-  TestProperty* p1 = new TestProperty();
+  TestProperty* p1 =
-  src->SetProperty(kOwnedKey, p1);
+      src->SetProperty(kOwnedKey, std::make_unique<TestProperty>());
   src->SetProperty(kIntKey, INT_MAX);
 
   // dest will take ownership of the owned property. Existing properties with
@@ -296,5 +317,21 @@ TEST(PropertyTest, PropertyChangedEvent) {
   EXPECT_EQ(6, h.num_events());
 }
 
+TEST(PropertyTest, CascadingProperties) {
+  TestPropertyHandler h;
+  TestPropertyHandler h2(&h);
+
+  // Set the property on the parent handler.
+  h.SetProperty(kCascadingOwnedKey,
+                std::make_unique<TestCascadingProperty>(&h));
+
+  // Get the property value from the child handler.
+  auto* value = h2.GetProperty(kCascadingOwnedKey);
+
+  EXPECT_TRUE(value);
+  // The property value should have a reference to |h|.
+  EXPECT_EQ(&h, value->handler());
+}
+
 } // namespace test
 } // namespace ui

ui/platform_window/stub/BUILD.gn

@@ -7,6 +7,7 @@ component("stub") {
 
   deps = [
     "//base",
+    "//ui/base",
     "//ui/gfx",
     "//ui/gfx/geometry",
     "//ui/platform_window",