Don't allow VoteConsumer to replace the VoteReceipt in ChangeVote().

All of the implementations of ChangeVote() already reuse the same
VoteReceipt as it is more efficient. This means that this feature of
the interface is not worth the complexity cost.

Change-Id: I434edb1d77acaecb6b2a151c86ac3b79453ec3eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2506393
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823865}

chrome/browser/performance_manager/decorators/execution_context_priority_decorator.cc

@@ -77,11 +77,9 @@ VoteReceipt ExecutionContextPriorityDecorator::SubmitVote(
   return accepted_vote->IssueReceipt();
 }
 
-VoteReceipt ExecutionContextPriorityDecorator::ChangeVote(
-    util::PassKey<AcceptedVote>,
-    VoteReceipt receipt,
-    AcceptedVote* old_vote,
-    const Vote& new_vote) {
+void ExecutionContextPriorityDecorator::ChangeVote(util::PassKey<AcceptedVote>,
+                                                   AcceptedVote* old_vote,
+                                                   const Vote& new_vote) {
   const auto* execution_context = new_vote.context();
   auto* accepted_vote =
       ExecutionContextPriorityAccess::GetAcceptedVote(execution_context);
@@ -90,7 +88,6 @@ VoteReceipt ExecutionContextPriorityDecorator::ChangeVote(
   accepted_vote->UpdateVote(new_vote);
   SetPriorityAndReason(execution_context,
                        PriorityAndReason(new_vote.value(), new_vote.reason()));
-  return receipt;
 }
 
 void ExecutionContextPriorityDecorator::VoteInvalidated(

chrome/browser/performance_manager/decorators/execution_context_priority_decorator.h

@@ -28,10 +28,9 @@ class ExecutionContextPriorityDecorator : public GraphOwnedDefaultImpl,
   VoteReceipt SubmitVote(util::PassKey<VotingChannel>,
                          voting::VoterId<Vote> voter_id,
                          const Vote& vote) override;
-  VoteReceipt ChangeVote(util::PassKey<AcceptedVote>,
-                         VoteReceipt receipt,
-                         AcceptedVote* old_vote,
-                         const Vote& new_vote) override;
+  void ChangeVote(util::PassKey<AcceptedVote>,
+                  AcceptedVote* old_vote,
+                  const Vote& new_vote) override;
   void VoteInvalidated(util::PassKey<AcceptedVote>,
                        AcceptedVote* vote) override;
 

components/performance_manager/execution_context_priority/boosting_vote_aggregator.cc

@@ -419,10 +419,9 @@ VoteReceipt BoostingVoteAggregator::SubmitVote(util::PassKey<VotingChannel>,
   return receipt;
 }
 
-VoteReceipt BoostingVoteAggregator::ChangeVote(util::PassKey<AcceptedVote>,
-                                               VoteReceipt receipt,
-                                               AcceptedVote* old_vote,
-                                               const Vote& new_vote) {
+void BoostingVoteAggregator::ChangeVote(util::PassKey<AcceptedVote>,
+                                        AcceptedVote* old_vote,
+                                        const Vote& new_vote) {
   // Remember the old and new priorities before committing any changes.
   base::TaskPriority old_priority = old_vote->vote().value();
   base::TaskPriority new_priority = new_vote.value();
@@ -448,9 +447,6 @@ VoteReceipt BoostingVoteAggregator::ChangeVote(util::PassKey<AcceptedVote>,
   }
 
   UpstreamChanges(changes);
-
-  // Return the original receipt, as its still valid.
-  return receipt;
 }
 
 void BoostingVoteAggregator::VoteInvalidated(util::PassKey<AcceptedVote>,

components/performance_manager/execution_context_priority/max_vote_aggregator.cc

@@ -54,11 +54,9 @@ VoteReceipt MaxVoteAggregator::SubmitVote(util::PassKey<VotingChannel>,
   return receipt;
 }
 
-VoteReceipt MaxVoteAggregator::ChangeVote(util::PassKey<AcceptedVote>,
-                                          VoteReceipt receipt,
-                                          AcceptedVote* old_vote,
-                                          const Vote& new_vote) {
-  DCHECK(receipt.HasVote(old_vote));
+void MaxVoteAggregator::ChangeVote(util::PassKey<AcceptedVote>,
+                                   AcceptedVote* old_vote,
+                                   const Vote& new_vote) {
   DCHECK(old_vote->IsValid());
   VoteData& vote_data = GetVoteData(old_vote)->second;
   size_t index = vote_data.GetVoteIndex(old_vote);
@@ -67,9 +65,6 @@ VoteReceipt MaxVoteAggregator::ChangeVote(util::PassKey<AcceptedVote>,
   old_vote->UpdateVote(new_vote);
   if (vote_data.UpdateVote(index, next_vote_id_++))
     vote_data.UpstreamVote(&channel_);
-
-  // Return the same receipt back to the user.
-  return receipt;
 }
 
 void MaxVoteAggregator::VoteInvalidated(util::PassKey<AcceptedVote>,

components/performance_manager/execution_context_priority/override_vote_aggregator.cc

@@ -64,11 +64,9 @@ VoteReceipt OverrideVoteAggregator::SubmitVote(util::PassKey<VotingChannel>,
   }
 }
 
-VoteReceipt OverrideVoteAggregator::ChangeVote(util::PassKey<AcceptedVote>,
-                                               VoteReceipt receipt,
-                                               AcceptedVote* old_vote,
-                                               const Vote& new_vote) {
-  DCHECK(receipt.HasVote(old_vote));
+void OverrideVoteAggregator::ChangeVote(util::PassKey<AcceptedVote>,
+                                        AcceptedVote* old_vote,
+                                        const Vote& new_vote) {
   DCHECK(old_vote->IsValid());
   VoteData& vote_data = GetVoteData(old_vote)->second;
 
@@ -82,9 +80,6 @@ VoteReceipt OverrideVoteAggregator::ChangeVote(util::PassKey<AcceptedVote>,
       !vote_data.override_vote.IsValid()) {
     UpstreamVote(new_vote, &vote_data);
   }
-
-  // Pass the same receipt right back to the user.
-  return receipt;
 }
 
 void OverrideVoteAggregator::VoteInvalidated(util::PassKey<AcceptedVote>,

components/performance_manager/public/execution_context_priority/boosting_vote_aggregator.h

@@ -270,10 +270,9 @@ class BoostingVoteAggregator : public VoteConsumer {
   VoteReceipt SubmitVote(util::PassKey<VotingChannel>,
                          voting::VoterId<Vote> voter_id,
                          const Vote& vote) override;
-  VoteReceipt ChangeVote(util::PassKey<AcceptedVote>,
-                         VoteReceipt receipt,
-                         AcceptedVote* old_vote,
-                         const Vote& new_vote) override;
+  void ChangeVote(util::PassKey<AcceptedVote>,
+                  AcceptedVote* old_vote,
+                  const Vote& new_vote) override;
   void VoteInvalidated(util::PassKey<AcceptedVote>,
                        AcceptedVote* vote) override;
 

components/performance_manager/public/execution_context_priority/max_vote_aggregator.h

@@ -34,10 +34,9 @@ class MaxVoteAggregator : public VoteConsumer {
   VoteReceipt SubmitVote(util::PassKey<VotingChannel>,
                          voting::VoterId<Vote> voter_id,
                          const Vote& vote) override;
-  VoteReceipt ChangeVote(util::PassKey<AcceptedVote>,
-                         VoteReceipt receipt,
-                         AcceptedVote* old_vote,
-                         const Vote& new_vote) override;
+  void ChangeVote(util::PassKey<AcceptedVote>,
+                  AcceptedVote* old_vote,
+                  const Vote& new_vote) override;
   void VoteInvalidated(util::PassKey<AcceptedVote>,
                        AcceptedVote* vote) override;
 

components/performance_manager/public/execution_context_priority/override_vote_aggregator.h

@@ -35,10 +35,9 @@ class OverrideVoteAggregator : public VoteConsumer {
   VoteReceipt SubmitVote(util::PassKey<VotingChannel>,
                          voting::VoterId<Vote> voter_id,
                          const Vote& vote) override;
-  VoteReceipt ChangeVote(util::PassKey<AcceptedVote>,
-                         VoteReceipt receipt,
-                         AcceptedVote* old_vote,
-                         const Vote& new_vote) override;
+  void ChangeVote(util::PassKey<AcceptedVote>,
+                  AcceptedVote* old_vote,
+                  const Vote& new_vote) override;
   void VoteInvalidated(util::PassKey<AcceptedVote>,
                        AcceptedVote* vote) override;
 

components/performance_manager/public/voting/voting.h

@@ -232,12 +232,10 @@ class AcceptedVote final {
                    VoteReceipt* old_receipt,
                    VoteReceipt* new_receipt);
 
-  // Allows a VoteReceipt to change this vote. The vote receipt gives up its
-  // receipt only to be returned a new one.
-  VoteReceipt ChangeVote(util::PassKey<VoteReceipt>,
-                         VoteReceipt receipt,
-                         typename VoteImpl::VoteType vote,
-                         const char* reason);
+  // Allows a VoteReceipt to change this vote.
+  void ChangeVote(util::PassKey<VoteReceipt>,
+                  typename VoteImpl::VoteType vote,
+                  const char* reason);
 
   // Allows a VoteReceipt to invalidate this vote.
   void InvalidateVote(util::PassKey<VoteReceipt>, VoteReceipt* receipt);
@@ -372,23 +370,11 @@ class VoteConsumer {
                                            const VoteImpl& vote) = 0;
 
   // Used by an AcceptedVote to notify a consumer that a previously issued vote
-  // has been changed. Both the |new_vote| and the |receipt| are provided to the
-  // consumer, as it may be necessary for the consumer to create an entirely
-  // new vote and issue a new receipt for it (although ideally it does not do so
-  // for the sake of efficiency). Alternatively, the consumer can choose to
-  // update the |old_vote| in-place, using the data from |new_vote|. This is
-  // kept protected as it is part of a private contract between an AcceptedVote
-  // and a VoteConsumer. A naive implementation of this would be the following:
-  //
-  //   // Tear down the old vote before submitting a new one in order to prevent
-  //   // the voter from having 2 simultaneous votes for the same context.
-  //   auto voter_id = receipt.GetVoterId();
-  //   receipt.Reset();
-  //   return SubmitVote(PassKey(), voter_id, new_vote);
-  virtual VoteReceipt<VoteImpl> ChangeVote(util::PassKey<AcceptedVote>,
-                                           VoteReceipt<VoteImpl> receipt,
-                                           AcceptedVote* old_vote,
-                                           const VoteImpl& new_vote) = 0;
+  // has been changed. The consumer should update |old_vote| in-place using the
+  // data from |new_vote|.
+  virtual void ChangeVote(util::PassKey<AcceptedVote>,
+                          AcceptedVote* old_vote,
+                          const VoteImpl& new_vote) = 0;
 
   // Used by a AcceptedVote to notify a consumer that a previously issued
   // receipt has been destroyed, and the vote is now invalidated. This is kept
@@ -499,7 +485,7 @@ void VoteReceipt<VoteImpl>::ChangeVote(typename VoteImpl::VoteType new_vote,
   if (vote.value() == new_vote && vote.reason() == reason)
     return;
 
-  *this = vote_->ChangeVote(PassKey(), std::move(*this), new_vote, reason);
+  vote_->ChangeVote(PassKey(), new_vote, reason);
 }
 
 template <class VoteImpl>
@@ -644,12 +630,9 @@ void AcceptedVote<VoteImpl>::MoveReceipt(util::PassKey<VoteReceipt>,
 }
 
 template <class VoteImpl>
-VoteReceipt<VoteImpl> AcceptedVote<VoteImpl>::ChangeVote(
-    util::PassKey<VoteReceipt>,
-    VoteReceipt receipt,
-    typename VoteImpl::VoteType vote,
-    const char* reason) {
-  DCHECK_EQ(receipt_, &receipt);
+void AcceptedVote<VoteImpl>::ChangeVote(util::PassKey<VoteReceipt>,
+                                        typename VoteImpl::VoteType vote,
+                                        const char* reason) {
   DCHECK(!invalidated_);
   DCHECK(vote_.value() != vote || vote_.reason() != reason);
 
@@ -659,17 +642,7 @@ VoteReceipt<VoteImpl> AcceptedVote<VoteImpl>::ChangeVote(
 
   // Notify the consumer of the new vote.
   VoteImpl new_vote = VoteImpl(old_vote.context(), vote, reason);
-  receipt =
-      consumer_->ChangeVote(PassKey(), std::move(receipt), this, new_vote);
-
-  // Ensure that the returned receipt refers to a vote with the expected
-  // properties.
-  const VoteImpl& returned_vote = receipt.GetVote();
-  DCHECK_EQ(new_vote.context(), returned_vote.context());
-  DCHECK_EQ(new_vote.value(), returned_vote.value());
-  DCHECK_EQ(new_vote.reason(), returned_vote.reason());
-
-  return receipt;
+  consumer_->ChangeVote(PassKey(), this, new_vote);
 }
 
 template <class VoteImpl>

components/performance_manager/test_support/voting.h

@@ -33,10 +33,9 @@ class DummyVoteConsumer : public VoteConsumer<VoteImpl> {
   VoteReceipt<VoteImpl> SubmitVote(util::PassKey<VotingChannel>,
                                    voting::VoterId<VoteImpl> voter_id,
                                    const VoteImpl& vote) override;
-  VoteReceipt<VoteImpl> ChangeVote(util::PassKey<AcceptedVote>,
-                                   VoteReceipt<VoteImpl> receipt,
-                                   AcceptedVote* old_vote,
-                                   const VoteImpl& new_vote) override;
+  void ChangeVote(util::PassKey<AcceptedVote>,
+                  AcceptedVote* old_vote,
+                  const VoteImpl& new_vote) override;
   void VoteInvalidated(util::PassKey<AcceptedVote>,
                        AcceptedVote* vote) override;
 
@@ -108,21 +107,17 @@ VoteReceipt<VoteImpl> DummyVoteConsumer<VoteImpl>::SubmitVote(
 }
 
 template <class VoteImpl>
-VoteReceipt<VoteImpl> DummyVoteConsumer<VoteImpl>::ChangeVote(
-    util::PassKey<AcceptedVote>,
-    VoteReceipt<VoteImpl> receipt,
-    AcceptedVote* old_vote,
-    const VoteImpl& new_vote) {
+void DummyVoteConsumer<VoteImpl>::ChangeVote(util::PassKey<AcceptedVote>,
+                                             AcceptedVote* old_vote,
+                                             const VoteImpl& new_vote) {
   // We should own this vote and it should be valid.
-  EXPECT_TRUE(receipt.HasVote(old_vote));
   EXPECT_LE(votes_.data(), old_vote);
   EXPECT_LT(old_vote, votes_.data() + votes_.size());
   EXPECT_TRUE(old_vote->IsValid());
   EXPECT_LT(0u, valid_vote_count_);
 
-  // Just update the vote in-place, and return the existing receipt for it.
+  // Update the vote in-place.
   old_vote->UpdateVote(new_vote);
-  return receipt;
 }
 
 template <class VoteImpl>