Track whether a frame has had user-initiated edits in PerformanceManager

This will be useful to avoid discarding tabs that have user-entered data
even if that data was entered in non-forms elements (such as elements
with the contenteditable property set to true).

Bug: 1156388
Change-Id: Ie960fb00e3d6103c62d90b4dfc38a5b58c14f216
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4281658
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Auto-Submit: Anthony Vallée-Dubois <anthonyvd@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Francois Pierre Doray <fdoray@chromium.org>
Reviewed-by: Joe Mason <joenotcharles@google.com>
Cr-Commit-Position: refs/heads/main@{#1110710}

chrome/browser/performance_manager/decorators/page_aggregator.cc

@@ -4,7 +4,7 @@
 
 #include "chrome/browser/performance_manager/decorators/page_aggregator.h"
 
-#include <stdint.h>
+#include <cstdint>
 
 #include "components/performance_manager/graph/node_attached_data_impl.h"
 #include "components/performance_manager/graph/page_node_impl.h"
@@ -46,6 +46,10 @@ class PageAggregatorAccess {
     page_node->SetHadFormInteraction(base::PassKey<PageAggregatorAccess>(),
                                      value);
   }
+
+  static void SetPageHadUserEdits(PageNodeImpl* page_node, bool value) {
+    page_node->SetHadUserEdits(base::PassKey<PageAggregatorAccess>(), value);
+  }
 };
 
 // Specify the packing alignment for this class as it's expected to have a
@@ -90,6 +94,14 @@ class PageAggregator::Data : public NodeAttachedDataImpl<Data> {
       PageNodeImpl* page_node,
       const FrameNode* frame_node_being_removed);
 
+  // Updates the counter of frames with user-initiated edits and sets the
+  // corresponding page-level property.  |frame_node_being_removed| indicates
+  // if this function is called while removing a frame node.
+  void UpdateCurrentFrameCountForUserEdits(
+      bool frame_had_user_edits,
+      PageNodeImpl* page_node,
+      const FrameNode* frame_node_being_removed);
+
  private:
   friend class PageAggregator;
 
@@ -100,6 +112,9 @@ class PageAggregator::Data : public NodeAttachedDataImpl<Data> {
 
   // The number of current frames which have received some form interaction.
   uint32_t num_current_frames_with_form_interaction_ = 0;
+
+  // The number of current frames which have some user-initiated edits.
+  uint32_t num_current_frames_with_user_edits_ = 0;
 };
 #pragma pack(pop)
 
@@ -160,6 +175,36 @@ void PageAggregator::Data::UpdateCurrentFrameCountForFormInteraction(
       page_node, num_current_frames_with_form_interaction_ > 0);
 }
 
+void PageAggregator::Data::UpdateCurrentFrameCountForUserEdits(
+    bool frame_had_user_edits,
+    PageNodeImpl* page_node,
+    const FrameNode* frame_node_being_removed) {
+  if (frame_had_user_edits) {
+    ++num_current_frames_with_user_edits_;
+  } else {
+    DCHECK_GT(num_current_frames_with_user_edits_, 0U);
+    --num_current_frames_with_user_edits_;
+  }
+  // DCHECK that the |num_current_frames_with_user_edits_| accounting is
+  // correct.
+  DCHECK_EQ(
+      [&]() {
+        const auto frame_nodes = GraphOperations::GetFrameNodes(page_node);
+        size_t num_current_frames_with_user_edits = 0;
+        for (const auto* node : frame_nodes) {
+          if (node != frame_node_being_removed && node->IsCurrent() &&
+              node->HadUserEdits()) {
+            ++num_current_frames_with_user_edits;
+          }
+        }
+        return num_current_frames_with_user_edits;
+      }(),
+      num_current_frames_with_user_edits_);
+
+  PageAggregatorAccess::SetPageHadUserEdits(
+      page_node, num_current_frames_with_user_edits_ > 0);
+}
+
 PageAggregator::PageAggregator() = default;
 PageAggregator::~PageAggregator() = default;
 
@@ -173,11 +218,15 @@ void PageAggregator::OnBeforeFrameNodeRemoved(const FrameNode* frame_node) {
   if (frame_node->IsCurrent()) {
     // Data should have been created when the frame became current.
     DCHECK(data);
+    // Decrement the form interaction and user edits counters for this page if
+    // needed.
     if (frame_node->HadFormInteraction()) {
-      // Decrement the form interaction counter for this page.
       data->UpdateCurrentFrameCountForFormInteraction(false, page_node,
                                                       frame_node);
     }
+    if (frame_node->HadUserEdits()) {
+      data->UpdateCurrentFrameCountForUserEdits(false, page_node, frame_node);
+    }
   }
 
   // It is not guaranteed that the graph will be notified that the frame has
@@ -192,8 +241,8 @@ void PageAggregator::OnIsCurrentChanged(const FrameNode* frame_node) {
   auto* page_node = PageNodeImpl::FromNode(frame_node->GetPageNode());
   Data* data = Data::GetOrCreate(page_node);
 
-  // Check if the frame node had some form interaction, in this case there's two
-  // possibilities:
+  // Check if the frame node had some form interaction or user edit, in this
+  // case there's two possibilities:
   //   - The frame became current: The counter of current frames with form
   //     interactions should be increased.
   //   - The frame became non current: The counter of current frames with form
@@ -202,6 +251,10 @@ void PageAggregator::OnIsCurrentChanged(const FrameNode* frame_node) {
     data->UpdateCurrentFrameCountForFormInteraction(frame_node->IsCurrent(),
                                                     page_node, nullptr);
   }
+  if (frame_node->HadUserEdits()) {
+    data->UpdateCurrentFrameCountForUserEdits(frame_node->IsCurrent(),
+                                              page_node, nullptr);
+  }
 }
 
 void PageAggregator::OnFrameIsHoldingWebLockChanged(
@@ -229,6 +282,15 @@ void PageAggregator::OnHadFormInteractionChanged(const FrameNode* frame_node) {
   }
 }
 
+void PageAggregator::OnHadUserEditsChanged(const FrameNode* frame_node) {
+  if (frame_node->IsCurrent()) {
+    auto* page_node = PageNodeImpl::FromNode(frame_node->GetPageNode());
+    Data* data = Data::GetOrCreate(page_node);
+    data->UpdateCurrentFrameCountForUserEdits(frame_node->HadUserEdits(),
+                                              page_node, nullptr);
+  }
+}
+
 void PageAggregator::OnPassedToGraph(Graph* graph) {
   // This observer presumes that it's been added before any frame nodes exist in
   // the graph.

chrome/browser/performance_manager/decorators/page_aggregator.h

@@ -39,6 +39,7 @@ class PageAggregator : public FrameNode::ObserverDefaultImpl,
   void OnFrameIsHoldingIndexedDBLockChanged(
       const FrameNode* frame_node) override;
   void OnHadFormInteractionChanged(const FrameNode* frame_node) override;
+  void OnHadUserEditsChanged(const FrameNode* frame_node) override;
 
   // GraphOwned implementation:
   void OnPassedToGraph(Graph* graph) override;

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

@@ -101,6 +101,9 @@ class DiscardsGraphDumpImpl : public discards::mojom::GraphDump,
   void OnHadFormInteractionChanged(
       const performance_manager::FrameNode* frame_node) override {}
   // Ignored.
+  void OnHadUserEditsChanged(
+      const performance_manager::FrameNode* frame_node) override {}
+  // Ignored.
   void OnIsAudibleChanged(
       const performance_manager::FrameNode* frame_node) override {}
   // Ignored.
@@ -159,6 +162,9 @@ class DiscardsGraphDumpImpl : public discards::mojom::GraphDump,
   // Ignored.
   void OnHadFormInteractionChanged(
       const performance_manager::PageNode* page_node) override {}
+  // Ignored
+  void OnHadUserEditsChanged(
+      const performance_manager::PageNode* page_node) override {}
   // Ignored.
   void OnTitleUpdated(const performance_manager::PageNode* page_node) override {
   }

components/performance_manager/graph/frame_node_impl.cc

@@ -93,6 +93,11 @@ void FrameNodeImpl::SetHadFormInteraction() {
   document_.had_form_interaction.SetAndMaybeNotify(this, true);
 }
 
+void FrameNodeImpl::SetHadUserEdits() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  document_.had_user_edits.SetAndMaybeNotify(this, true);
+}
+
 void FrameNodeImpl::OnNonPersistentNotificationCreated() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   for (auto* observer : GetObservers())
@@ -228,6 +233,11 @@ bool FrameNodeImpl::had_form_interaction() const {
   return document_.had_form_interaction.value();
 }
 
+bool FrameNodeImpl::had_user_edits() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return document_.had_user_edits.value();
+}
+
 bool FrameNodeImpl::is_audible() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return is_audible_.value();
@@ -577,6 +587,11 @@ bool FrameNodeImpl::HadFormInteraction() const {
   return had_form_interaction();
 }
 
+bool FrameNodeImpl::HadUserEdits() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return had_user_edits();
+}
+
 bool FrameNodeImpl::IsAudible() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return is_audible();
@@ -781,6 +796,7 @@ void FrameNodeImpl::DocumentProperties::Reset(FrameNodeImpl* frame_node,
   // Network is busy on navigation.
   network_almost_idle.SetAndMaybeNotify(frame_node, false);
   had_form_interaction.SetAndMaybeNotify(frame_node, false);
+  had_user_edits.SetAndMaybeNotify(frame_node, false);
 }
 
 void FrameNodeImpl::OnWebMemoryMeasurementRequested(

components/performance_manager/graph/frame_node_impl.h

@@ -67,6 +67,7 @@ class FrameNodeImpl
   void SetHasNonEmptyBeforeUnload(bool has_nonempty_beforeunload) override;
   void SetIsAdFrame(bool is_ad_frame) override;
   void SetHadFormInteraction() override;
+  void SetHadUserEdits() override;
   void OnNonPersistentNotificationCreated() override;
   void OnFirstContentfulPaint(
       base::TimeDelta time_since_navigation_start) override;
@@ -103,6 +104,7 @@ class FrameNodeImpl
   const base::flat_set<WorkerNodeImpl*>& child_worker_nodes() const;
   const PriorityAndReason& priority_and_reason() const;
   bool had_form_interaction() const;
+  bool had_user_edits() const;
   bool is_audible() const;
   const absl::optional<gfx::Rect>& viewport_intersection() const;
   Visibility visibility() const;
@@ -189,6 +191,7 @@ class FrameNodeImpl
       const WorkerNodeVisitor& visitor) const override;
   const PriorityAndReason& GetPriorityAndReason() const override;
   bool HadFormInteraction() const override;
+  bool HadUserEdits() const override;
   bool IsAudible() const override;
   const absl::optional<gfx::Rect>& GetViewportIntersection() const override;
   Visibility GetVisibility() const override;
@@ -218,10 +221,19 @@ class FrameNodeImpl
         network_almost_idle{false};
 
     // Indicates if a form in the frame has been interacted with.
+    // TODO(crbug.com/1156388): Remove this once HadUserEdits is known to cover
+    // all existing cases.
     ObservedProperty::NotifiesOnlyOnChanges<
         bool,
         &FrameNodeObserver::OnHadFormInteractionChanged>
         had_form_interaction{false};
+
+    // Indicates that the user has made edits to the page. This is a superset of
+    // `had_form_interaction`, but can also represent changes to
+    // `contenteditable` elements.
+    ObservedProperty::
+        NotifiesOnlyOnChanges<bool, &FrameNodeObserver::OnHadUserEditsChanged>
+            had_user_edits{false};
   };
 
   // Invoked by subframes on joining/leaving the graph.

components/performance_manager/graph/frame_node_impl_describer.cc

@@ -66,6 +66,7 @@ base::Value::Dict FrameNodeImplDescriber::DescribeFrameNodeData(
           impl->document_.has_nonempty_beforeunload);
   doc.Set("network_almost_idle", impl->document_.network_almost_idle.value());
   doc.Set("had_form_interaction", impl->document_.had_form_interaction.value());
+  ret.Set("had_user_edits", impl->document_.had_user_edits.value());
   ret.Set("document", std::move(doc));
 
   // Frame node properties.

components/performance_manager/graph/frame_node_impl_unittest.cc

@@ -148,6 +148,7 @@ class LenientMockObserver : public FrameNodeImpl::Observer {
   MOCK_METHOD2(OnPriorityAndReasonChanged,
                void(const FrameNode*, const PriorityAndReason& previous_value));
   MOCK_METHOD1(OnHadFormInteractionChanged, void(const FrameNode*));
+  MOCK_METHOD1(OnHadUserEditsChanged, void(const FrameNode*));
   MOCK_METHOD1(OnIsAudibleChanged, void(const FrameNode*));
   MOCK_METHOD1(OnViewportIntersectionChanged, void(const FrameNode*));
   MOCK_METHOD2(OnFrameVisibilityChanged,
@@ -406,6 +407,21 @@ TEST_F(FrameNodeImplTest, FormInteractions) {
   graph()->RemoveFrameNodeObserver(&obs);
 }
 
+TEST_F(FrameNodeImplTest, UserEdits) {
+  auto process = CreateNode<ProcessNodeImpl>();
+  auto page = CreateNode<PageNodeImpl>();
+  auto frame_node = CreateFrameNodeAutoId(process.get(), page.get());
+
+  MockObserver obs;
+  graph()->AddFrameNodeObserver(&obs);
+
+  EXPECT_CALL(obs, OnHadUserEditsChanged(frame_node.get()));
+  frame_node->SetHadUserEdits();
+  EXPECT_TRUE(frame_node->had_user_edits());
+
+  graph()->RemoveFrameNodeObserver(&obs);
+}
+
 TEST_F(FrameNodeImplTest, IsAudible) {
   auto process = CreateNode<ProcessNodeImpl>();
   auto page = CreateNode<PageNodeImpl>();
@@ -519,6 +535,7 @@ TEST_F(FrameNodeImplTest, PublicInterface) {
             public_frame_node->IsHoldingIndexedDBLock());
   EXPECT_EQ(frame_node->had_form_interaction(),
             public_frame_node->HadFormInteraction());
+  EXPECT_EQ(frame_node->had_user_edits(), public_frame_node->HadUserEdits());
   // Use the child frame node to test the viewport intersection because the
   // viewport intersection of the main frame is not tracked.
   EXPECT_EQ(child_frame_node->viewport_intersection(),

components/performance_manager/graph/page_node_impl.cc

@@ -314,6 +314,11 @@ bool PageNodeImpl::had_form_interaction() const {
   return had_form_interaction_.value();
 }
 
+bool PageNodeImpl::had_user_edits() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return had_user_edits_.value();
+}
+
 const absl::optional<freezing::FreezingVote>& PageNodeImpl::freezing_vote()
     const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -568,6 +573,11 @@ bool PageNodeImpl::HadFormInteraction() const {
   return had_form_interaction();
 }
 
+bool PageNodeImpl::HadUserEdits() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return had_user_edits();
+}
+
 const WebContentsProxy& PageNodeImpl::GetContentsProxy() const {
   return contents_proxy();
 }
@@ -623,4 +633,9 @@ void PageNodeImpl::SetHadFormInteraction(bool had_form_interaction) {
   had_form_interaction_.SetAndMaybeNotify(this, had_form_interaction);
 }
 
+void PageNodeImpl::SetHadUserEdits(bool had_user_edits) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  had_user_edits_.SetAndMaybeNotify(this, had_user_edits);
+}
+
 }  // namespace performance_manager