Commit a1a21b01 authored by Patrick Monette's avatar Patrick Monette Committed by Commit Bot

[PM] Add VoteObserver

This interface is basically a simpler VoteConsumer. It allows
to observe votes made through a voting channel without having
to deal with AcceptedVotes.

One limitation it has is that it is no longer possible to
submit multiple votes to the same context through a single
VotingChannel. In practice, this was never done anyways. To
support multiple votes for one context, it is easy to build
multiple voting channels.

Of course, AcceptedVotes still exists. Those are managed by
the VoteConsumerDefaultImpl, which is a temporary class that
handles creating and deleting AcceptedVotes as needed, then
notifying the VoteObserver.

The goal in the future is to fold this functionality into
the VotingChannelFactory and remove VoteConsumer entirely.

This will be done after each users of VoteConsumer are migrated
one at a time.

Bug: 971272
Change-Id: I4e890435cf29d4375ccb41b393b6d80b2d392778
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2519207
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: default avatarChris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825971}
parent 5e5543ca
...@@ -59,6 +59,7 @@ ...@@ -59,6 +59,7 @@
// maintenance is a reasonable trade-off for memory efficiency. // maintenance is a reasonable trade-off for memory efficiency.
#include <cstring> #include <cstring>
#include <map>
#include <utility> #include <utility>
#include "base/check.h" #include "base/check.h"
...@@ -363,10 +364,7 @@ class VoteConsumer { ...@@ -363,10 +364,7 @@ class VoteConsumer {
public: public:
using ContextType = typename VoteImpl::ContextType; using ContextType = typename VoteImpl::ContextType;
VoteConsumer();
virtual ~VoteConsumer(); virtual ~VoteConsumer();
VoteConsumer(const VoteConsumer& rhs) = delete;
VoteConsumer& operator=(const VoteConsumer& rhs) = delete;
// Used by a VotingChannel to submit votes to this consumer. // Used by a VotingChannel to submit votes to this consumer.
virtual VoteReceipt<VoteImpl> SubmitVote( virtual VoteReceipt<VoteImpl> SubmitVote(
...@@ -390,6 +388,69 @@ class VoteConsumer { ...@@ -390,6 +388,69 @@ class VoteConsumer {
AcceptedVote<VoteImpl>* vote) = 0; AcceptedVote<VoteImpl>* vote) = 0;
}; };
template <class VoteImpl>
class VoteConsumerDefaultImpl;
template <class VoteImpl>
class VoteObserver {
public:
using ContextType = typename VoteImpl::ContextType;
virtual ~VoteObserver();
// Invoked when a |vote| is submitted for |context|. |voter_id| identifies the
// voting channel.
virtual void OnVoteSubmitted(VoterId<VoteImpl> voter_id,
const ContextType* context,
const VoteImpl& vote) = 0;
// Invoked when the vote for |context| is changed to |new_vote|. |voter_id|
// identifies the voting channel.
virtual void OnVoteChanged(VoterId<VoteImpl> voter_id,
const ContextType* context,
const VoteImpl& new_vote) = 0;
// Invoked when a vote for |context| is invalided. |voter_id| identifies the
// voting channel.
virtual void OnVoteInvalidated(VoterId<VoteImpl> voter_id,
const ContextType* context) = 0;
};
template <class VoteImpl>
class VoteConsumerDefaultImpl : public VoteConsumer<VoteImpl> {
public:
using ContextType = typename VoteImpl::ContextType;
using PassKey = util::PassKey<VoteConsumerDefaultImpl>;
explicit VoteConsumerDefaultImpl(VoteObserver<VoteImpl>* vote_observer);
~VoteConsumerDefaultImpl() override;
// Builds a new VotingChannel that routes votes to |vote_observer_|.
VotingChannel<VoteImpl> BuildVotingChannel();
size_t voting_channels_issued() const {
return voting_channel_factory_.voting_channels_issued();
}
// VoteConsumer:
VoteReceipt<VoteImpl> SubmitVote(util::PassKey<VotingChannel<VoteImpl>>,
VoterId<VoteImpl> voter_id,
const ContextType* context,
const VoteImpl& vote) override;
void ChangeVote(util::PassKey<AcceptedVote<VoteImpl>>,
AcceptedVote<VoteImpl>* old_vote,
const VoteImpl& new_vote) override;
void VoteInvalidated(util::PassKey<AcceptedVote<VoteImpl>>,
AcceptedVote<VoteImpl>* vote) override;
private:
VoteObserver<VoteImpl>* vote_observer_;
VotingChannelFactory<VoteImpl> voting_channel_factory_;
std::map<const ContextType*, AcceptedVote<VoteImpl>> accepted_votes_;
};
///////////////////////////////////////////////////////////////////// /////////////////////////////////////////////////////////////////////
// Vote // Vote
...@@ -779,11 +840,82 @@ void VotingChannelFactory<VoteImpl>::OnVotingChannelDestroyed( ...@@ -779,11 +840,82 @@ void VotingChannelFactory<VoteImpl>::OnVotingChannelDestroyed(
///////////////////////////////////////////////////////////////////// /////////////////////////////////////////////////////////////////////
// VoteConsumer // VoteConsumer
template <class VoteImpl>
VoteConsumer<VoteImpl>::VoteConsumer() = default;
template <class VoteImpl> template <class VoteImpl>
VoteConsumer<VoteImpl>::~VoteConsumer() = default; VoteConsumer<VoteImpl>::~VoteConsumer() = default;
/////////////////////////////////////////////////////////////////////
// VoteObserver
template <class VoteImpl>
VoteObserver<VoteImpl>::~VoteObserver() = default;
/////////////////////////////////////////////////////////////////////
// VoteConsumerDefaultImpl
template <class VoteImpl>
VoteConsumerDefaultImpl<VoteImpl>::VoteConsumerDefaultImpl(
VoteObserver<VoteImpl>* vote_observer)
: vote_observer_(vote_observer), voting_channel_factory_(this) {}
template <class VoteImpl>
VoteConsumerDefaultImpl<VoteImpl>::~VoteConsumerDefaultImpl() = default;
template <class VoteImpl>
VotingChannel<VoteImpl>
VoteConsumerDefaultImpl<VoteImpl>::BuildVotingChannel() {
return voting_channel_factory_.BuildVotingChannel();
}
template <class VoteImpl>
VoteReceipt<VoteImpl> VoteConsumerDefaultImpl<VoteImpl>::SubmitVote(
util::PassKey<VotingChannel<VoteImpl>>,
VoterId<VoteImpl> voter_id,
const ContextType* context,
const VoteImpl& vote) {
AcceptedVote<VoteImpl> accepted_vote(this, voter_id, context, vote);
VoteReceipt<VoteImpl> vote_receipt = accepted_vote.IssueReceipt();
bool inserted =
accepted_votes_.emplace(context, std::move(accepted_vote)).second;
DCHECK(inserted);
vote_observer_->OnVoteSubmitted(voter_id, context, vote);
return vote_receipt;
}
template <class VoteImpl>
void VoteConsumerDefaultImpl<VoteImpl>::ChangeVote(
util::PassKey<AcceptedVote<VoteImpl>>,
AcceptedVote<VoteImpl>* old_vote,
const VoteImpl& new_vote) {
auto it = accepted_votes_.find(old_vote->context());
DCHECK(it != accepted_votes_.end());
auto* accepted_vote = &it->second;
DCHECK_EQ(accepted_vote, old_vote);
accepted_vote->UpdateVote(new_vote);
vote_observer_->OnVoteChanged(accepted_vote->voter_id(),
accepted_vote->context(), new_vote);
}
template <class VoteImpl>
void VoteConsumerDefaultImpl<VoteImpl>::VoteInvalidated(
util::PassKey<AcceptedVote<VoteImpl>>,
AcceptedVote<VoteImpl>* vote) {
auto it = accepted_votes_.find(vote->context());
DCHECK(it != accepted_votes_.end());
auto* accepted_vote = &it->second;
DCHECK_EQ(accepted_vote, vote);
vote_observer_->OnVoteInvalidated(accepted_vote->voter_id(),
accepted_vote->context());
accepted_votes_.erase(it);
}
} // namespace voting } // namespace voting
} // namespace performance_manager } // namespace performance_manager
......
...@@ -7,8 +7,10 @@ ...@@ -7,8 +7,10 @@
#include "components/performance_manager/public/voting/voting.h" #include "components/performance_manager/public/voting/voting.h"
#include <map>
#include <vector> #include <vector>
#include "base/stl_util.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace performance_manager { namespace performance_manager {
...@@ -80,6 +82,35 @@ class DummyVoter { ...@@ -80,6 +82,35 @@ class DummyVoter {
std::vector<VoteReceipt<VoteImpl>> receipts_; std::vector<VoteReceipt<VoteImpl>> receipts_;
}; };
template <class VoteImpl>
class DummyVoteObserver : public VoteObserver<VoteImpl> {
public:
using ContextType = typename VoteImpl::ContextType;
using VoteConsumerDefaultImpl = VoteConsumerDefaultImpl<VoteImpl>;
DummyVoteObserver();
~DummyVoteObserver() override;
// VoteObserver:
void OnVoteSubmitted(voting::VoterId<VoteImpl> voter_id,
const ContextType* context,
const VoteImpl& vote) override;
void OnVoteChanged(voting::VoterId<VoteImpl> voter_id,
const ContextType* context,
const VoteImpl& new_vote) override;
void OnVoteInvalidated(voting::VoterId<VoteImpl> voter_id,
const ContextType* context) override;
bool HasVote(voting::VoterId<VoteImpl> voter_id,
const typename VoteImpl::ContextType* context,
typename VoteImpl::VoteType vote_value,
const char* reason = nullptr);
VoteConsumerDefaultImpl vote_consumer_default_impl_{this};
std::map<voting::VoterId<VoteImpl>, std::map<const ContextType*, VoteImpl>>
votes_by_voter_id_;
};
// IMPLEMENTATION // IMPLEMENTATION
template <class VoteImpl> template <class VoteImpl>
...@@ -186,6 +217,61 @@ void DummyVoter<VoteImpl>::EmitVote( ...@@ -186,6 +217,61 @@ void DummyVoter<VoteImpl>::EmitVote(
voting_channel_.SubmitVote(context, VoteImpl(vote_value, reason))); voting_channel_.SubmitVote(context, VoteImpl(vote_value, reason)));
} }
template <class VoteImpl>
DummyVoteObserver<VoteImpl>::DummyVoteObserver() = default;
template <class VoteImpl>
DummyVoteObserver<VoteImpl>::~DummyVoteObserver() = default;
template <class VoteImpl>
void DummyVoteObserver<VoteImpl>::OnVoteSubmitted(VoterId<VoteImpl> voter_id,
const ContextType* context,
const VoteImpl& vote) {
bool inserted = votes_by_voter_id_[voter_id].emplace(context, vote).second;
DCHECK(inserted);
}
template <class VoteImpl>
void DummyVoteObserver<VoteImpl>::OnVoteChanged(VoterId<VoteImpl> voter_id,
const ContextType* context,
const VoteImpl& new_vote) {
auto it = votes_by_voter_id_[voter_id].find(context);
DCHECK(it != votes_by_voter_id_[voter_id].end());
it->second = new_vote;
}
template <class VoteImpl>
void DummyVoteObserver<VoteImpl>::OnVoteInvalidated(
VoterId<VoteImpl> voter_id,
const ContextType* context) {
auto it = votes_by_voter_id_.find(voter_id);
DCHECK(it != votes_by_voter_id_.end());
std::map<const ContextType*, VoteImpl>& votes = it->second;
size_t removed = votes.erase(context);
DCHECK_EQ(removed, 1u);
if (votes.empty())
votes_by_voter_id_.erase(it);
}
template <class VoteImpl>
bool DummyVoteObserver<VoteImpl>::HasVote(
voting::VoterId<VoteImpl> voter_id,
const typename VoteImpl::ContextType* context,
typename VoteImpl::VoteType vote_value,
const char* reason) {
if (!base::Contains(votes_by_voter_id_, voter_id))
return false;
std::map<const ContextType*, VoteImpl>& votes = votes_by_voter_id_[voter_id];
if (!base::Contains(votes, context))
return false;
return votes[context] == VoteImpl(vote_value, reason);
}
} // namespace test } // namespace test
} // namespace voting } // namespace voting
} // namespace performance_manager } // namespace performance_manager
......
...@@ -24,6 +24,7 @@ using TestAcceptedVote = voting::AcceptedVote<TestVote>; ...@@ -24,6 +24,7 @@ using TestAcceptedVote = voting::AcceptedVote<TestVote>;
using DummyVoter = voting::test::DummyVoter<TestVote>; using DummyVoter = voting::test::DummyVoter<TestVote>;
using DummyVoteConsumer = voting::test::DummyVoteConsumer<TestVote>; using DummyVoteConsumer = voting::test::DummyVoteConsumer<TestVote>;
using DummyVoteObserver = voting::test::DummyVoteObserver<TestVote>;
// Some dummy contexts. // Some dummy contexts.
const void* kDummyContext1 = reinterpret_cast<const void*>(0xDEADBEEF); const void* kDummyContext1 = reinterpret_cast<const void*>(0xDEADBEEF);
...@@ -181,4 +182,20 @@ TEST(VotingTest, OverwriteVoteReceipt) { ...@@ -181,4 +182,20 @@ TEST(VotingTest, OverwriteVoteReceipt) {
consumer.ExpectInvalidVote(0); consumer.ExpectInvalidVote(0);
} }
TEST(VotingTest, VoteObserver) {
DummyVoteObserver observer;
TestVotingChannel voting_channel =
observer.vote_consumer_default_impl_.BuildVotingChannel();
voting::VoterId<TestVote> voter_id = voting_channel.voter_id();
{
TestVoteReceipt receipt =
voting_channel.SubmitVote(kDummyContext1, TestVote(5, kReason));
EXPECT_TRUE(observer.HasVote(voter_id, kDummyContext1, 5, kReason));
}
EXPECT_FALSE(observer.HasVote(voter_id, kDummyContext1, 5, kReason));
}
} // namespace performance_manager } // namespace performance_manager
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment