[PM] Add an observer mechanism for the PageLiveStateData decorator.

Bug: 1144025
Change-Id: I880dde77edadb8fc7ccad8a7040f6c97d141db37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2586107
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835975}

components/performance_manager/decorators/page_live_state_decorator.cc

@@ -4,6 +4,7 @@
 
 #include "components/performance_manager/public/decorators/page_live_state_decorator.h"
 
+#include "base/sequence_checker.h"
 #include "components/performance_manager/decorators/decorators_utils.h"
 #include "components/performance_manager/freezing/freezing_vote_aggregator.h"
 #include "components/performance_manager/graph/node_attached_data_impl.h"
@@ -71,31 +72,69 @@ class PageLiveStateDataImpl
   }
 
   void set_is_connected_to_usb_device(bool is_connected_to_usb_device) {
+    if (is_connected_to_usb_device_ == is_connected_to_usb_device)
+      return;
     is_connected_to_usb_device_ = is_connected_to_usb_device;
+    for (auto& obs : observers_)
+      obs.OnIsConnectedToUSBDeviceChanged(page_node_);
   }
   void set_is_connected_to_bluetooth_device(
       bool is_connected_to_bluetooth_device) {
+    if (is_connected_to_bluetooth_device_ == is_connected_to_bluetooth_device)
+      return;
     is_connected_to_bluetooth_device_ = is_connected_to_bluetooth_device;
+    for (auto& obs : observers_)
+      obs.OnIsConnectedToBluetoothDeviceChanged(page_node_);
   }
   void set_is_capturing_video(bool is_capturing_video) {
+    if (is_capturing_video_ == is_capturing_video)
+      return;
     is_capturing_video_ = is_capturing_video;
+    for (auto& obs : observers_)
+      obs.OnIsCapturingVideoChanged(page_node_);
   }
   void set_is_capturing_audio(bool is_capturing_audio) {
+    if (is_capturing_audio_ == is_capturing_audio)
+      return;
     is_capturing_audio_ = is_capturing_audio;
+    for (auto& obs : observers_)
+      obs.OnIsCapturingAudioChanged(page_node_);
   }
   void set_is_being_mirrored(bool is_being_mirrored) {
+    if (is_being_mirrored_ == is_being_mirrored)
+      return;
     is_being_mirrored_ = is_being_mirrored;
+    for (auto& obs : observers_)
+      obs.OnIsBeingMirroredChanged(page_node_);
   }
   void set_is_capturing_window(bool is_capturing_window) {
+    if (is_capturing_window_ == is_capturing_window)
+      return;
     is_capturing_window_ = is_capturing_window;
+    for (auto& obs : observers_)
+      obs.OnIsCapturingWindowChanged(page_node_);
   }
   void set_is_capturing_display(bool is_capturing_display) {
+    if (is_capturing_display_ == is_capturing_display)
+      return;
     is_capturing_display_ = is_capturing_display;
+    for (auto& obs : observers_)
+      obs.OnIsCapturingDisplayChanged(page_node_);
   }
   void set_is_auto_discardable(bool is_auto_discardable) {
+    if (is_auto_discardable_ == is_auto_discardable)
+      return;
     is_auto_discardable_ = is_auto_discardable;
+    for (auto& obs : observers_)
+      obs.OnIsAutoDiscardableChanged(page_node_);
+  }
+  void set_was_discarded(bool was_discarded) {
+    if (was_discarded_ == was_discarded)
+      return;
+    was_discarded_ = was_discarded;
+    for (auto& obs : observers_)
+      obs.OnWasDiscardedChanged(page_node_);
   }
-  void set_was_discarded(bool was_discarded) { was_discarded_ = was_discarded; }
 
  private:
   // Make the impl our friend so it can access the constructor and any
@@ -103,7 +142,8 @@ class PageLiveStateDataImpl
   friend class ::performance_manager::NodeAttachedDataImpl<
       PageLiveStateDataImpl>;
 
-  explicit PageLiveStateDataImpl(const PageNodeImpl* page_node) {}
+  explicit PageLiveStateDataImpl(const PageNodeImpl* page_node)
+      : page_node_(page_node) {}
 
   bool is_connected_to_usb_device_ = false;
   bool is_connected_to_bluetooth_device_ = false;
@@ -114,6 +154,8 @@ class PageLiveStateDataImpl
   bool is_capturing_display_ = false;
   bool is_auto_discardable_ = true;
   bool was_discarded_ = false;
+
+  const PageNode* const page_node_;
 };
 
 const char kDescriberName[] = "PageLiveStateDecorator";
@@ -202,10 +244,12 @@ void PageLiveStateDecorator::SetWasDiscarded(content::WebContents* contents,
 void PageLiveStateDecorator::OnPassedToGraph(Graph* graph) {
   graph->GetNodeDataDescriberRegistry()->RegisterDescriber(this,
                                                            kDescriberName);
+  graph->RegisterObject(this);
 }
 
 void PageLiveStateDecorator::OnTakenFromGraph(Graph* graph) {
   graph->GetNodeDataDescriberRegistry()->UnregisterDescriber(this);
+  graph->UnregisterObject(this);
 }
 
 base::Value PageLiveStateDecorator::DescribePageNodeData(
@@ -232,6 +276,18 @@ base::Value PageLiveStateDecorator::DescribePageNodeData(
 PageLiveStateDecorator::Data::Data() = default;
 PageLiveStateDecorator::Data::~Data() = default;
 
+void PageLiveStateDecorator::Data::AddObserver(
+    PageLiveStateObserver* observer) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  observers_.AddObserver(observer);
+}
+
+void PageLiveStateDecorator::Data::RemoveObserver(
+    PageLiveStateObserver* observer) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  observers_.RemoveObserver(observer);
+}
+
 const PageLiveStateDecorator::Data* PageLiveStateDecorator::Data::FromPageNode(
     const PageNode* page_node) {
   return PageLiveStateDataImpl::Get(PageNodeImpl::FromNode(page_node));
@@ -242,4 +298,7 @@ PageLiveStateDecorator::Data::GetOrCreateForTesting(PageNode* page_node) {
   return PageLiveStateDataImpl::GetOrCreate(PageNodeImpl::FromNode(page_node));
 }
 
+PageLiveStateObserver::PageLiveStateObserver() = default;
+PageLiveStateObserver::~PageLiveStateObserver() = default;
+
 }  // namespace performance_manager

components/performance_manager/decorators/page_live_state_decorator_unittest.cc

@@ -4,6 +4,9 @@
 
 #include "components/performance_manager/public/decorators/page_live_state_decorator.h"
 
+#include "base/bind.h"
+#include "base/callback_forward.h"
+#include "base/run_loop.h"
 #include "components/performance_manager/test_support/decorators_utils.h"
 #include "components/performance_manager/test_support/performance_manager_test_harness.h"
 #include "content/public/browser/web_contents.h"
@@ -11,6 +14,79 @@
 
 namespace performance_manager {
 
+namespace {
+
+// A test version of a PageLiveStateObserver that records the latest function
+// that has been called. Gmock isn't used here as instances of this class will
+// be used on a different sequence than the main one and that this add a lot of
+// extra complexity.
+class TestPageLiveStateObserver : public PageLiveStateObserver {
+ public:
+  TestPageLiveStateObserver() = default;
+  ~TestPageLiveStateObserver() override = default;
+  TestPageLiveStateObserver(const TestPageLiveStateObserver& other) = delete;
+  TestPageLiveStateObserver& operator=(const TestPageLiveStateObserver&) =
+      delete;
+
+  enum class ObserverFunction {
+    kNone,
+    kOnIsConnectedToUSBDeviceChanged,
+    kOnIsConnectedToBluetoothDeviceChanged,
+    kOnIsCapturingVideoChanged,
+    kOnIsCapturingAudioChanged,
+    kOnIsBeingMirroredChanged,
+    kOnIsCapturingWindowChanged,
+    kOnIsCapturingDisplayChanged,
+    kOnIsAutoDiscardableChanged,
+    kOnWasDiscardedChanged,
+  };
+
+  void OnIsConnectedToUSBDeviceChanged(const PageNode* page_node) override {
+    latest_function_called_ =
+        ObserverFunction::kOnIsConnectedToUSBDeviceChanged;
+    page_node_passed_ = page_node;
+  }
+  void OnIsConnectedToBluetoothDeviceChanged(
+      const PageNode* page_node) override {
+    latest_function_called_ =
+        ObserverFunction::kOnIsConnectedToBluetoothDeviceChanged;
+    page_node_passed_ = page_node;
+  }
+  void OnIsCapturingVideoChanged(const PageNode* page_node) override {
+    latest_function_called_ = ObserverFunction::kOnIsCapturingVideoChanged;
+    page_node_passed_ = page_node;
+  }
+  void OnIsCapturingAudioChanged(const PageNode* page_node) override {
+    latest_function_called_ = ObserverFunction::kOnIsCapturingAudioChanged;
+    page_node_passed_ = page_node;
+  }
+  void OnIsBeingMirroredChanged(const PageNode* page_node) override {
+    latest_function_called_ = ObserverFunction::kOnIsBeingMirroredChanged;
+    page_node_passed_ = page_node;
+  }
+  void OnIsCapturingWindowChanged(const PageNode* page_node) override {
+    latest_function_called_ = ObserverFunction::kOnIsCapturingWindowChanged;
+    page_node_passed_ = page_node;
+  }
+  void OnIsCapturingDisplayChanged(const PageNode* page_node) override {
+    latest_function_called_ = ObserverFunction::kOnIsCapturingDisplayChanged;
+    page_node_passed_ = page_node;
+  }
+  void OnIsAutoDiscardableChanged(const PageNode* page_node) override {
+    latest_function_called_ = ObserverFunction::kOnIsAutoDiscardableChanged;
+    page_node_passed_ = page_node;
+  }
+  void OnWasDiscardedChanged(const PageNode* page_node) override {
+    latest_function_called_ = ObserverFunction::kOnWasDiscardedChanged;
+    page_node_passed_ = page_node;
+  }
+
+  ObserverFunction latest_function_called_ = ObserverFunction::kNone;
+  const PageNode* page_node_passed_ = nullptr;
+};
+
+}  // namespace
+
 class PageLiveStateDecoratorTest : public PerformanceManagerTestHarness {
  protected:
   PageLiveStateDecoratorTest() = default;
@@ -22,18 +98,84 @@ class PageLiveStateDecoratorTest : public PerformanceManagerTestHarness {
   void SetUp() override {
     PerformanceManagerTestHarness::SetUp();
     SetContents(CreateTestWebContents());
+    observer_ = std::make_unique<TestPageLiveStateObserver>();
+
+    base::RunLoop run_loop;
+    auto quit_closure = run_loop.QuitClosure();
+    PerformanceManager::CallOnGraph(
+        FROM_HERE,
+        base::BindOnce(
+            [](base::WeakPtr<PageNode> page_node,
+               TestPageLiveStateObserver* observer,
+               base::OnceClosure quit_closure) {
+              EXPECT_TRUE(page_node);
+              PageLiveStateDecorator::Data::GetOrCreateForTesting(
+                  page_node.get())
+                  ->AddObserver(observer);
+              std::move(quit_closure).Run();
+            },
+            PerformanceManager::GetPageNodeForWebContents(web_contents()),
+            observer_.get(), std::move(quit_closure)));
+    run_loop.Run();
   }
 
   void TearDown() override {
+    base::RunLoop run_loop;
+    auto quit_closure = run_loop.QuitClosure();
+    PerformanceManager::CallOnGraph(
+        FROM_HERE,
+        base::BindOnce(
+            [](base::WeakPtr<PageNode> page_node,
+               TestPageLiveStateObserver* observer,
+               base::OnceClosure quit_closure) {
+              EXPECT_TRUE(page_node);
+              PageLiveStateDecorator::Data::GetOrCreateForTesting(
+                  page_node.get())
+                  ->RemoveObserver(observer);
+              std::move(quit_closure).Run();
+            },
+            PerformanceManager::GetPageNodeForWebContents(web_contents()),
+            observer_.get(), std::move(quit_closure)));
+    run_loop.Run();
+
+    PerformanceManager::GetTaskRunner()->DeleteSoon(FROM_HERE,
+                                                    std::move(observer_));
     DeleteContents();
     PerformanceManagerTestHarness::TearDown();
   }
+
+  void VerifyObserverExpectationOnPMSequence(
+      TestPageLiveStateObserver::ObserverFunction expected_call) {
+    base::RunLoop run_loop;
+    auto quit_closure = run_loop.QuitClosure();
+    PerformanceManager::CallOnGraph(
+        FROM_HERE,
+        base::BindOnce(
+            [](base::WeakPtr<PageNode> page_node,
+               TestPageLiveStateObserver* observer,
+               TestPageLiveStateObserver::ObserverFunction expected_call,
+               base::OnceClosure quit_closure) {
+              EXPECT_TRUE(page_node);
+              EXPECT_EQ(expected_call, observer->latest_function_called_);
+              EXPECT_EQ(page_node.get(), observer->page_node_passed_);
+              std::move(quit_closure).Run();
+            },
+            PerformanceManager::GetPageNodeForWebContents(web_contents()),
+            observer_.get(), expected_call, std::move(quit_closure)));
+    run_loop.Run();
+  }
+
+ private:
+  std::unique_ptr<TestPageLiveStateObserver> observer_;
 };
 
 TEST_F(PageLiveStateDecoratorTest, OnIsConnectedToUSBDeviceChanged) {
   testing::EndToEndBooleanPropertyTest(
       web_contents(), &PageLiveStateDecorator::Data::IsConnectedToUSBDevice,
       &PageLiveStateDecorator::OnIsConnectedToUSBDeviceChanged);
+  VerifyObserverExpectationOnPMSequence(
+      TestPageLiveStateObserver::ObserverFunction::
+          kOnIsConnectedToUSBDeviceChanged);
 }
 
 TEST_F(PageLiveStateDecoratorTest, OnIsConnectedToBluetoothDeviceChanged) {
@@ -41,36 +183,50 @@ TEST_F(PageLiveStateDecoratorTest, OnIsConnectedToBluetoothDeviceChanged) {
       web_contents(),
       &PageLiveStateDecorator::Data::IsConnectedToBluetoothDevice,
       &PageLiveStateDecorator::OnIsConnectedToBluetoothDeviceChanged);
+  VerifyObserverExpectationOnPMSequence(
+      TestPageLiveStateObserver::ObserverFunction::
+          kOnIsConnectedToBluetoothDeviceChanged);
 }
 
 TEST_F(PageLiveStateDecoratorTest, OnIsCapturingVideoChanged) {
   testing::EndToEndBooleanPropertyTest(
       web_contents(), &PageLiveStateDecorator::Data::IsCapturingVideo,
       &PageLiveStateDecorator::OnIsCapturingVideoChanged);
+  VerifyObserverExpectationOnPMSequence(
+      TestPageLiveStateObserver::ObserverFunction::kOnIsCapturingVideoChanged);
 }
 
 TEST_F(PageLiveStateDecoratorTest, OnIsCapturingAudioChanged) {
   testing::EndToEndBooleanPropertyTest(
       web_contents(), &PageLiveStateDecorator::Data::IsCapturingAudio,
       &PageLiveStateDecorator::OnIsCapturingAudioChanged);
+  VerifyObserverExpectationOnPMSequence(
+      TestPageLiveStateObserver::ObserverFunction::kOnIsCapturingAudioChanged);
 }
 
 TEST_F(PageLiveStateDecoratorTest, OnIsBeingMirroredChanged) {
   testing::EndToEndBooleanPropertyTest(
       web_contents(), &PageLiveStateDecorator::Data::IsBeingMirrored,
       &PageLiveStateDecorator::OnIsBeingMirroredChanged);
+  VerifyObserverExpectationOnPMSequence(
+      TestPageLiveStateObserver::ObserverFunction::kOnIsBeingMirroredChanged);
 }
 
 TEST_F(PageLiveStateDecoratorTest, OnIsCapturingWindowChanged) {
   testing::EndToEndBooleanPropertyTest(
       web_contents(), &PageLiveStateDecorator::Data::IsCapturingWindow,
       &PageLiveStateDecorator::OnIsCapturingWindowChanged);
+  VerifyObserverExpectationOnPMSequence(
+      TestPageLiveStateObserver::ObserverFunction::kOnIsCapturingWindowChanged);
 }
 
 TEST_F(PageLiveStateDecoratorTest, OnIsCapturingDisplayChanged) {
   testing::EndToEndBooleanPropertyTest(
       web_contents(), &PageLiveStateDecorator::Data::IsCapturingDisplay,
       &PageLiveStateDecorator::OnIsCapturingDisplayChanged);
+  VerifyObserverExpectationOnPMSequence(
+      TestPageLiveStateObserver::ObserverFunction::
+          kOnIsCapturingDisplayChanged);
 }
 
 TEST_F(PageLiveStateDecoratorTest, SetIsAutoDiscardable) {
@@ -78,6 +234,17 @@ TEST_F(PageLiveStateDecoratorTest, SetIsAutoDiscardable) {
       web_contents(), &PageLiveStateDecorator::Data::IsAutoDiscardable,
       &PageLiveStateDecorator::SetIsAutoDiscardable,
       /*default_state=*/true);
+  VerifyObserverExpectationOnPMSequence(
+      TestPageLiveStateObserver::ObserverFunction::kOnIsAutoDiscardableChanged);
 }
 
-}  // namespace performance_manager
+TEST_F(PageLiveStateDecoratorTest, OnWasDiscardedChanged) {
+  testing::EndToEndBooleanPropertyTest(
+      web_contents(), &PageLiveStateDecorator::Data::WasDiscarded,
+      &PageLiveStateDecorator::SetWasDiscarded,
+      /*default_state=*/false);
+  VerifyObserverExpectationOnPMSequence(
+      TestPageLiveStateObserver::ObserverFunction::kOnWasDiscardedChanged);
+}
+
+}  // namespace performance_manager

components/performance_manager/graph_features_helper_unittest.cc

@@ -5,6 +5,7 @@
 #include "components/performance_manager/embedder/graph_features_helper.h"
 
 #include "build/build_config.h"
+#include "components/performance_manager/public/decorators/page_live_state_decorator.h"
 #include "components/performance_manager/public/execution_context/execution_context_registry.h"
 #include "components/performance_manager/test_support/graph_test_harness.h"
 #include "components/performance_manager/v8_memory/v8_context_tracker.h"
@@ -59,12 +60,13 @@ TEST(GraphFeaturesHelperTest, EnableDefault) {
   features.EnableDefault();
   features.ConfigureGraph(&graph);
   EXPECT_EQ(graph_owned_count, graph.GraphOwnedCountForTesting());
-  EXPECT_EQ(2u, graph.GraphRegisteredCountForTesting());
+  EXPECT_EQ(3u, graph.GraphRegisteredCountForTesting());
   EXPECT_EQ(8u, graph.NodeDataDescriberCountForTesting());
   // Ensure the GraphRegistered objects can be queried directly.
   EXPECT_TRUE(
       execution_context::ExecutionContextRegistry::GetFromGraph(&graph));
   EXPECT_TRUE(v8_memory::V8ContextTracker::GetFromGraph(&graph));
+  EXPECT_TRUE(PageLiveStateDecorator::GetFromGraph(&graph));
 
   graph.TearDown();
 }

components/performance_manager/public/decorators/page_live_state_decorator.h

@@ -5,7 +5,11 @@
 #ifndef COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_DECORATORS_PAGE_LIVE_STATE_DECORATOR_H_
 #define COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_DECORATORS_PAGE_LIVE_STATE_DECORATOR_H_
 
+#include "base/observer_list.h"
+#include "base/observer_list_types.h"
+#include "base/sequence_checker.h"
 #include "components/performance_manager/public/graph/graph.h"
+#include "components/performance_manager/public/graph/graph_registered.h"
 #include "components/performance_manager/public/graph/node_data_describer.h"
 #include "components/performance_manager/public/graph/page_node.h"
 
@@ -16,13 +20,16 @@ class WebContents;
 namespace performance_manager {
 
 class PageNode;
+class PageLiveStateObserver;
 
 // Used to record some live state information about the PageNode.
 // All the functions that take a WebContents* as a parameter should only be
 // called from the UI thread, the event will be forwarded to the corresponding
 // PageNode on the Performance Manager's sequence.
-class PageLiveStateDecorator : public GraphOwnedDefaultImpl,
-                               public NodeDataDescriberDefaultImpl {
+class PageLiveStateDecorator
+    : public GraphOwnedDefaultImpl,
+      public GraphRegisteredImpl<PageLiveStateDecorator>,
+      public NodeDataDescriberDefaultImpl {
  public:
   class Data;
 
@@ -77,6 +84,9 @@ class PageLiveStateDecorator::Data {
   Data(const Data& other) = delete;
   Data& operator=(const Data&) = delete;
 
+  void AddObserver(PageLiveStateObserver* observer);
+  void RemoveObserver(PageLiveStateObserver* observer);
+
   virtual bool IsConnectedToUSBDevice() const = 0;
   virtual bool IsConnectedToBluetoothDevice() const = 0;
   virtual bool IsCapturingVideo() const = 0;
@@ -99,6 +109,31 @@ class PageLiveStateDecorator::Data {
   virtual void SetIsCapturingDisplayForTesting(bool value) = 0;
   virtual void SetIsAutoDiscardableForTesting(bool value) = 0;
   virtual void SetWasDiscardedForTesting(bool value) = 0;
+
+ protected:
+  base::ObserverList<PageLiveStateObserver> observers_;
+
+ private:
+  SEQUENCE_CHECKER(sequence_checker_);
+};
+
+class PageLiveStateObserver : public base::CheckedObserver {
+ public:
+  PageLiveStateObserver();
+  ~PageLiveStateObserver() override;
+  PageLiveStateObserver(const PageLiveStateObserver& other) = delete;
+  PageLiveStateObserver& operator=(const PageLiveStateObserver&) = delete;
+
+  virtual void OnIsConnectedToUSBDeviceChanged(const PageNode* page_node) = 0;
+  virtual void OnIsConnectedToBluetoothDeviceChanged(
+      const PageNode* page_node) = 0;
+  virtual void OnIsCapturingVideoChanged(const PageNode* page_node) = 0;
+  virtual void OnIsCapturingAudioChanged(const PageNode* page_node) = 0;
+  virtual void OnIsBeingMirroredChanged(const PageNode* page_node) = 0;
+  virtual void OnIsCapturingWindowChanged(const PageNode* page_node) = 0;
+  virtual void OnIsCapturingDisplayChanged(const PageNode* page_node) = 0;
+  virtual void OnIsAutoDiscardableChanged(const PageNode* page_node) = 0;
+  virtual void OnWasDiscardedChanged(const PageNode* page_node) = 0;
 };
 
 }  // namespace performance_manager