Address 2 crashes in download attribution code

chrome/browser/safe_browsing/download_protection_service.cc

 // Copyright (c) 2012 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/download_protection_service.h"
 
 #include <stddef.h>
 
 #include <memory>
 
@@ skipping 1816 matching lines @@
   if (!base::FeatureList::IsEnabled(
       SafeBrowsingNavigationObserverManager::kDownloadAttribution) ||
       !navigation_observer_manager_) {
     return;
   }
 
   int download_tab_id = SessionTabHelper::IdForTab(web_contents);
   UMA_HISTOGRAM_BOOLEAN(
       "SafeBrowsing.ReferrerHasInvalidTabID.DownloadAttribution",
       download_tab_id == -1);
-  std::vector<ReferrerChainEntry> attribution_chain;
+  SafeBrowsingNavigationObserverManager::ReferrerChain attribution_chain;
   SafeBrowsingNavigationObserverManager::AttributionResult result =
       navigation_observer_manager_->IdentifyReferrerChainForDownload(
           download_url,
           download_tab_id,
           kDownloadAttributionUserGestureLimit,
           &attribution_chain);
   UMA_HISTOGRAM_COUNTS_100(
       "SafeBrowsing.ReferrerURLChainSize.DownloadAttribution",
       attribution_chain.size());
   UMA_HISTOGRAM_ENUMERATION(
       "SafeBrowsing.ReferrerAttributionResult.DownloadAttribution", result,
       SafeBrowsingNavigationObserverManager::ATTRIBUTION_FAILURE_TYPE_MAX);
-  for (auto entry : attribution_chain)
-    out_request->add_referrer_chain()->Swap(&entry);
+  for (auto it = attribution_chain.begin(); it != attribution_chain.end(); it++)

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

Could this be

for (auto& entry : attribution_chain) {
  .. Swap(it.release())
}
?

Actually, I think neither is valid... Swap() doesn't take ownership of the
pointer -- it swaps the innards of the proto (internal pointer, I think).

So maybe you'd want

for (auto& entry : attribution_chain) {
  ... Swap(entry.get())
}
and then let attribution_chain go out of scope, so all the unique_ptrs will get
deleted.

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

You're right. Done.

+    out_request->add_referrer_chain()->Swap(it->release());
 }
 
 void DownloadProtectionService::AddReferrerChainToPPAPIClientDownloadRequest(
   const GURL& initiating_frame_url,
   int tab_id,
   bool has_user_gesture,
   ClientDownloadRequest* out_request) {
   if (!base::FeatureList::IsEnabled(
       SafeBrowsingNavigationObserverManager::kDownloadAttribution) ||
       !navigation_observer_manager_) {
     return;
   }
 
   UMA_HISTOGRAM_BOOLEAN(
       "SafeBrowsing.ReferrerHasInvalidTabID.DownloadAttribution",
       tab_id == -1);
-  std::vector<ReferrerChainEntry> attribution_chain;
+  SafeBrowsingNavigationObserverManager::ReferrerChain attribution_chain;
   SafeBrowsingNavigationObserverManager::AttributionResult result =
       navigation_observer_manager_->IdentifyReferrerChainForPPAPIDownload(
           initiating_frame_url,
           tab_id,
           has_user_gesture,
           kDownloadAttributionUserGestureLimit,
           &attribution_chain);
   UMA_HISTOGRAM_COUNTS_100(
       "SafeBrowsing.ReferrerURLChainSize.PPAPIDownloadAttribution",
       attribution_chain.size());
   UMA_HISTOGRAM_ENUMERATION(
       "SafeBrowsing.ReferrerAttributionResult.PPAPIDownloadAttribution", result,
       SafeBrowsingNavigationObserverManager::ATTRIBUTION_FAILURE_TYPE_MAX);
-  for (auto entry : attribution_chain)
-    out_request->add_referrer_chain()->Swap(&entry);
+  for (auto it = attribution_chain.begin(); it != attribution_chain.end(); it++)
+    out_request->add_referrer_chain()->Swap(it->release());
 }
 
 }  // namespace safe_browsing