Address 2 crashes in download attribution code

chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.h"
 
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
@@ skipping 176 matching lines @@
   CleanUpIpAddresses();
   ScheduleNextCleanUpAfterInterval(
       base::TimeDelta::FromSecondsD(kNavigationFootprintTTLInSecond));
 }
 
 SafeBrowsingNavigationObserverManager::AttributionResult
 SafeBrowsingNavigationObserverManager::IdentifyReferrerChainForDownload(
     const GURL& target_url,
     int target_tab_id,
     int user_gesture_count_limit,
-    std::vector<ReferrerChainEntry>* out_referrer_chain) {
+    ReferrerChain* out_referrer_chain) {
   if (!target_url.is_valid())
     return INVALID_URL;
 
   NavigationEvent* nav_event = FindNavigationEvent(
       target_url,
       GURL(),
       target_tab_id);
   if (!nav_event) {
     // We cannot find a single navigation event related to this download.
     return NAVIGATION_EVENT_NOT_FOUND;
@@ skipping 10 matching lines @@
       &result);
   return result;
 }
 
 SafeBrowsingNavigationObserverManager::AttributionResult
 SafeBrowsingNavigationObserverManager::IdentifyReferrerChainForPPAPIDownload(
     const GURL& initiating_frame_url,
     int tab_id,
     bool has_user_gesture,
     int user_gesture_count_limit,
-    std::vector<ReferrerChainEntry>* out_referrer_chain) {
+    ReferrerChain* out_referrer_chain) {
   if (!initiating_frame_url.is_valid())
     return INVALID_URL;
 
   NavigationEvent* nav_event =
       FindNavigationEvent(initiating_frame_url, GURL(), tab_id);
   if (!nav_event) {
     // We cannot find a single navigation event related to this download.
     return NAVIGATION_EVENT_NOT_FOUND;
   }
 
@@ skipping 151 matching lines @@
   GURL search_url =
       target_url.is_empty() ? target_main_frame_url : target_url;
   auto it = navigation_map_.find(search_url);
   if (it == navigation_map_.end())
     return nullptr;
 
   // Since navigation events are recorded in chronological order, we traverse
   // the vector in reverse order to get the latest match.
   for (auto rit = it->second.rbegin(); rit != it->second.rend(); ++rit) {
     // If tab id is not valid, we only compare url, otherwise we compare both.
-    if (rit->destination_url == search_url)

[Jialiu Lin, 2017/01/09 23:23:04]

not sure why this line slipped through last CL... 
Looks like a rebase went bad. 
There should be at lease a clang warning. 

[Nathan Parker, 2017/01/10 01:25:13]

Weird! At least it's a NOOP. :-)

     if (rit->destination_url == search_url &&
         (target_tab_id == -1 || rit->target_tab_id == target_tab_id)) {
       // If both source_url and source_main_frame_url are empty, and this
       // navigation is not triggered by user, a retargeting navigation probably
       // causes this navigation. In this case, we skip this navigation event and
       // looks for the retargeting navigation event.
       if (rit->source_url.is_empty() && rit->source_main_frame_url.is_empty() &&
           !rit->is_user_initiated) {
         // If there is a server redirection immediately after retargeting, we
         // need to adjust our search url to the original request.
         if (rit->has_server_redirect){
           NavigationEvent* retargeting_nav_event =
               FindNavigationEvent(rit->original_request_url,
                                   GURL(),
                                   rit->target_tab_id);
+          if (!retargeting_nav_event)

[Nathan Parker, 2017/01/10 01:25:13]

Does this imply has_server_redirect was wrong?

[Jialiu Lin, 2017/01/10 02:15:12]

Not necessarily. For some cases, previous NavigationEvent might just been
cleared by the periodic cleanup

+            return nullptr;
           // Adjust retargeting navigation event's attributes.
           retargeting_nav_event->has_server_redirect = true;
           retargeting_nav_event->destination_url = search_url;
           return retargeting_nav_event;
         } else {
           continue;
         }
       } else {
         return &*rit;
       }
     }
   }
   return nullptr;
 }
 
 void SafeBrowsingNavigationObserverManager::AddToReferrerChain(
-    std::vector<ReferrerChainEntry>* referrer_chain,
+    ReferrerChain* referrer_chain,
     NavigationEvent* nav_event,
     ReferrerChainEntry::URLType type) {
-  ReferrerChainEntry referrer_chain_entry;
-  referrer_chain_entry.set_url(nav_event->destination_url.spec());
-  referrer_chain_entry.set_type(type);
+  std::unique_ptr<ReferrerChainEntry> referrer_chain_entry =
+      base::MakeUnique<ReferrerChainEntry>();
+  referrer_chain_entry->set_url(nav_event->destination_url.spec());
+  referrer_chain_entry->set_type(type);
   auto ip_it = host_to_ip_map_.find(nav_event->destination_url.host());
   if (ip_it != host_to_ip_map_.end()) {
     for (ResolvedIPAddress entry : ip_it->second) {
-      referrer_chain_entry.add_ip_addresses(entry.ip);
+      referrer_chain_entry->add_ip_addresses(entry.ip);
     }
   }
   // Since we only track navigation to landing referrer, we will not log the
   // referrer of the landing referrer page.
   if (type != ReferrerChainEntry::LANDING_REFERRER) {
-    referrer_chain_entry.set_referrer_url(nav_event->source_url.spec());
-    referrer_chain_entry.set_referrer_main_frame_url(
+    referrer_chain_entry->set_referrer_url(nav_event->source_url.spec());
+    referrer_chain_entry->set_referrer_main_frame_url(
         nav_event->source_main_frame_url.spec());
   }
-  referrer_chain_entry.set_is_retargeting(nav_event->source_tab_id !=
+  referrer_chain_entry->set_is_retargeting(nav_event->source_tab_id !=
                                           nav_event->target_tab_id);
-  referrer_chain_entry.set_navigation_time_msec(
+  referrer_chain_entry->set_navigation_time_msec(
       nav_event->last_updated.ToJavaTime());
   referrer_chain->push_back(std::move(referrer_chain_entry));
 }
 
 void SafeBrowsingNavigationObserverManager::GetRemainingReferrerChain(
     NavigationEvent* last_nav_event_traced,
     int current_user_gesture_count,
     int user_gesture_count_limit,
-    std::vector<ReferrerChainEntry>* out_referrer_chain,
+    ReferrerChain* out_referrer_chain,
     SafeBrowsingNavigationObserverManager::AttributionResult* out_result) {
 
   while (current_user_gesture_count < user_gesture_count_limit) {
     // Back trace to the next nav_event that was initiated by the user.
     while (!last_nav_event_traced->is_user_initiated) {
       last_nav_event_traced =
           FindNavigationEvent(last_nav_event_traced->source_url,
                               last_nav_event_traced->source_main_frame_url,
                               last_nav_event_traced->source_tab_id);
       if (!last_nav_event_traced)
@@ skipping 26 matching lines @@
 
     AddToReferrerChain(out_referrer_chain, last_nav_event_traced,
                        GetURLTypeAndAdjustAttributionResult(
                           current_user_gesture_count ==
                               user_gesture_count_limit,
                           out_result));
   }
 }
 
 }  // namespace safe_browsing