Prefer opening PDF downloads in the browser.

chrome/browser/download/chrome_download_manager_delegate.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/download/chrome_download_manager_delegate.h"
 
 #include <string>
 
+#include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/prefs/pref_member.h"
 #include "base/prefs/pref_service.h"
 #include "base/rand_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/download/download_completion_blocker.h"
 #include "chrome/browser/download/download_crx_util.h"
 #include "chrome/browser/download/download_file_picker.h"
 #include "chrome/browser/download/download_history.h"
+#include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/download_path_reservation_tracker.h"
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/download/download_service.h"
 #include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/download/download_target_determiner.h"
 #include "chrome/browser/download/save_package_file_picker.h"
 #include "chrome/browser/extensions/api/downloads/downloads_api.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_finder.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/pref_names.h"
 #include "components/user_prefs/pref_registry_syncable.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager.h"
 #include "content/public/browser/notification_source.h"
+#include "content/public/browser/page_navigator.h"
 #include "extensions/common/constants.h"
+#include "net/base/mime_util.h"
+#include "net/base/net_util.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/drive/download_handler.h"
 #include "chrome/browser/chromeos/drive/file_system_util.h"
 #endif
 
 using content::BrowserThread;
 using content::DownloadItem;
 using content::DownloadManager;
 using safe_browsing::DownloadProtectionService;
@@ skipping 101 matching lines @@
   } else {
     // If the URL is malicious, we'll use that as the danger type. The results
     // of the content check, if one is performed, will be ignored.
     danger_type = content::DOWNLOAD_DANGER_TYPE_DANGEROUS_URL;
   }
   callback.Run(danger_type);
 }
 
 #endif  // FULL_SAFE_BROWSING
 
+// Called on the blocking pool to determine the MIME type for |path|.
+void GetMimeTypeAndReplyOnUIThread(
+    const base::FilePath& path,
+    const base::Callback<void(const std::string&)>& callback) {

[Randy Smith (Not in Mondays), 2013/11/04 19:53:26]

Worth an assertion that we're on the IO thread?

[asanka, 2013/11/04 21:20:11]

This is actually called on the blocking pool. The only requirement is that
GetMimeTypeFromFile be allowed to block on IO if the system MIME associations
have to be looked up. This is asserted with an 
base::ThreadRestrictions::AssertIOAllowed() deep in the guts of
GetMimeTypeFromFile(). 

+  std::string mime_type;
+  net::GetMimeTypeFromFile(path, &mime_type);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE, base::Bind(callback, mime_type));
+}
+
+bool IsOpenInBrowserPreferreredForFile(const base::FilePath& path,
+                                       const std::string& mime_type) {
+  if (net::IsSupportedImageMimeType(mime_type))
+    return true;
+
+#if ENABLE_PLUGINS
+  if (path.MatchesExtension(FILE_PATH_LITERAL(".pdf")))

[Randy Smith (Not in Mondays), 2013/11/04 19:53:26]

Doesn't this duplicate the plugin information as to what plugins handle what
mime-types/file types?  Also, won't this break if the user has specifically
disabled the PDF plugin?

[asanka, 2013/11/04 21:20:11]

I'll fiddle with the names to make this clearer, but there are several inputs
into the decision:

- Whether the browser is capable of handling the file type securely: This is
determined in DownloadTargetDeterminer and takes into consideration whether any
sandboxed plugins are available and enabled to handle the file type.

- Whether we want to handle this file in the browser if it is possible to do so:
Not every file that can be handled in the browser should be handled. At least
for now. This decision is made here in IsOpenInBrowserPreferredForFile().

A file will default to being opened in the browser if both of the above are
true.

+    return true;
+#endif
+  return false;
+}
+
 }  // namespace
 
 ChromeDownloadManagerDelegate::ChromeDownloadManagerDelegate(Profile* profile)
     : profile_(profile),
       next_download_id_(content::DownloadItem::kInvalidId),
       download_prefs_(new DownloadPrefs(profile)) {
 }
 
 ChromeDownloadManagerDelegate::~ChromeDownloadManagerDelegate() {
 }
@@ skipping 39 matching lines @@
     const content::DownloadIdCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!profile_->IsOffTheRecord());
   DCHECK_NE(content::DownloadItem::kInvalidId, next_download_id_);
   callback.Run(next_download_id_++);
 }
 
 bool ChromeDownloadManagerDelegate::DetermineDownloadTarget(
     DownloadItem* download,
     const content::DownloadTargetCallback& callback) {
+  DownloadTargetDeterminer::CompletionCallback target_determined_callback =
+      base::Bind(&ChromeDownloadManagerDelegate::OnDownloadTargetDetermined,
+                 this,
+                 download->GetId(),
+                 callback);
   DownloadTargetDeterminer::Start(
       download,
-      GetPlatformDownloadPath(
-          profile_, download, PLATFORM_TARGET_PATH),
+      GetPlatformDownloadPath(profile_, download, PLATFORM_TARGET_PATH),
       download_prefs_.get(),
       this,
-      callback);
+      target_determined_callback);
   return true;
 }
 
 bool ChromeDownloadManagerDelegate::ShouldOpenFileBasedOnExtension(
     const base::FilePath& path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (path.Extension().empty())
     return false;
   // TODO(asanka): This determination is done based on |path|, while
   // ShouldOpenDownload() detects extension downloads based on the
@@ skipping 124 matching lines @@
   // Deletes itself.
   new SavePackageFilePicker(
       web_contents,
       suggested_path,
       default_extension,
       can_save_as_complete,
       download_prefs_.get(),
       callback);
 }
 
+void ChromeDownloadManagerDelegate::OpenDownloadUsingPlatformHandler(
+    DownloadItem* download) {
+  base::FilePath platform_path(
+      GetPlatformDownloadPath(profile_, download, PLATFORM_TARGET_PATH));
+  DCHECK(!platform_path.empty());
+  platform_util::OpenItem(platform_path);
+}
+
 void ChromeDownloadManagerDelegate::OpenDownload(DownloadItem* download) {
   DCHECK_EQ(DownloadItem::COMPLETE, download->GetState());
   if (!download->CanOpenDownload())
     return;
-  base::FilePath platform_path(
-      GetPlatformDownloadPath(profile_, download, PLATFORM_TARGET_PATH));
-  DCHECK(!platform_path.empty());
-  platform_util::OpenItem(platform_path);
+#if !defined(OS_ANDROID)

[Randy Smith (Not in Mondays), 2013/11/04 19:53:26]

Is there a reason not to hide the ifdef in the ShouldPreferOpeningInBrowser()
logic?

[asanka, 2013/11/04 21:20:11]

Moved to IsOpenInBrowserPreferredForFile.

+  if (DownloadItemModel(download).ShouldPreferOpeningInBrowser()) {

[Randy Smith (Not in Mondays), 2013/11/04 19:53:26]

Suggestion: I'm tempted to suggest reversing the text and code inclusion (i.e.
making the OpenDownloadUsingPlatformHandler() the if dependency).  That goes
(somewhat) against the "common path should be outside of if" guideline, but it
will result in a lot less indented code, which might make the code a bit easier
to read.  Not sure what's best here, so just raising the idea.

[asanka, 2013/11/04 21:20:11]

Done.

+    content::WebContents* web_contents = download->GetWebContents();
+    Browser* browser =
+        web_contents ? chrome::FindBrowserWithWebContents(web_contents) : NULL;
+    if (!browser ||
+        !browser->CanSupportWindowFeature(Browser::FEATURE_TABSTRIP))
+      browser = chrome::FindTabbedBrowser(
+          profile_, true, chrome::GetActiveDesktop());
+    if (!browser)
+      browser = chrome::FindOrCreateTabbedBrowser(
+          profile_, chrome::GetActiveDesktop());
+    if (browser) {
+      content::OpenURLParams params(
+          net::FilePathToFileURL(download->GetTargetFilePath()),
+          content::Referrer(),
+          NEW_FOREGROUND_TAB,
+          content::PAGE_TRANSITION_LINK,
+          false);
+      browser->OpenURL(params);
+      return;
+    }
+  }
+#endif
+  OpenDownloadUsingPlatformHandler(download);
 }
 
 void ChromeDownloadManagerDelegate::ShowDownloadInShell(
     DownloadItem* download) {
   if (!download->CanShowInFolder())
     return;
   base::FilePath platform_path(
       GetPlatformDownloadPath(profile_, download, PLATFORM_CURRENT_PATH));
   DCHECK(!platform_path.empty());
   platform_util::ShowItemInFolder(platform_path);
@@ skipping 124 matching lines @@
     service->CheckDownloadUrl(*download,
                               base::Bind(&CheckDownloadUrlDone,
                                          callback,
                                          is_content_check_supported));
     return;
   }
 #endif
   callback.Run(content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
 }
 
+void ChromeDownloadManagerDelegate::GetFileMimeType(
+    const base::FilePath& path,
+    const GetFileMimeTypeCallback& callback) {
+  BrowserThread::PostBlockingPoolTask(
+      FROM_HERE,
+      base::Bind(&GetMimeTypeAndReplyOnUIThread, path, callback));
+}
+
 #if defined(FULL_SAFE_BROWSING)
 void ChromeDownloadManagerDelegate::CheckClientDownloadDone(
     uint32 download_id,
     DownloadProtectionService::DownloadCheckResult result) {
   DownloadItem* item = download_manager_->GetDownload(download_id);
   if (!item || (item->GetState() != DownloadItem::IN_PROGRESS))
     return;
 
   VLOG(2) << __FUNCTION__ << "() download = " << item->DebugString(false)
           << " verdict = " << result;
@@ skipping 43 matching lines @@
                     chrome::NOTIFICATION_CRX_INSTALLER_DONE,
                     source);
 
   scoped_refptr<extensions::CrxInstaller> installer =
       content::Source<extensions::CrxInstaller>(source).ptr();
   content::DownloadOpenDelayedCallback callback =
       crx_installers_[installer.get()];
   crx_installers_.erase(installer.get());
   callback.Run(installer->did_handle_successfully());
 }
+
+void ChromeDownloadManagerDelegate::OnDownloadTargetDetermined(
+    int32 download_id,
+    const content::DownloadTargetCallback& callback,
+    scoped_ptr<DownloadTargetInfo> target_info) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DownloadItem* item = download_manager_->GetDownload(download_id);
+  if (!target_info->target_path.empty() && item &&
+      IsOpenInBrowserPreferreredForFile(target_info->target_path,
+                                        target_info->mime_type) &&
+      target_info->is_filetype_handled_securely)
+    DownloadItemModel(item).SetShouldPreferOpeningInBrowser(true);
+  callback.Run(target_info->target_path,
+               target_info->target_disposition,
+               target_info->danger_type,
+               target_info->intermediate_path);
+}