Wireup SafeBrowsingNavigationObserverManager to help PPAPI download attribution

chrome/browser/safe_browsing/download_protection_service.cc

Index: chrome/browser/safe_browsing/download_protection_service.cc
diff --git a/chrome/browser/safe_browsing/download_protection_service.cc b/chrome/browser/safe_browsing/download_protection_service.cc
index cc34930f95ae5da4d718050233bcc952f30bcbf4..861c85f2a7c26c89829f8fce9b65dab4aa4bca7e 100644
--- a/chrome/browser/safe_browsing/download_protection_service.cc
+++ b/chrome/browser/safe_browsing/download_protection_service.cc
@@ -1230,6 +1230,8 @@ class DownloadProtectionService::PPAPIDownloadRequest
 
   PPAPIDownloadRequest(
       const GURL& requestor_url,
+      const GURL& initiating_frame_url,
+      content::WebContents* web_contents,
       const base::FilePath& default_file_path,
       const std::vector<base::FilePath::StringType>& alternate_extensions,
       Profile* profile,
@@ -1237,6 +1239,8 @@ class DownloadProtectionService::PPAPIDownloadRequest
       DownloadProtectionService* service,
       scoped_refptr<SafeBrowsingDatabaseManager> database_manager)
       : requestor_url_(requestor_url),
+        initiating_frame_url_(initiating_frame_url),
+        web_contents_(web_contents),

[asanka, 2017/01/05 18:00:44]

not safe to store web_contents without observing it. It might get destroyed
while PPAPIDownloadRequest is thread-hopping.

If possible, considering capturing the attribution chain here instead of waiting
until later.

[Jialiu Lin, 2017/01/05 20:12:21]

Though my current code can work with null web_contents_, your suggestion is
definitely much better. In my attribution code, web_contents_ is used to
determine two things: (1) tab_id (default -1, if web_contents is not available,
then attribution will be based only on url), (2) whether this web_content_ has
any user gesture (default false).  So I'll compute these two things here instead
of later. Thanks!

[asanka, 2017/01/05 20:35:31]

Cool. Though my worry here was that web_contents_ would point to invalid memory
rather than be nullptr by the time the original code got around to using it. The
lifetime of these objects are quite finicky :-)

[Jialiu Lin, 2017/01/05 22:10:12]

We can probably trust WebContents::FromRenderFrameHost(rfh) (called in
FileSelectHelper::CheckDownloadRequestWithSafeBrowsing) to return the valid
web_contents or nullptr in case rfh is not avaialble. 
And there is no thread-hopping or delay between
CheckDownloadRequestWithSafeBrowsing() and this constructor here.
And now all the uses of web_contents are moved to this constructor. I think
we're quite safe here. 

         default_file_path_(default_file_path),
         alternate_extensions_(alternate_extensions),
         callback_(callback),
@@ -1359,8 +1363,10 @@ class DownloadProtectionService::PPAPIDownloadRequest
           base::FilePath(default_file_path_.FinalExtension()).AsUTF8Unsafe();
     }
 
-    // TODO(676691): We should add reliable download referrer chain for PPAPI
-    // downloads too.
+    service_->AddReferrerChainToPPAPIClientDownloadRequest(
+        initiating_frame_url_,
+        web_contents_,
+        &request);
 
     if (!request.SerializeToString(&client_download_request_data_)) {
       // More of an internal error than anything else. Note that the UNKNOWN
@@ -1484,6 +1490,12 @@ class DownloadProtectionService::PPAPIDownloadRequest
   // URL of document that requested the PPAPI download.
   const GURL requestor_url_;
 
+  // URL of the frame that hosted the PPAPI plugin.
+  const GURL initiating_frame_url_;
+
+  // WebContents that is associated with this PPAPI download.
+  content::WebContents* web_contents_;
+
   // Default download path requested by the PPAPI plugin.
   const base::FilePath default_file_path_;
 
@@ -1614,6 +1626,8 @@ bool DownloadProtectionService::IsSupportedDownload(
 
 void DownloadProtectionService::CheckPPAPIDownloadRequest(
     const GURL& requestor_url,
+    const GURL& initiating_frame_url,
+    content::WebContents* web_contents,
     const base::FilePath& default_file_path,
     const std::vector<base::FilePath::StringType>& alternate_extensions,
     Profile* profile,
@@ -1621,8 +1635,8 @@ void DownloadProtectionService::CheckPPAPIDownloadRequest(
   DVLOG(1) << __func__ << " url:" << requestor_url
            << " default_file_path:" << default_file_path.value();
   std::unique_ptr<PPAPIDownloadRequest> request(new PPAPIDownloadRequest(
-      requestor_url, default_file_path, alternate_extensions, profile, callback,
-      this, database_manager_));
+      requestor_url, initiating_frame_url, web_contents, default_file_path,
+      alternate_extensions, profile, callback, this, database_manager_));
   PPAPIDownloadRequest* request_copy = request.get();
   auto insertion_result = ppapi_download_requests_.insert(
       std::make_pair(request_copy, std::move(request)));
@@ -1808,7 +1822,7 @@ void DownloadProtectionService::AddReferrerChainToClientDownloadRequest(
       download_tab_id == -1);
   std::vector<ReferrerChainEntry> attribution_chain;
   SafeBrowsingNavigationObserverManager::AttributionResult result =
-      navigation_observer_manager_->IdentifyReferrerChain(
+      navigation_observer_manager_->IdentifyReferrerChainForDownload(
           download_url,
           download_tab_id,
           kDownloadAttributionUserGestureLimit,
@@ -1823,4 +1837,31 @@ void DownloadProtectionService::AddReferrerChainToClientDownloadRequest(
     out_request->add_referrer_chain()->Swap(&entry);
 }
 
+void DownloadProtectionService::AddReferrerChainToPPAPIClientDownloadRequest(
+  const GURL& initiating_frame_url,
+  content::WebContents* web_contents,
+  ClientDownloadRequest* out_request) {
+  if (!base::FeatureList::IsEnabled(
+      SafeBrowsingNavigationObserverManager::kDownloadAttribution) ||
+      !navigation_observer_manager_) {
+    return;
+  }
+
+  std::vector<ReferrerChainEntry> attribution_chain;
+  SafeBrowsingNavigationObserverManager::AttributionResult result =
+      navigation_observer_manager_->IdentifyReferrerChainForPPAPIDownload(
+          initiating_frame_url,
+          web_contents,
+          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);
+}
+
 }  // namespace safe_browsing