Treat files downloaded from the address bar as "always safe" (includi…

…ng extensions per discussion with asargent and the extensions folks).

This required plumbing the PageTransition::Type from render_view.cc back up through various layers to the download system, as well as adding an extra state qualifier bit on the Type to mark navigations triggered "FROM_ADDRESS_BAR" (since the Type itself sans-qualifier cannot be used to reliably check this).

This also fixes an inconsistency in IsDangerousFile() where "auto-open" lowered our safety checks for Dangerous files but not for AllowOnUserGesture files.

BUG=87192,92345
TEST=Paste the PDF link from bug 87192 comment 0 into your address bar and hit enter.  The file should download without triggering any warning UI in the download shelf.
Review URL: http://codereview.chromium.org/7624031

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@98897 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/download/chrome_download_manager_delegate.cc

@@ -257,7 +257,8 @@ void ChromeDownloadManagerDelegate::DownloadProgressUpdated() {
 }
 
 void ChromeDownloadManagerDelegate::CheckDownloadUrlDone(
-    int32 download_id, bool is_dangerous_url) {
+    int32 download_id,
+    bool is_dangerous_url) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DownloadItem* download =
@@ -268,10 +269,8 @@ void ChromeDownloadManagerDelegate::CheckDownloadUrlDone(
   if (is_dangerous_url)
     download->MarkUrlDangerous();
 
-  download_history_->CheckVisitedReferrerBefore(
-      download_id,
-      download->referrer_url(),
-      NewCallback(this,
+  download_history_->CheckVisitedReferrerBefore(download_id,
+      download->referrer_url(), NewCallback(this,
           &ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone));
 }
 
@@ -474,25 +473,31 @@ bool ChromeDownloadManagerDelegate::IsDangerousFile(
     bool visited_referrer_before) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  bool auto_open = ShouldOpenFileBasedOnExtension(state.suggested_path);
-  download_util::DownloadDangerLevel danger_level =
-      download_util::GetFileDangerLevel(state.suggested_path.BaseName());
-
-  if (danger_level == download_util::Dangerous)
-    return !(auto_open && state.has_user_gesture);
-
-  if (danger_level == download_util::AllowOnUserGesture &&
-      (!state.has_user_gesture || !visited_referrer_before))
-    return true;
+  // Anything loaded directly from the address bar is OK.
+  if (state.transition_type & PageTransition::FROM_ADDRESS_BAR)
+    return false;
 
+  // Extensions that are not from the gallery are considered dangerous.
   if (IsExtensionDownload(&download)) {
-    // Extensions that are not from the gallery are considered dangerous.
     ExtensionService* service = profile_->GetExtensionService();
     if (!service || !service->IsDownloadFromGallery(download.GetURL(),
                                                     download.referrer_url()))
       return true;
   }
-  return false;
+
+  // Anything the user has marked auto-open is OK if it's user-initiated.
+  if (ShouldOpenFileBasedOnExtension(state.suggested_path) &&
+      state.has_user_gesture)
+    return false;
+
+  // "Allow on user gesture" is OK when we have a user gesture and the hosting
+  // page has been visited before today.
+  download_util::DownloadDangerLevel danger_level =
+      download_util::GetFileDangerLevel(state.suggested_path.BaseName());
+  if (danger_level == download_util::AllowOnUserGesture)
+    return !state.has_user_gesture || !visited_referrer_before;
+
+  return danger_level == download_util::Dangerous;
 }
 
 void ChromeDownloadManagerDelegate::OnItemAddedToPersistentStore(

chrome/browser/extensions/extension_webnavigation_api.cc

@@ -108,6 +108,8 @@ void DispatchOnCommitted(TabContents* tab_contents,
     qualifiers->Append(Value::CreateStringValue("server_redirect"));
   if (transition_type & PageTransition::FORWARD_BACK)
     qualifiers->Append(Value::CreateStringValue("forward_back"));
+  if (transition_type & PageTransition::FROM_ADDRESS_BAR)
+    qualifiers->Append(Value::CreateStringValue("from_address_bar"));
   dict->Set(keys::kTransitionQualifiersKey, qualifiers);
   dict->SetDouble(keys::kTimeStampKey, MilliSecondsFromTime(base::Time::Now()));
   args.Append(dict);

chrome/browser/extensions/user_script_listener_unittest.cc

@@ -90,8 +90,8 @@ class DummyResourceHandler : public ResourceHandler {
 ResourceDispatcherHostRequestInfo* CreateRequestInfo(int request_id) {
   return new ResourceDispatcherHostRequestInfo(
       new DummyResourceHandler(), ChildProcessInfo::RENDER_PROCESS, 0, 0, 0,
-      request_id, false, -1, ResourceType::MAIN_FRAME, 0, false, false, false,
-      content::MockResourceContext::GetInstance());
+      request_id, false, -1, ResourceType::MAIN_FRAME, PageTransition::LINK, 0,
+      false, false, false, content::MockResourceContext::GetInstance());
 }
 
 // A simple test net::URLRequestJob. We don't care what it does, only that

chrome/browser/metrics/metrics_service.cc

@@ -282,9 +282,9 @@ struct MetricsService::ChildProcessStats {
   // an index for which no value has been assigned.
   ChildProcessStats()
       : process_launches(0),
-      process_crashes(0),
-      instances(0),
-      process_type(ChildProcessInfo::UNKNOWN_PROCESS) {}
+        process_crashes(0),
+        instances(0),
+        process_type(ChildProcessInfo::UNKNOWN_PROCESS) {}
 
   // The number of times that the given child process has been launched
   int process_launches;

chrome/browser/ui/browser_navigator.cc

@@ -369,8 +369,13 @@ void Navigate(NavigateParams* params) {
   // inform the target TabContents, and we may need to update the UI.
   PageTransition::Type base_transition =
       PageTransition::StripQualifier(params->transition);
-  bool user_initiated = base_transition == PageTransition::TYPED ||
-                        base_transition == PageTransition::AUTO_BOOKMARK;
+  bool user_initiated = params->transition & PageTransition::FROM_ADDRESS_BAR ||
+      base_transition == PageTransition::TYPED ||
+      base_transition == PageTransition::AUTO_BOOKMARK ||
+      base_transition == PageTransition::GENERATED ||
+      base_transition == PageTransition::START_PAGE ||
+      base_transition == PageTransition::RELOAD ||
+      base_transition == PageTransition::KEYWORD;
 
   // Check if this is a singleton tab that already exists
   int singleton_index = GetIndexOfSingletonTab(params);

chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm

@@ -91,7 +91,7 @@ new KeywordHintDecoration(OmniboxViewMac::GetFieldFont())),
       profile_(profile),
       browser_(browser),
       toolbar_model_(toolbar_model),
-      transition_(PageTransition::TYPED),
+      transition_(PageTransition::TYPED | PageTransition::FROM_ADDRESS_BAR),
       first_run_bubble_(this) {
   for (size_t i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i) {
     DCHECK_EQ(i, content_setting_decorations_.size());
@@ -226,7 +226,7 @@ new KeywordHintDecoration(OmniboxViewMac::GetFieldFont())),
   if (url.is_valid()) {
     location_input_ = UTF8ToUTF16(url.spec());
     disposition_ = disposition;
-    transition_ = transition;
+    transition_ = transition | PageTransition::FROM_ADDRESS_BAR;
 
     if (command_updater_) {
       if (!alternate_nav_url.is_valid()) {

chrome/browser/ui/gtk/location_bar_view_gtk.cc

@@ -166,7 +166,7 @@ LocationBarViewGtk::LocationBarViewGtk(Browser* browser)
       toolbar_model_(browser->toolbar_model()),
       browser_(browser),
       disposition_(CURRENT_TAB),
-      transition_(PageTransition::TYPED),
+      transition_(PageTransition::TYPED | PageTransition::FROM_ADDRESS_BAR),
       first_run_bubble_(this),
       popup_window_mode_(false),
       theme_service_(NULL),
@@ -467,7 +467,7 @@ void LocationBarViewGtk::OnAutocompleteAccept(const GURL& url,
   if (url.is_valid()) {
     location_input_ = UTF8ToUTF16(url.spec());
     disposition_ = disposition;
-    transition_ = transition;
+    transition_ = transition | PageTransition::FROM_ADDRESS_BAR;
 
     if (command_updater_) {
       if (!alternate_nav_url.is_valid()) {

chrome/browser/ui/views/location_bar/location_bar_view.cc

@@ -111,7 +111,7 @@ LocationBarView::LocationBarView(Browser* browser,
       model_(model),
       delegate_(delegate),
       disposition_(CURRENT_TAB),
-      transition_(PageTransition::LINK),
+      transition_(PageTransition::TYPED | PageTransition::FROM_ADDRESS_BAR),
       location_icon_view_(NULL),
       ev_bubble_view_(NULL),
       location_entry_view_(NULL),
@@ -806,7 +806,7 @@ void LocationBarView::OnAutocompleteAccept(
   if (url.is_valid()) {
     location_input_ = UTF8ToUTF16(url.spec());
     disposition_ = disposition;
-    transition_ = transition;
+    transition_ = transition | PageTransition::FROM_ADDRESS_BAR;
 
     if (browser_->command_updater()) {
       if (!alternate_nav_url.is_valid()) {

chrome/common/extensions/api/extension_api.json

@@ -5222,7 +5222,7 @@
               "url": {"type": "string"},
               "frameId": {"type": "integer", "description": "0 indicates the navigation happens in the tab content window; a positive value indicates navigation in a subframe. Frame IDs are unique within a tab."},
               "transitionType": {"type": "string", "enum": ["link", "typed", "auto_bookmark", "auto_subframe", "manual_subframe", "generated", "start_page", "form_submit", "reload", "keyword", "keyword_generated"], "description": "Cause of the navigation. The same transition types as defined in the history API are used."},
-              "transitionQualifiers": {"type": "array", "description": "A list of transition qualifiers.", "items:": {"type": "string", "enum": ["client_redirect", "server_redirect", "forward_back"]}},
+              "transitionQualifiers": {"type": "array", "description": "A list of transition qualifiers.", "items:": {"type": "string", "enum": ["client_redirect", "server_redirect", "forward_back", "from_address_bar"]}},
               "timeStamp": {"type": "number", "description": "The time when the navigation was committed, in milliseconds since the epoch."}
             }
           }

chrome/common/extensions/docs/experimental.clear.html

@@ -471,12 +471,10 @@ <h4>Parameters</h4>
 
               </em>
           </dt>
-          <dd class="todo">
+          <dd class="todo" style="display: none; ">
             Undocumented.
           </dd>
-          <dd style="display: none; ">
-            Description of this parameter from the json schema.
-          </dd>
+          <dd>An object whose properties specify which browsing data types ought to be cleared. You may set as many or as few as you like in a single call, each is optional (defaulting to <code>false</code>).</dd>
           <dd style="display: none; ">
             This parameter was added in version
             <b><span></span></b>.
@@ -2246,7 +2244,7 @@ <h4>event name</h4>
 
               <div class="summary">
                 <!-- Note: intentionally longer 80 columns -->
-                <span class="subdued">chrome.bookmarks</span><span>onEvent</span><span class="subdued">.addListener</span>(function(<span>Type param1, Type param2</span>) <span class="subdued">{...}</span><span>, Type opt_param1, Type opt_param2</span>));
+                <span class="subdued">chrome.bookmarks</span><span>onEvent</span><span class="subdued">.addListener</span>(function(<span>Type param1, Type param2</span>) <span class="subdued">{...}</span><span>, Type opt_param1, Type opt_param2</span>);
               </div>
 
               <div class="description">

content/browser/download/download_create_info.cc

@@ -16,7 +16,8 @@ DownloadCreateInfo::DownloadCreateInfo(const FilePath& path,
                                        int64 total_bytes,
                                        int32 state,
                                        int32 download_id,
-                                       bool has_user_gesture)
+                                       bool has_user_gesture,
+                                       PageTransition::Type transition_type)
     : path(path),
       url_chain(1, url),
       path_uniquifier(0),
@@ -26,6 +27,7 @@ DownloadCreateInfo::DownloadCreateInfo(const FilePath& path,
       state(state),
       download_id(download_id),
       has_user_gesture(has_user_gesture),
+      transition_type(transition_type),
       db_handle(0),
       prompt_user_for_save_location(false) {
 }
@@ -37,6 +39,7 @@ DownloadCreateInfo::DownloadCreateInfo()
       state(-1),
       download_id(-1),
       has_user_gesture(false),
+      transition_type(PageTransition::LINK),
       db_handle(0),
       prompt_user_for_save_location(false) {
 }

content/browser/download/download_create_info.h

@@ -14,6 +14,7 @@
 #include "base/time.h"
 #include "content/browser/download/download_file.h"
 #include "content/browser/download/download_request_handle.h"
+#include "content/common/page_transition_types.h"
 #include "googleurl/src/gurl.h"
 
 // Used for informing the download manager of a new download, since we don't
@@ -26,7 +27,8 @@ struct DownloadCreateInfo {
                      int64 total_bytes,
                      int32 state,
                      int32 download_id,
-                     bool has_user_gesture);
+                     bool has_user_gesture,
+                     PageTransition::Type transition_type);
   DownloadCreateInfo();
   ~DownloadCreateInfo();
 
@@ -68,6 +70,8 @@ struct DownloadCreateInfo {
   // True if the download was initiated by user action.
   bool has_user_gesture;
 
+  PageTransition::Type transition_type;
+
   // The handle to the download request information.  Used for operations
   // outside the download system.
   DownloadRequestHandle request_handle;

content/browser/download/download_item.cc

@@ -152,8 +152,9 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
                            const DownloadCreateInfo& info,
                            bool is_otr)
     : state_info_(info.original_name, info.save_info.file_path,
-                  info.has_user_gesture, info.prompt_user_for_save_location,
-                  info.path_uniquifier, false, false),
+                  info.has_user_gesture, info.transition_type,
+                  info.prompt_user_for_save_location, info.path_uniquifier,
+                  false, false),
       request_handle_(info.request_handle),
       download_id_(info.download_id),
       full_path_(info.path),

content/browser/download/download_resource_handler.cc

@@ -80,15 +80,12 @@ bool DownloadResourceHandler::OnResponseStarted(int request_id,
   download_id_ = download_file_manager_->GetNextId();
 
   // Deleted in DownloadManager.
-  DownloadCreateInfo* info = new DownloadCreateInfo;
+  DownloadCreateInfo* info = new DownloadCreateInfo(FilePath(), GURL(),
+      base::Time::Now(), 0, content_length_, DownloadItem::IN_PROGRESS,
+      download_id_, request_info->has_user_gesture(),
+      request_info->transition_type());
   info->url_chain = request_->url_chain();
   info->referrer_url = GURL(request_->referrer());
-  info->start_time = base::Time::Now();
-  info->received_bytes = 0;
-  info->total_bytes = content_length_;
-  info->state = DownloadItem::IN_PROGRESS;
-  info->download_id = download_id_;
-  info->has_user_gesture = request_info->has_user_gesture();
   info->request_handle = DownloadRequestHandle(rdh_,
                                                global_id_.child_id,
                                                render_view_id_,

content/browser/download/download_state_info.cc

@@ -9,6 +9,7 @@
 DownloadStateInfo::DownloadStateInfo()
     : path_uniquifier(0),
       has_user_gesture(false),
+      transition_type(PageTransition::LINK),
       prompt_user_for_save_location(false),
       is_dangerous_file(false),
       is_dangerous_url(false) {
@@ -19,6 +20,7 @@ DownloadStateInfo::DownloadStateInfo(
     bool prompt_user_for_save_location)
     : path_uniquifier(0),
       has_user_gesture(has_user_gesture),
+      transition_type(PageTransition::LINK),
       prompt_user_for_save_location(prompt_user_for_save_location),
       is_dangerous_file(false),
       is_dangerous_url(false) {
@@ -28,13 +30,15 @@ DownloadStateInfo::DownloadStateInfo(
     const FilePath& target,
     const FilePath& forced_name,
     bool has_user_gesture,
+    PageTransition::Type transition_type,
     bool prompt_user_for_save_location,
     int uniquifier,
     bool dangerous_file,
     bool dangerous_url)
     : target_name(target),
       path_uniquifier(uniquifier),
       has_user_gesture(has_user_gesture),
+      transition_type(transition_type),
       prompt_user_for_save_location(prompt_user_for_save_location),
       is_dangerous_file(dangerous_file),
       is_dangerous_url(dangerous_url),

content/browser/download/download_state_info.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include "base/file_path.h"
+#include "content/common/page_transition_types.h"
 
 // Contains information relating to the process of determining what to do with
 // the download.
@@ -17,6 +18,7 @@ struct DownloadStateInfo {
   DownloadStateInfo(const FilePath& target,
                     const FilePath& forced_name,
                     bool has_user_gesture,
+                    PageTransition::Type transition_type,
                     bool prompt_user_for_save_location,
                     int uniquifier,
                     bool dangerous_file,
@@ -39,6 +41,8 @@ struct DownloadStateInfo {
   // True if the download is the result of user action.
   bool has_user_gesture;
 
+  PageTransition::Type transition_type;
+
   // True if we should display the 'save as...' UI and prompt the user
   // for the download location.
   // False if the UI should be suppressed and the download performed to the

content/browser/renderer_host/resource_dispatcher_host.cc

@@ -589,6 +589,7 @@ void ResourceDispatcherHost::BeginRequest(
           request_data.is_main_frame,
           request_data.frame_id,
           request_data.resource_type,
+          request_data.transition_type,
           upload_size,
           false,  // is download
           ResourceType::IsFrame(request_data.resource_type),  // allow_download
@@ -730,8 +731,7 @@ void ResourceDispatcherHost::OnFollowRedirect(
                          new_first_party_for_cookies);
 }
 
-ResourceDispatcherHostRequestInfo*
-ResourceDispatcherHost::CreateRequestInfoForBrowserRequest(
+ResourceDispatcherHostRequestInfo* ResourceDispatcherHost::CreateRequestInfo(
     ResourceHandler* handler,
     int child_id,
     int route_id,
@@ -746,6 +746,7 @@ ResourceDispatcherHost::CreateRequestInfoForBrowserRequest(
                                                false,     // is_main_frame
                                                -1,        // frame_id
                                                ResourceType::SUB_RESOURCE,
+                                               PageTransition::LINK,
                                                0,         // upload_size
                                                download,  // is_download
                                                download,  // allow_download
@@ -840,8 +841,7 @@ void ResourceDispatcherHost::BeginDownload(
       net::LOAD_IS_DOWNLOAD);
 
   ResourceDispatcherHostRequestInfo* extra_info =
-      CreateRequestInfoForBrowserRequest(
-          handler, child_id, route_id, true, context);
+      CreateRequestInfo(handler, child_id, route_id, true, context);
   SetRequestInfo(request, extra_info);  // Request takes ownership.
 
   BeginRequestInternal(request);
@@ -885,8 +885,7 @@ void ResourceDispatcherHost::BeginSaveFile(
 
   // Since we're just saving some resources we need, disallow downloading.
   ResourceDispatcherHostRequestInfo* extra_info =
-      CreateRequestInfoForBrowserRequest(
-          handler, child_id, route_id, false, context);
+      CreateRequestInfo(handler, child_id, route_id, false, context);
   SetRequestInfo(request, extra_info);  // Request takes ownership.
 
   BeginRequestInternal(request);