PM: Eliminate or localize and test reinterpret_cast use.

Change-Id: I9bf7d0b0f5cbd33ded4ff55bd3b11bdf3c380376
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2507509
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822188}

components/performance_manager/execution_context/execution_context_impl.cc

@@ -77,15 +77,11 @@ class ExecutionContextImpl : public ExecutionContext,
 
   const FrameNode* GetFrameNode() const override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-    if (std::is_same<FrameNodeImpl, NodeImplType>::value)
-      return reinterpret_cast<const FrameNode*>(node_);
     return nullptr;
   }
 
   const WorkerNode* GetWorkerNode() const override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-    if (std::is_same<WorkerNodeImpl, NodeImplType>::value)
-      return reinterpret_cast<const WorkerNodeImpl*>(node_);
     return nullptr;
   }
 
@@ -120,6 +116,11 @@ class FrameExecutionContext
     return blink::ExecutionContextToken(node_->frame_token());
   }
 
+  const FrameNode* GetFrameNode() const override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    return node_;
+  }
+
  protected:
   friend class NodeAttachedDataImpl<FrameExecutionContext>;
   explicit FrameExecutionContext(const FrameNodeImpl* frame_node)
@@ -143,6 +144,11 @@ class WorkerExecutionContext
     return ToExecutionContextToken(node_->worker_token());
   }
 
+  const WorkerNode* GetWorkerNode() const override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    return node_;
+  }
+
  protected:
   friend class NodeAttachedDataImpl<WorkerExecutionContext>;
   explicit WorkerExecutionContext(const WorkerNodeImpl* worker_node)

components/performance_manager/execution_context_priority/max_vote_aggregator.cc

@@ -10,6 +10,14 @@
 namespace performance_manager {
 namespace execution_context_priority {
 
+// static
+MaxVoteAggregator::StampedVote*
+MaxVoteAggregator::StampedVote::FromAcceptedVote(AcceptedVote* accepted_vote) {
+  static_assert(offsetof(StampedVote, vote) == 0,
+                "AcceptedVote is expected to be at offset 0 of StampedVote");
+  return reinterpret_cast<StampedVote*>(accepted_vote);
+}
+
 MaxVoteAggregator::MaxVoteAggregator() : factory_(this) {}
 
 MaxVoteAggregator::~MaxVoteAggregator() = default;
@@ -138,9 +146,7 @@ bool MaxVoteAggregator::VoteData::RemoveVote(size_t index) {
 }
 
 size_t MaxVoteAggregator::VoteData::GetVoteIndex(AcceptedVote* vote) {
-  static_assert(offsetof(StampedVote, vote) == 0,
-                "AcceptedVote is expected to be at offset 0 of StampedVote");
-  StampedVote* stamped_vote = reinterpret_cast<StampedVote*>(vote);
+  StampedVote* stamped_vote = StampedVote::FromAcceptedVote(vote);
   DCHECK_NE(0u, votes_.size());
   DCHECK_LE(votes_.data(), stamped_vote);
   DCHECK_LT(stamped_vote, votes_.data() + votes_.size());

components/performance_manager/execution_context_priority/max_vote_aggregator_unittest.cc

@@ -15,8 +15,10 @@ namespace execution_context_priority {
 class MaxVoteAggregatorTestAccess {
  public:
   using VoteData = MaxVoteAggregator::VoteData;
+  using StampedVote = MaxVoteAggregator::StampedVote;
 };
 using VoteData = MaxVoteAggregatorTestAccess::VoteData;
+using StampedVote = MaxVoteAggregatorTestAccess::StampedVote;
 
 namespace {
 
@@ -76,6 +78,12 @@ class FakeVoteConsumer : public test::DummyVoteConsumer {
 
 }  // namespace
 
+TEST(MaxVoteAggregatorTest, StampedVoteCast) {
+  StampedVote stamped_vote;
+
+  EXPECT_EQ(&stamped_vote, StampedVote::FromAcceptedVote(&stamped_vote.vote));
+}
+
 TEST(MaxVoteAggregatorTest, VoteDataHeapStressTest) {
   // Build a simple consumer/voter chain so that we generate an actual VoterId.
   FakeVoteConsumer consumer;

components/performance_manager/public/execution_context_priority/max_vote_aggregator.h

@@ -61,6 +61,10 @@ class MaxVoteAggregator : public VoteConsumer {
       return vote_id > rhs.vote_id;
     }
 
+    // Given an AcceptedVote that's embedded in a StampedVote::vote, retrieve
+    // the embedding StampedVote instance.
+    static StampedVote* FromAcceptedVote(AcceptedVote* accepted_vote);
+
     // IntrusiveHeap contract. We actually don't need HeapHandles, as we already
     // know the positions of the elements in the heap directly, as they are
     // tracked with explicit back pointers.