Improve ObservedProperty to better handle movable types

Also changes NotifiesOnlyOnChangesWithPreviousValue to pass
the previous value by const-ref.

Bug: 999594
Change-Id: I63ce2cdb1d085ac68a7abb2a7b8211be0a353c56
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1787727
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697810}

chrome/browser/performance_manager/decorators/freeze_origin_trial_policy_aggregator.cc

@@ -129,7 +129,7 @@ void FreezeOriginTrialPolicyAggregator::OnIsCurrentChanged(
 
 void FreezeOriginTrialPolicyAggregator::OnOriginTrialFreezePolicyChanged(
     const FrameNode* frame_node,
-    InterventionPolicy previous_value) {
+    const InterventionPolicy& previous_value) {
   if (frame_node->IsCurrent()) {
     auto* page_node = PageNodeImpl::FromNode(frame_node->GetPageNode());
     Data* data = Data::Get(page_node);

chrome/browser/performance_manager/decorators/freeze_origin_trial_policy_aggregator.h

@@ -27,7 +27,7 @@ class FreezeOriginTrialPolicyAggregator : public FrameNode::ObserverDefaultImpl,
   void OnIsCurrentChanged(const FrameNode* frame_node) override;
   void OnOriginTrialFreezePolicyChanged(
       const FrameNode* frame_node,
-      InterventionPolicy previous_value) override;
+      const InterventionPolicy& previous_value) override;
 
   // GraphOwned implementation:
   void OnPassedToGraph(Graph* graph) override;

chrome/browser/performance_manager/graph/frame_node_impl_unittest.cc

@@ -108,7 +108,7 @@ class LenientMockObserver : public FrameNodeImpl::Observer {
   MOCK_METHOD1(OnFrameLifecycleStateChanged, void(const FrameNode*));
   MOCK_METHOD2(OnOriginTrialFreezePolicyChanged,
                void(const FrameNode*,
-                    resource_coordinator::mojom::InterventionPolicy));
+                    const resource_coordinator::mojom::InterventionPolicy&));
   MOCK_METHOD1(OnURLChanged, void(const FrameNode*));
   MOCK_METHOD1(OnIsAdFrameChanged, void(const FrameNode*));
   MOCK_METHOD1(OnNonPersistentNotificationCreated, void(const FrameNode*));

chrome/browser/performance_manager/graph/properties.h

@@ -29,15 +29,18 @@ class ObservedPropertyImpl {
             void (ObserverType::*NotifyFunctionPtr)(const NodeType*)>
   class NotifiesAlways {
    public:
-    NotifiesAlways() {}
-    explicit NotifiesAlways(PropertyType initial_value)
-        : value_(initial_value) {}
+    NotifiesAlways() = default;
 
-    ~NotifiesAlways() {}
+    template <typename U = PropertyType>
+    explicit NotifiesAlways(U&& initial_value)
+        : value_(std::forward<U>(initial_value)) {}
+
+    ~NotifiesAlways() = default;
 
     // Sets the property and sends a notification.
-    void SetAndNotify(NodeImplType* node, PropertyType value) {
-      value_ = std::forward<PropertyType>(value);
+    template <typename U = PropertyType>
+    void SetAndNotify(NodeImplType* node, U&& value) {
+      value_ = std::forward<U>(value);
       for (auto* observer : node->GetObservers())
         ((observer)->*(NotifyFunctionPtr))(node);
     }
@@ -56,18 +59,21 @@ class ObservedPropertyImpl {
             void (ObserverType::*NotifyFunctionPtr)(const NodeType*)>
   class NotifiesOnlyOnChanges {
    public:
-    NotifiesOnlyOnChanges() {}
-    explicit NotifiesOnlyOnChanges(PropertyType initial_value)
-        : value_(initial_value) {}
+    NotifiesOnlyOnChanges() = default;
+
+    template <typename U = PropertyType>
+    explicit NotifiesOnlyOnChanges(U&& initial_value)
+        : value_(std::forward<U>(initial_value)) {}
 
-    ~NotifiesOnlyOnChanges() {}
+    ~NotifiesOnlyOnChanges() = default;
 
     // Sets the property and sends a notification if needed. Returns true if a
     // notification was sent, false otherwise.
-    bool SetAndMaybeNotify(NodeImplType* node, PropertyType value) {
+    template <typename U = PropertyType>
+    bool SetAndMaybeNotify(NodeImplType* node, U&& value) {
       if (value_ == value)
         return false;
-      value_ = std::forward<PropertyType>(value);
+      value_ = std::forward<U>(value);
       for (auto* observer : node->GetObservers())
         ((observer)->*(NotifyFunctionPtr))(node);
       return true;
@@ -83,23 +89,26 @@ class ObservedPropertyImpl {
   // notifying observers.
   template <typename PropertyType,
             void (ObserverType::*NotifyFunctionPtr)(
-                const NodeType*,
-                PropertyType previous_value)>
+                const NodeType* node,
+                const PropertyType& previous_value)>
   class NotifiesOnlyOnChangesWithPreviousValue {
    public:
-    NotifiesOnlyOnChangesWithPreviousValue() {}
-    explicit NotifiesOnlyOnChangesWithPreviousValue(PropertyType initial_value)
-        : value_(initial_value) {}
+    NotifiesOnlyOnChangesWithPreviousValue() = default;
+
+    template <typename U = PropertyType>
+    explicit NotifiesOnlyOnChangesWithPreviousValue(U&& initial_value)
+        : value_(std::forward<U>(initial_value)) {}
 
-    ~NotifiesOnlyOnChangesWithPreviousValue() {}
+    ~NotifiesOnlyOnChangesWithPreviousValue() = default;
 
     // Sets the property and sends a notification if needed. Returns true if a
     // notification was sent, false otherwise.
-    bool SetAndMaybeNotify(NodeImplType* node, PropertyType value) {
+    template <typename U = PropertyType>
+    bool SetAndMaybeNotify(NodeImplType* node, U&& value) {
       if (value_ == value)
         return false;
-      PropertyType previous_value = value_;
-      value_ = std::forward<PropertyType>(value);
+      PropertyType previous_value = std::move(value_);
+      value_ = std::forward<U>(value);
       for (auto* observer : node->GetObservers())
         ((observer)->*(NotifyFunctionPtr))(node, previous_value);
       return true;

chrome/browser/performance_manager/graph/properties_unittest.cc

@@ -22,24 +22,17 @@ class DummyObserver {
   MOCK_METHOD1(NotifyAlwaysConst, void(const DummyNode*));
   MOCK_METHOD1(NotifyOnlyOnChangesConst, void(const DummyNode*));
   MOCK_METHOD2(NotifyOnlyOnChangesWithPreviousValueConst,
-               void(const DummyNode*, bool));
+               void(const DummyNode*, const bool&));
 };
 
 class DummyNode {
  public:
-  DummyNode() {}
-  ~DummyNode() {}
+  DummyNode() = default;
+  ~DummyNode() = default;
 
-  void AddObserver(DummyObserver* observer) {
-    observers_.AddObserver(observer);
-    new_observers_.push_back(observer);
-  }
-
-  base::ObserverList<DummyObserver>::Unchecked& observers() {
-    return observers_;
-  }
+  void AddObserver(DummyObserver* observer) { observers_.push_back(observer); }
 
-  const std::vector<DummyObserver*>& GetObservers() { return new_observers_; }
+  const std::vector<DummyObserver*>& GetObservers() { return observers_; }
 
   bool observed_always() const { return observed_always_.value(); }
   bool observed_only_on_changes() const {
@@ -74,8 +67,7 @@ class DummyNode {
       &DummyObserver::NotifyOnlyOnChangesWithPreviousValueConst>
       observed_only_on_changes_with_previous_value_{false};
 
-  base::ObserverList<DummyObserver>::Unchecked observers_;
-  std::vector<DummyObserver*> new_observers_;
+  std::vector<DummyObserver*> observers_;
 };
 
 class GraphPropertiesTest : public ::testing::Test {
@@ -135,7 +127,7 @@ TEST_F(GraphPropertiesTest, ObservedOnlyOnChangesProperty) {
 TEST_F(GraphPropertiesTest, ObservedOnlyOnChangesWithPreviousValueProperty) {
   EXPECT_FALSE(node_.observed_only_on_changes_with_previous_value());
 
-  EXPECT_FALSE(node_.SetObservedOnlyOnChanges(false));
+  EXPECT_FALSE(node_.SetObservedOnlyOnChangesWithPreviousValue(false));
   EXPECT_EQ(false, node_.observed_only_on_changes_with_previous_value());
 
   EXPECT_CALL(observer_,

chrome/browser/performance_manager/public/graph/frame_node.h

@@ -176,7 +176,7 @@ class FrameNodeObserver {
   // Invoked when the OriginTrialFreezePolicy changes.
   virtual void OnOriginTrialFreezePolicyChanged(
       const FrameNode* frame_node,
-      InterventionPolicy previous_value) = 0;
+      const InterventionPolicy& previous_value) = 0;
 
   // Invoked when the URL property changes.
   virtual void OnURLChanged(const FrameNode* frame_node) = 0;
@@ -213,7 +213,7 @@ class FrameNode::ObserverDefaultImpl : public FrameNodeObserver {
   void OnFrameLifecycleStateChanged(const FrameNode* frame_node) override {}
   void OnOriginTrialFreezePolicyChanged(
       const FrameNode* frame_node,
-      InterventionPolicy previous_value) override {}
+      const InterventionPolicy& previous_value) override {}
   void OnURLChanged(const FrameNode* frame_node) override {}
   void OnIsAdFrameChanged(const FrameNode* frame_node) override {}
   void OnNonPersistentNotificationCreated(

chrome/browser/ui/webui/discards/graph_dump_impl.h

@@ -62,7 +62,7 @@ class DiscardsGraphDumpImpl : public discards::mojom::GraphDump,
   // Ignored.
   void OnOriginTrialFreezePolicyChanged(
       const performance_manager::FrameNode* frame_node,
-      InterventionPolicy previous_value) override {}
+      const InterventionPolicy& previous_value) override {}
   void OnURLChanged(const performance_manager::FrameNode* frame_node) override;
   // Ignored.
   void OnIsAdFrameChanged(