[PM] Migrate BoostingVoteAggregator to VoteObserver

Bug: 971272
Change-Id: I56b43e5119db30fae638be3503a2c990241e5430
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2530215
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828080}

components/performance_manager/execution_context_priority/boosting_vote_aggregator.cc

@@ -189,15 +189,6 @@ void BoostingVoteAggregator::NodeData::DecrementEdgeCount() {
   --edge_count_;
 }
 
-VoteReceipt BoostingVoteAggregator::NodeData::SetIncomingVote(
-    VoteConsumer* consumer,
-    voting::VoterId<Vote> voter_id,
-    const ExecutionContext* execution_context,
-    const Vote& vote) {
-  incoming_ = AcceptedVote(consumer, voter_id, execution_context, vote);
-  return incoming_.IssueReceipt();
-}
-
 BoostingVoteAggregator::EdgeData::EdgeData() = default;
 
 BoostingVoteAggregator::EdgeData::EdgeData(EdgeData&&) = default;
@@ -247,7 +238,8 @@ const char* BoostingVoteAggregator::EdgeData::GetActiveReason() const {
   return reasons_[0];
 }
 
-BoostingVoteAggregator::BoostingVoteAggregator() : factory_(this) {}
+BoostingVoteAggregator::BoostingVoteAggregator()
+    : vote_consumer_default_impl_(this) {}
 
 BoostingVoteAggregator::~BoostingVoteAggregator() {
   DCHECK(forward_edges_.empty());
@@ -257,8 +249,8 @@ BoostingVoteAggregator::~BoostingVoteAggregator() {
 VotingChannel BoostingVoteAggregator::GetVotingChannel() {
   DCHECK(nodes_.empty());
   DCHECK_EQ(voting::kInvalidVoterId<Vote>, input_voter_id_);
-  DCHECK_GT(1u, factory_.voting_channels_issued());
-  auto channel = factory_.BuildVotingChannel();
+  DCHECK_GT(1u, vote_consumer_default_impl_.voting_channels_issued());
+  auto channel = vote_consumer_default_impl_.BuildVotingChannel();
   input_voter_id_ = channel.voter_id();
   return channel;
 }
@@ -395,19 +387,16 @@ void BoostingVoteAggregator::CancelBoostingVote(
   }
 }
 
-VoteReceipt BoostingVoteAggregator::SubmitVote(
-    util::PassKey<VotingChannel>,
-    voting::VoterId<Vote> voter_id,
+void BoostingVoteAggregator::OnVoteSubmitted(
+    VoterId voter_id,
     const ExecutionContext* execution_context,
     const Vote& vote) {
   DCHECK(IsSetup());
   DCHECK_EQ(input_voter_id_, voter_id);
-  DCHECK(vote.IsValid());
 
   // Store the vote.
   auto node_data_it = FindOrCreateNodeData(execution_context);
-  VoteReceipt receipt = node_data_it->second.SetIncomingVote(
-      this, voter_id, execution_context, vote);
+  node_data_it->second.SetIncomingVote(vote);
 
   NodeDataPtrSet changes;
 
@@ -418,21 +407,21 @@ VoteReceipt BoostingVoteAggregator::SubmitVote(
   }
 
   UpstreamChanges(changes);
-
-  return receipt;
 }
 
-void BoostingVoteAggregator::ChangeVote(util::PassKey<AcceptedVote>,
-                                        AcceptedVote* old_vote,
-                                        const Vote& new_vote) {
+void BoostingVoteAggregator::OnVoteChanged(
+    VoterId voter_id,
+    const ExecutionContext* execution_context,
+    const Vote& new_vote) {
+  auto node_data_it = FindNodeData(execution_context);
+  auto* node_data = &(*node_data_it);
+
   // Remember the old and new priorities before committing any changes.
-  base::TaskPriority old_priority = old_vote->vote().value();
+  base::TaskPriority old_priority = node_data->second.incoming_vote().value();
   base::TaskPriority new_priority = new_vote.value();
 
   // Update the vote in place.
-  auto node_data_it = GetNodeDataByVote(old_vote);
-  auto* node_data = &(*node_data_it);
-  old_vote->UpdateVote(new_vote);
+  node_data->second.UpdateIncomingVote(new_vote);
 
   NodeDataPtrSet changes;
   changes.insert(node_data);  // This node is changing regardless.
@@ -452,10 +441,17 @@ void BoostingVoteAggregator::ChangeVote(util::PassKey<AcceptedVote>,
   UpstreamChanges(changes);
 }
 
-void BoostingVoteAggregator::VoteInvalidated(util::PassKey<AcceptedVote>,
-                                             AcceptedVote* vote) {
-  auto node_data_it = GetNodeDataByVote(vote);
-  base::TaskPriority old_priority = vote->vote().value();
+void BoostingVoteAggregator::OnVoteInvalidated(
+    VoterId voter_id,
+    const ExecutionContext* execution_context) {
+  auto node_data_it = FindNodeData(execution_context);
+
+  // Remember the old priority before committing any changes.
+  base::TaskPriority old_priority =
+      node_data_it->second.incoming_vote().value();
+
+  // Update the vote.
+  node_data_it->second.RemoveIncomingVote();
 
   NodeDataPtrSet changes;
 
@@ -498,21 +494,6 @@ void BoostingVoteAggregator::ForEachOutgoingEdge(const ExecutionContext* node,
   }
 }
 
-BoostingVoteAggregator::NodeDataMap::iterator
-BoostingVoteAggregator::GetNodeDataByVote(AcceptedVote* vote) {
-  // The vote being retrieved should have us as its consumer, and have been
-  // emitted by our known voter.
-  DCHECK(vote);
-  DCHECK_EQ(this, vote->consumer());
-  DCHECK(vote->voter_id() == input_voter_id_);
-  DCHECK(IsSetup());
-
-  auto it = nodes_.find(vote->context());
-  DCHECK(it != nodes_.end());
-  DCHECK_EQ(vote, &it->second.incoming());
-  return it;
-}
-
 BoostingVoteAggregator::NodeDataMap::iterator
 BoostingVoteAggregator::FindOrCreateNodeData(const ExecutionContext* node) {
   auto it = nodes_.lower_bound(node);
@@ -524,7 +505,7 @@ BoostingVoteAggregator::FindOrCreateNodeData(const ExecutionContext* node) {
 
 BoostingVoteAggregator::NodeDataMap::iterator
 BoostingVoteAggregator::FindNodeData(const ExecutionContext* node) {
-  auto it = nodes_.lower_bound(node);
+  auto it = nodes_.find(node);
   DCHECK(it != nodes_.end());
   return it;
 }
@@ -542,11 +523,11 @@ const char* BoostingVoteAggregator::GetVoteReason(
   // If a vote has been expressed for this node at the given priority level then
   // preferentially use that reason.
   if (node_data.HasIncomingVote()) {
-    const Vote& incoming = node_data.incoming().vote();
-    if (priority == incoming.value()) {
+    const Vote& incoming_vote = node_data.incoming_vote();
+    if (priority == incoming_vote.value()) {
       // This node will not have an active incoming edge in this case.
       DCHECK(GetActiveInboundEdge(layer_bit, node) == reverse_edges_.end());
-      return incoming.reason();
+      return incoming_vote.reason();
     }
   }
 

components/performance_manager/public/execution_context_priority/boosting_vote_aggregator.h

@@ -9,6 +9,7 @@
 #include <set>
 
 #include "base/containers/flat_map.h"
+#include "base/optional.h"
 #include "base/task/task_traits.h"
 #include "components/performance_manager/public/execution_context_priority/execution_context_priority.h"
 
@@ -69,7 +70,7 @@ class BoostingVote {
 // execution context Y". It is intended to serve as the root of a tree of voters
 // and aggregators, allowing priority boost semantics to be implemented. This
 // class must outlive all boosting votes registered with it.
-class BoostingVoteAggregator : public VoteConsumer {
+class BoostingVoteAggregator : public VoteObserver {
  public:
   BoostingVoteAggregator();
   ~BoostingVoteAggregator() override;
@@ -126,15 +127,21 @@ class BoostingVoteAggregator : public VoteConsumer {
     NodeData& operator=(NodeData&& rhs) = default;
     ~NodeData();
 
-    const AcceptedVote& incoming() const { return incoming_; }
+    const Vote& incoming_vote() const { return incoming_vote_.value(); }
     const VoteReceipt& receipt() const { return receipt_; }
 
-    // For modifying |incoming_|.
-    VoteReceipt SetIncomingVote(VoteConsumer* consumer,
-                                voting::VoterId<Vote> voter_id,
-                                const ExecutionContext* execution_context,
-                                const Vote& vote);
-    void UpdateIncomingVote(const Vote& vote) { incoming_.UpdateVote(vote); }
+    void SetIncomingVote(const Vote& incoming_vote) {
+      DCHECK(!incoming_vote_.has_value());
+      incoming_vote_ = incoming_vote;
+    }
+    void UpdateIncomingVote(const Vote& incoming_vote) {
+      DCHECK(incoming_vote_.has_value());
+      incoming_vote_ = incoming_vote;
+    }
+    void RemoveIncomingVote() {
+      DCHECK(incoming_vote_.has_value());
+      incoming_vote_ = base::nullopt;
+    }
 
     // For modifying |receipt_|.
     void ChangeOutgoingVote(base::TaskPriority priority, const char* reason) {
@@ -147,10 +154,10 @@ class BoostingVoteAggregator : public VoteConsumer {
 
     // Returns true if this node has an active |incoming| vote. If false that
     // means this node exists only because it is referenced by a BoostedVote.
-    // Same as |incoming_.IsValid()|, but more readable.
-    bool HasIncomingVote() const { return incoming_.IsValid(); }
+    // Same as |incoming_vote_.has_value()|, but more readable.
+    bool HasIncomingVote() const { return incoming_vote_.has_value(); }
 
-    // Returns true if this node has an active outgoing vote. Sam as
+    // Returns true if this node has an active outgoing vote. Same as
     // |receipt_.HasVote()|, but more readable.
     bool HasOutgoingVote() const { return receipt_.HasVote(); }
 
@@ -174,7 +181,7 @@ class BoostingVoteAggregator : public VoteConsumer {
     size_t edge_count_ = 0;
 
     // The input vote we've received, if any.
-    AcceptedVote incoming_;
+    base::Optional<Vote> incoming_vote_;
 
     // The receipt for the vote we've upstreamed, if any.
     VoteReceipt receipt_;
@@ -267,16 +274,15 @@ class BoostingVoteAggregator : public VoteConsumer {
   void SubmitBoostingVote(const BoostingVote* boosting_vote);
   void CancelBoostingVote(const BoostingVote* boosting_vote);
 
-  // VoteConsumer implementation:
-  VoteReceipt SubmitVote(util::PassKey<VotingChannel>,
-                         voting::VoterId<Vote> voter_id,
-                         const ExecutionContext* execution_context,
-                         const Vote& vote) override;
-  void ChangeVote(util::PassKey<AcceptedVote>,
-                  AcceptedVote* old_vote,
-                  const Vote& new_vote) override;
-  void VoteInvalidated(util::PassKey<AcceptedVote>,
-                       AcceptedVote* vote) override;
+  // VoteObserver implementation:
+  void OnVoteSubmitted(VoterId voter_id,
+                       const ExecutionContext* execution_context,
+                       const Vote& vote) override;
+  void OnVoteChanged(VoterId voter_id,
+                     const ExecutionContext* execution_context,
+                     const Vote& new_vote) override;
+  void OnVoteInvalidated(VoterId voter_id,
+                         const ExecutionContext* execution_context) override;
 
   // Helper functions for enumerating over incoming and outgoing edges.
   // |function| should accept a single input parameter that is a
@@ -288,10 +294,6 @@ class BoostingVoteAggregator : public VoteConsumer {
   template <typename Function>
   void ForEachOutgoingEdge(const ExecutionContext* node, Function&& function);
 
-  // Looks up the NodeData associated with the provided |vote|. The data is
-  // expected to already exist (enforced by a DCHECK).
-  NodeDataMap::iterator GetNodeDataByVote(AcceptedVote* vote);
-
   // Finds or creates the node data associated with the given node.
   NodeDataMap::iterator FindOrCreateNodeData(const ExecutionContext* node);
   NodeDataMap::iterator FindNodeData(const ExecutionContext* node);
@@ -371,8 +373,8 @@ class BoostingVoteAggregator : public VoteConsumer {
   // Our channel for upstreaming our votes.
   VotingChannel channel_;
 
-  // Our VotingChannelFactory for providing a VotingChannel to our input voter.
-  VotingChannelFactory factory_;
+  // Provides a VotingChannel to our input voter.
+  VoteConsumerDefaultImpl vote_consumer_default_impl_;
 
   // Nodes and associated metadata in the "priority flow graph". An entry exists
   // in this map for any node that has an active non-default vote, or for any