Move packet events away from linked_ptr in EncodingEventSubscriber.

BUG=137767

Change-Id: I695459f8b60ecdce8d0b476d7eb373464c5e4be7
Reviewed-on: https://chromium-review.googlesource.com/1112775
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Xiangjun Zhang <xjz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570292}

media/cast/logging/encoding_event_subscriber.cc

@@ -149,7 +149,6 @@ void EncodingEventSubscriber::OnReceivePacketEvent(
   uint32_t lower_32_bits = relative_rtp_timestamp.lower_32_bits();
   PacketEventMap::iterator it =
       packet_event_map_.find(relative_rtp_timestamp);
-  linked_ptr<AggregatedPacketEvent> event_proto;
   BasePacketEvent* base_packet_event_proto = nullptr;
 
   // Look up existing entry. If not found, create a new entry and add to map.
@@ -158,18 +157,17 @@ void EncodingEventSubscriber::OnReceivePacketEvent(
       return;
 
     IncrementStoredProtoCount(lower_32_bits);
-    event_proto.reset(new AggregatedPacketEvent);
+    auto event_proto = std::make_unique<AggregatedPacketEvent>();
     event_proto->set_relative_rtp_timestamp(lower_32_bits);
-    packet_event_map_.insert(
-        std::make_pair(relative_rtp_timestamp, event_proto));
     base_packet_event_proto = GetNewBasePacketEvent(
         event_proto.get(), packet_event.packet_id, packet_event.size);
+    packet_event_map_.insert(
+        std::make_pair(relative_rtp_timestamp, std::move(event_proto)));
   } else {
     // Found existing entry, now look up existing BasePacketEvent using packet
     // ID. If not found, create a new entry and add to proto.
-    event_proto = it->second;
     RepeatedPtrField<BasePacketEvent>* field =
-        event_proto->mutable_base_packet_event();
+        it->second->mutable_base_packet_event();
     for (RepeatedPtrField<BasePacketEvent>::pointer_iterator base_it =
              field->pointer_begin();
          base_it != field->pointer_end();
@@ -180,41 +178,39 @@ void EncodingEventSubscriber::OnReceivePacketEvent(
       }
     }
     if (!base_packet_event_proto) {
-      if (event_proto->base_packet_event_size() >= kMaxPacketsPerFrame) {
+      if (it->second->base_packet_event_size() >= kMaxPacketsPerFrame) {
         DVLOG(3) << "Too many packets in AggregatedPacketEvent "
                  << packet_event.rtp_timestamp << ". "
                  << "Using new packet event proto.";
-        AddPacketEventToStorage(event_proto);
+        AddPacketEventToStorage(std::move(it->second));
         if (!ShouldCreateNewProto(lower_32_bits)) {
           packet_event_map_.erase(it);
           return;
         }
 
         IncrementStoredProtoCount(lower_32_bits);
-        event_proto.reset(new AggregatedPacketEvent);
-        event_proto->set_relative_rtp_timestamp(lower_32_bits);
-        it->second = event_proto;
+        it->second = std::make_unique<AggregatedPacketEvent>();
+        it->second->set_relative_rtp_timestamp(lower_32_bits);
       }
 
       base_packet_event_proto = GetNewBasePacketEvent(
-          event_proto.get(), packet_event.packet_id, packet_event.size);
+          it->second.get(), packet_event.packet_id, packet_event.size);
     } else if (base_packet_event_proto->event_type_size() >=
                kMaxEventsPerProto) {
       DVLOG(3) << "Too many events in packet "
                << packet_event.rtp_timestamp << ", "
                << packet_event.packet_id << ". Using new packet event proto.";
-      AddPacketEventToStorage(event_proto);
+      AddPacketEventToStorage(std::move(it->second));
       if (!ShouldCreateNewProto(lower_32_bits)) {
         packet_event_map_.erase(it);
         return;
       }
 
       IncrementStoredProtoCount(lower_32_bits);
-      event_proto.reset(new AggregatedPacketEvent);
-      event_proto->set_relative_rtp_timestamp(lower_32_bits);
-      it->second = event_proto;
+      it->second = std::make_unique<AggregatedPacketEvent>();
+      it->second->set_relative_rtp_timestamp(lower_32_bits);
       base_packet_event_proto = GetNewBasePacketEvent(
-          event_proto.get(), packet_event.packet_id, packet_event.size);
+          it->second.get(), packet_event.packet_id, packet_event.size);
     }
   }
 
@@ -247,7 +243,7 @@ void EncodingEventSubscriber::GetEventsAndReset(LogMetadata* metadata,
   std::sort(frame_event_storage_.begin(), frame_event_storage_.end(),
             &IsRtpTimestampLessThan<std::unique_ptr<AggregatedFrameEvent>>);
   std::sort(packet_event_storage_.begin(), packet_event_storage_.end(),
-            &IsRtpTimestampLessThan<linked_ptr<AggregatedPacketEvent> >);
+            &IsRtpTimestampLessThan<std::unique_ptr<AggregatedPacketEvent>>);
 
   metadata->set_is_audio(event_media_type_ == AUDIO_EVENT);
   metadata->set_first_rtp_timestamp(first_rtp_timestamp_.lower_32_bits());
@@ -278,7 +274,7 @@ void EncodingEventSubscriber::TransferPacketEvents(size_t max_num_entries) {
   for (size_t i = 0;
        i < max_num_entries && it != packet_event_map_.end();
        i++, ++it) {
-    AddPacketEventToStorage(it->second);
+    AddPacketEventToStorage(std::move(it->second));
   }
 
   packet_event_map_.erase(packet_event_map_.begin(), it);
@@ -298,13 +294,13 @@ void EncodingEventSubscriber::AddFrameEventToStorage(
 }
 
 void EncodingEventSubscriber::AddPacketEventToStorage(
-    const linked_ptr<AggregatedPacketEvent>& packet_event_proto) {
+    std::unique_ptr<AggregatedPacketEvent> packet_event_proto) {
   if (packet_event_storage_.size() >= max_frames_) {
     auto& entry = packet_event_storage_[packet_event_storage_index_];
     DecrementStoredProtoCount(entry->relative_rtp_timestamp());
-    entry = packet_event_proto;
+    entry = std::move(packet_event_proto);
   } else {
-    packet_event_storage_.push_back(packet_event_proto);
+    packet_event_storage_.push_back(std::move(packet_event_proto));
   }
 
   packet_event_storage_index_ = (packet_event_storage_index_ + 1) % max_frames_;

media/cast/logging/encoding_event_subscriber.h

@@ -12,7 +12,6 @@
 #include <vector>
 
 #include "base/macros.h"
-#include "base/memory/linked_ptr.h"
 #include "base/threading/thread_checker.h"
 #include "media/cast/logging/logging_defines.h"
 #include "media/cast/logging/proto/raw_events.pb.h"
@@ -36,7 +35,8 @@ static const int kMaxProtosPerFrame = 10;
 
 using FrameEventList =
     std::vector<std::unique_ptr<proto::AggregatedFrameEvent>>;
-using PacketEventList = std::vector<linked_ptr<proto::AggregatedPacketEvent>>;
+using PacketEventList =
+    std::vector<std::unique_ptr<proto::AggregatedPacketEvent>>;
 
 // A RawEventSubscriber implementation that subscribes to events,
 // encodes them in protocol buffer format, and aggregates them into a more
@@ -76,7 +76,7 @@ class EncodingEventSubscriber : public RawEventSubscriber {
   using FrameEventMap =
       std::map<RtpTimeDelta, std::unique_ptr<proto::AggregatedFrameEvent>>;
   using PacketEventMap =
-      std::map<RtpTimeDelta, linked_ptr<proto::AggregatedPacketEvent>>;
+      std::map<RtpTimeDelta, std::unique_ptr<proto::AggregatedPacketEvent>>;
 
   // Transfer up to |max_num_entries| smallest entries from |frame_event_map_|
   // to |frame_event_storage_|. This helps keep size of |frame_event_map_| small
@@ -88,7 +88,7 @@ class EncodingEventSubscriber : public RawEventSubscriber {
   void AddFrameEventToStorage(
       std::unique_ptr<proto::AggregatedFrameEvent> frame_event_proto);
   void AddPacketEventToStorage(
-      const linked_ptr<proto::AggregatedPacketEvent>& packet_event_proto);
+      std::unique_ptr<proto::AggregatedPacketEvent> packet_event_proto);
 
   bool ShouldCreateNewProto(
       uint32_t relative_rtp_timestamp_lower_32_bits) const;

media/cast/logging/encoding_event_subscriber_unittest.cc

@@ -456,7 +456,7 @@ TEST_F(EncodingEventSubscriberTest, PacketEvent) {
 
   PacketEventList::iterator it = packet_events_.begin();
 
-  linked_ptr<AggregatedPacketEvent> event = *it;
+  const AggregatedPacketEvent* event = it->get();
 
   EXPECT_EQ((rtp_timestamp - first_rtp_timestamp_).lower_32_bits(),
             event->relative_rtp_timestamp());
@@ -511,7 +511,7 @@ TEST_F(EncodingEventSubscriberTest, MultiplePacketEventsForPacket) {
 
   PacketEventList::iterator it = packet_events_.begin();
 
-  linked_ptr<AggregatedPacketEvent> event = *it;
+  const AggregatedPacketEvent* event = it->get();
 
   EXPECT_EQ((rtp_timestamp - first_rtp_timestamp_).lower_32_bits(),
             event->relative_rtp_timestamp());
@@ -566,7 +566,7 @@ TEST_F(EncodingEventSubscriberTest, MultiplePacketEventsForFrame) {
 
   PacketEventList::iterator it = packet_events_.begin();
 
-  linked_ptr<AggregatedPacketEvent> event = *it;
+  const AggregatedPacketEvent* event = it->get();
 
   EXPECT_EQ((rtp_timestamp - first_rtp_timestamp_).lower_32_bits(),
             event->relative_rtp_timestamp());
@@ -627,35 +627,40 @@ TEST_F(EncodingEventSubscriberTest, MultiplePacketEvents) {
 
   PacketEventList::iterator it = packet_events_.begin();
 
-  linked_ptr<AggregatedPacketEvent> event = *it;
+  {
+    const AggregatedPacketEvent* event = it->get();
 
-  EXPECT_EQ((rtp_timestamp_1 - first_rtp_timestamp_).lower_32_bits(),
-            event->relative_rtp_timestamp());
+    EXPECT_EQ((rtp_timestamp_1 - first_rtp_timestamp_).lower_32_bits(),
+              event->relative_rtp_timestamp());
 
-  ASSERT_EQ(1, event->base_packet_event_size());
-  const BasePacketEvent& base_event = event->base_packet_event(0);
-  EXPECT_EQ(packet_id_1, base_event.packet_id());
-  ASSERT_EQ(1, base_event.event_type_size());
-  EXPECT_EQ(media::cast::proto::PACKET_SENT_TO_NETWORK,
-            base_event.event_type(0));
-  ASSERT_EQ(1, base_event.event_timestamp_ms_size());
-  EXPECT_EQ(InMilliseconds(now1), base_event.event_timestamp_ms(0));
+    ASSERT_EQ(1, event->base_packet_event_size());
+    const BasePacketEvent& base_event = event->base_packet_event(0);
+    EXPECT_EQ(packet_id_1, base_event.packet_id());
+    ASSERT_EQ(1, base_event.event_type_size());
+    EXPECT_EQ(media::cast::proto::PACKET_SENT_TO_NETWORK,
+              base_event.event_type(0));
+    ASSERT_EQ(1, base_event.event_timestamp_ms_size());
+    EXPECT_EQ(InMilliseconds(now1), base_event.event_timestamp_ms(0));
+  }
 
   ++it;
   ASSERT_TRUE(it != packet_events_.end());
 
-  event = *it;
-  EXPECT_EQ((rtp_timestamp_2 - first_rtp_timestamp_).lower_32_bits(),
-            event->relative_rtp_timestamp());
+  {
+    const AggregatedPacketEvent* event = it->get();
 
-  ASSERT_EQ(1, event->base_packet_event_size());
-  const BasePacketEvent& base_event_2 = event->base_packet_event(0);
-  EXPECT_EQ(packet_id_2, base_event_2.packet_id());
-  ASSERT_EQ(1, base_event_2.event_type_size());
-  EXPECT_EQ(media::cast::proto::PACKET_RETRANSMITTED,
-            base_event_2.event_type(0));
-  ASSERT_EQ(1, base_event_2.event_timestamp_ms_size());
-  EXPECT_EQ(InMilliseconds(now2), base_event_2.event_timestamp_ms(0));
+    EXPECT_EQ((rtp_timestamp_2 - first_rtp_timestamp_).lower_32_bits(),
+              event->relative_rtp_timestamp());
+
+    ASSERT_EQ(1, event->base_packet_event_size());
+    const BasePacketEvent& base_event_2 = event->base_packet_event(0);
+    EXPECT_EQ(packet_id_2, base_event_2.packet_id());
+    ASSERT_EQ(1, base_event_2.event_type_size());
+    EXPECT_EQ(media::cast::proto::PACKET_RETRANSMITTED,
+              base_event_2.event_type(0));
+    ASSERT_EQ(1, base_event_2.event_timestamp_ms_size());
+    EXPECT_EQ(InMilliseconds(now2), base_event_2.event_timestamp_ms(0));
+  }
 }
 
 TEST_F(EncodingEventSubscriberTest, FirstRtpTimeTicks) {
@@ -793,14 +798,17 @@ TEST_F(EncodingEventSubscriberTest, MaxEventsPerProto) {
   PacketEventList::iterator packet_it = packet_events_.begin();
   ASSERT_TRUE(packet_it != packet_events_.end());
 
-  linked_ptr<AggregatedPacketEvent> packet_event = *packet_it;
-
-  EXPECT_EQ(kMaxPacketsPerFrame,
-      packet_event->base_packet_event_size());
+  {
+    const AggregatedPacketEvent* packet_event = packet_it->get();
+    EXPECT_EQ(kMaxPacketsPerFrame, packet_event->base_packet_event_size());
+  }
 
   ++packet_it;
-  packet_event = *packet_it;
-  EXPECT_EQ(1, packet_event->base_packet_event_size());
+
+  {
+    const AggregatedPacketEvent* packet_event = packet_it->get();
+    EXPECT_EQ(1, packet_event->base_packet_event_size());
+  }
 
   for (int j = 0; j < kMaxEventsPerProto + 1; j++) {
     std::unique_ptr<PacketEvent> send_event(new PacketEvent());
@@ -823,14 +831,17 @@ TEST_F(EncodingEventSubscriberTest, MaxEventsPerProto) {
   packet_it = packet_events_.begin();
   ASSERT_TRUE(packet_it != packet_events_.end());
 
-  packet_event = *packet_it;
-
-  EXPECT_EQ(kMaxEventsPerProto,
-      packet_event->base_packet_event(0).event_type_size());
+  {
+    const AggregatedPacketEvent* packet_event = packet_it->get();
+    EXPECT_EQ(kMaxEventsPerProto,
+              packet_event->base_packet_event(0).event_type_size());
+  }
 
   ++packet_it;
-  packet_event = *packet_it;
-  EXPECT_EQ(1, packet_event->base_packet_event(0).event_type_size());
+  {
+    const AggregatedPacketEvent* packet_event = packet_it->get();
+    EXPECT_EQ(1, packet_event->base_packet_event(0).event_type_size());
+  }
 }
 
 }  // namespace cast

media/cast/logging/serialize_deserialize_test.cc

@@ -41,7 +41,7 @@ const int kIdealizedBitratePercentUtilized[] = {9, 9, 9, 15, 36, 38, 35, 40};
 
 const int kMaxSerializedBytes = 10000;
 
-}
+}  // namespace
 
 namespace media {
 namespace cast {
@@ -88,7 +88,7 @@ class SerializeDeserializeTest : public ::testing::Test {
     event_time_ms = 0;
     int packet_id = 0;
     for (int i = 0; i < metadata_.num_packet_events(); i++) {
-      linked_ptr<AggregatedPacketEvent> packet_event(new AggregatedPacketEvent);
+      auto packet_event = std::make_unique<AggregatedPacketEvent>();
       packet_event->set_relative_rtp_timestamp(i * 90);
       for (int j = 0; j < 10; j++) {
         BasePacketEvent* base_event = packet_event->add_base_packet_event();
@@ -102,7 +102,7 @@ class SerializeDeserializeTest : public ::testing::Test {
           event_time_ms += 256;
         }
       }
-      packet_event_list_.push_back(packet_event);
+      packet_event_list_.push_back(std::move(packet_event));
     }
   }