Fix undefined behavior for page_load_metrics resource updates

Currently resources that are modified between timing updates are held
as a set of pointers to PageResourceDataUse objects. However, these
are stored in a small map, which invalidates iterators/pointers on
insertion, causing undefined behavior when using |modified_resources_|.
Instead, store PageResourceDataUse as unique pointers and have
|modified_resources_| reference the raw pointer.

Change-Id: Id7d75343887f9be9b9ee405b5fabc29e635d93e8
Reviewed-on: https://chromium-review.googlesource.com/1224656Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Commit-Queue: John Delaney <johnidel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591124}

chrome/renderer/page_load_metrics/page_timing_metrics_sender.cc

@@ -39,7 +39,7 @@ PageTimingMetricsSender::PageTimingMetricsSender(
   page_resource_data_use_.emplace(
       std::piecewise_construct,
       std::forward_as_tuple(initial_request->resource_id()),
-      std::forward_as_tuple(*initial_request));
+      std::forward_as_tuple(std::move(initial_request)));
   buffer_timer_delay_ms_ = base::GetFieldTrialParamByFeatureAsInt(
       kPageLoadMetricsTimerDelayFeature, "BufferTimerDelayMillis",
       kBufferTimerDelayMillis /* default value */);
@@ -97,15 +97,15 @@ void PageTimingMetricsSender::DidStartResponse(
 
   auto resource_it = page_resource_data_use_.emplace(
       std::piecewise_construct, std::forward_as_tuple(resource_id),
-      std::forward_as_tuple());
-  resource_it.first->second.DidStartResponse(resource_id, response_head);
+      std::forward_as_tuple(std::make_unique<PageResourceDataUse>()));
+  resource_it.first->second->DidStartResponse(resource_id, response_head);
 }
 
 void PageTimingMetricsSender::DidReceiveTransferSizeUpdate(
     int resource_id,
     int received_data_length) {
   // Transfer size updates are called in a throttled manner.
-  const auto& resource_it = page_resource_data_use_.find(resource_id);
+  auto resource_it = page_resource_data_use_.find(resource_id);
 
   // It is possible that resources are not in the map, if response headers were
   // not received or for failed/cancelled resources.
@@ -113,8 +113,8 @@ void PageTimingMetricsSender::DidReceiveTransferSizeUpdate(
     return;
   }
 
-  resource_it->second.DidReceiveTransferSizeUpdate(received_data_length);
-  modified_resources_.insert(&resource_it->second);
+  resource_it->second->DidReceiveTransferSizeUpdate(received_data_length);
+  modified_resources_.insert(resource_it->second.get());
   EnsureSendTimer();
 }
 
@@ -129,14 +129,14 @@ void PageTimingMetricsSender::DidCompleteResponse(
   if (resource_it == page_resource_data_use_.end()) {
     auto new_resource_it = page_resource_data_use_.emplace(
         std::piecewise_construct, std::forward_as_tuple(resource_id),
-        std::forward_as_tuple());
+        std::forward_as_tuple(std::make_unique<PageResourceDataUse>()));
     resource_it = new_resource_it.first;
   }
 
-  if (resource_it->second.DidCompleteResponse(status)) {
+  if (resource_it->second->DidCompleteResponse(status)) {
     EnsureSendTimer();
   }
-  modified_resources_.insert(&resource_it->second);
+  modified_resources_.insert(resource_it->second.get());
 }
 
 void PageTimingMetricsSender::DidCancelResponse(int resource_id) {
@@ -144,7 +144,7 @@ void PageTimingMetricsSender::DidCancelResponse(int resource_id) {
   if (resource_it == page_resource_data_use_.end()) {
     return;
   }
-  resource_it->second.DidCancelResponse();
+  resource_it->second->DidCancelResponse();
 }
 
 void PageTimingMetricsSender::UpdateResourceMetadata(
@@ -157,8 +157,8 @@ void PageTimingMetricsSender::UpdateResourceMetadata(
   // This can get called multiple times for resources, and this
   // flag will only be true once.
   if (reported_as_ad_resource)
-    it->second.SetReportedAsAdResource(reported_as_ad_resource);
-  it->second.SetIsMainFrameResource(is_main_frame_resource);
+    it->second->SetReportedAsAdResource(reported_as_ad_resource);
+  it->second->SetIsMainFrameResource(is_main_frame_resource);
 }
 
 void PageTimingMetricsSender::Send(mojom::PageLoadTimingPtr timing) {

chrome/renderer/page_load_metrics/page_timing_metrics_sender.h

@@ -87,7 +87,7 @@ class PageTimingMetricsSender {
 
   // The page's resources that are currently loading,  or were completed after
   // the last timing update.
-  base::small_map<std::map<int, PageResourceDataUse>, 16>
+  base::small_map<std::map<int, std::unique_ptr<PageResourceDataUse>>, 16>
       page_resource_data_use_;
 
   // Set of all resources that have completed or received a transfer