Wireup SafeBrowsingNavigationObserverManager to help PPAPI download attribution

chrome/browser/file_select_helper.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/file_select_helper.h"
 
 #include <stddef.h>
 
 #include <string>
 #include <utility>
@@ skipping 528 matching lines @@
         base::FilePath::StringType extension =
             default_file_path.ReplaceExtension(extension_in_list)
                 .FinalExtension();
         alternate_extensions.push_back(extension);
       }
     }
   }
 
   GURL requestor_url = params->requestor;
   sb_service->download_protection_service()->CheckPPAPIDownloadRequest(
-      requestor_url, default_file_path, alternate_extensions, profile_,
+      requestor_url, render_frame_host_->GetLastCommittedURL(),
+      WebContents::FromRenderFrameHost(render_frame_host_),

[Nathan Parker, 2017/01/04 19:49:12]

possible idea: Do you want to just pass render_frame_host_?

[Jialiu Lin, 2017/01/04 21:53:35]

I thought of that. There is one risk though, during the checking (since it's a
remote call), the GetLastCommittedURL() might return a different URL, if this
bad frame tries to quickly redirect to another irrelevant site. 

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

web_contents_, but you'd need to be prepared for the possibility that the
web_contents_ may have been destroyed at this point. If so web_contents_ would
be nullptr.

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

That's fine. If web_contents_ is null,  
SessionTabHelper::IdForTab(web_contents_) will return -1. Then identifying
referrer chain will be purely based on url.

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

Yup. But we are still indirecting through a nullptr in the case of
render_frame_host_ here (render_frame_host_->GetLastCommitedURL() will crash).
Both web_contents_ and render_frame_host_ can become nullptr due to a number of
external factors. Unfortunately we need to check and handle these cases each
time we hop back on the UI thread.

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

Agree... I added checks here in case render_frame_host_ is nullptr. Function
WebContents::FromeRenderFrameHost(..) itself will check the validity of
renderframehost passed in.   

+      default_file_path, alternate_extensions, profile_,
       base::Bind(&InterpretSafeBrowsingVerdict,
                  base::Bind(&FileSelectHelper::ProceedWithSafeBrowsingVerdict,
                             this, default_file_path, base::Passed(&params))));
 }
 
 void FileSelectHelper::ProceedWithSafeBrowsingVerdict(
     const base::FilePath& default_file_path,
     std::unique_ptr<content::FileChooserParams> params,
     bool allowed_by_safe_browsing) {
   if (!allowed_by_safe_browsing) {
@@ skipping 143 matching lines @@
 
 // static
 base::FilePath FileSelectHelper::GetSanitizedFileName(
     const base::FilePath& suggested_filename) {
   if (suggested_filename.empty())
     return base::FilePath();
   return net::GenerateFileName(
       GURL(), std::string(), std::string(), suggested_filename.AsUTF8Unsafe(),
       std::string(), l10n_util::GetStringUTF8(IDS_DEFAULT_DOWNLOAD_FILENAME));
 }