Prefer opening PDF downloads in the browser.

chrome/browser/download/download_target_determiner.cc

Index: chrome/browser/download/download_target_determiner.cc
diff --git a/chrome/browser/download/download_target_determiner.cc b/chrome/browser/download/download_target_determiner.cc
index 7291336364e08ed85d6393a12df346948e3e32ae..899e9a14c45fc70976eca3f94bf4f2e6333ff258 100644
--- a/chrome/browser/download/download_target_determiner.cc
+++ b/chrome/browser/download/download_target_determiner.cc
@@ -23,9 +23,16 @@
 #include "content/public/browser/download_interrupt_reasons.h"
 #include "extensions/common/constants.h"
 #include "grit/generated_resources.h"
+#include "net/base/mime_util.h"
 #include "net/base/net_util.h"
 #include "ui/base/l10n/l10n_util.h"
 
+#if defined(ENABLE_PLUGINS)
+#include "chrome/browser/plugins/plugin_prefs.h"
+#include "content/public/browser/plugin_service.h"
+#include "content/public/common/webplugininfo.h"
+#endif
+
 using content::BrowserThread;
 using content::DownloadItem;
 
@@ -49,8 +56,51 @@ void VisitCountsToVisitedBefore(
       (first_visit.LocalMidnight() < base::Time::Now().LocalMidnight()));
 }
 
+#if defined(ENABLE_PLUGINS)
+typedef std::vector<content::WebPluginInfo> PluginVector;
+
+bool IsSafePluginAvailableForProfile(scoped_ptr<PluginVector> plugins,
+                                     Profile* profile) {
+  using content::WebPluginInfo;
+
+  if (plugins->size() == 0)
+    return false;
+
+  scoped_refptr<PluginPrefs> plugin_prefs = PluginPrefs::GetForProfile(profile);
+  if (!plugin_prefs)
+    return false;
+
+  for (PluginVector::iterator plugin = plugins->begin();
+       plugin != plugins->end(); ++plugin) {
+    if (plugin_prefs->IsPluginEnabled(*plugin) &&
+        (plugin->type == WebPluginInfo::PLUGIN_TYPE_PEPPER_IN_PROCESS ||
+         plugin->type == WebPluginInfo::PLUGIN_TYPE_PEPPER_OUT_OF_PROCESS))
+      return true;
+  }
+  return false;
+}
+
+base::Callback<bool(Profile*)> GetPluginListVerifier(
+    const std::string& mime_type) {
+  DCHECK(!mime_type.empty());
+
+  scoped_ptr<PluginVector> plugins(new PluginVector);
+  content::PluginService* plugin_service =
+      content::PluginService::GetInstance();
+  if (plugin_service)
+    plugin_service->GetPluginInfoArray(
+        GURL(), mime_type, false, plugins.get(), NULL);
+  return base::Bind(&IsSafePluginAvailableForProfile, base::Passed(&plugins));
+}
+#endif  // ENABLE_PLUGINS

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

I'm a bit torn about this code organization.  On the one hand, it keeps the
plugin logic together.  OTOH, you need to jump around in the file to figure out
what the code's doing.  My belief is that the code would be a bit clearer if you
moved the reference to IsSafePluginVaialbleForProfile() down to
DetermineIfHandledByBrowserDone().  But it's not a strong feeling, and if you
feel differently, feel free to keep this layout.

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

I tried that, but it still seemed a bit confusing. I moved this block down next
to DetermineIfHandledByBrowserDone so that it would at least be less effort to
figure out what the code is doing.

+
 } // namespace
 
+DownloadTargetInfo::DownloadTargetInfo()
+    : is_filetype_handled_securely(false) {}
+
+DownloadTargetInfo::~DownloadTargetInfo() {}
+
 DownloadTargetDeterminerDelegate::~DownloadTargetDeterminerDelegate() {
 }
 
@@ -59,7 +109,7 @@ DownloadTargetDeterminer::DownloadTargetDeterminer(
     const base::FilePath& initial_virtual_path,
     DownloadPrefs* download_prefs,
     DownloadTargetDeterminerDelegate* delegate,
-    const content::DownloadTargetCallback& callback)
+    const CompletionCallback& callback)
     : next_state_(STATE_GENERATE_TARGET_PATH),
       should_prompt_(false),
       should_notify_extensions_(false),
@@ -67,6 +117,7 @@ DownloadTargetDeterminer::DownloadTargetDeterminer(
       conflict_action_(DownloadPathReservationTracker::OVERWRITE),
       danger_type_(download->GetDangerType()),
       virtual_path_(initial_virtual_path),
+      is_filetype_handled_securely_(false),
       download_(download),
       is_resumption_(download_->GetLastReason() !=
                          content::DOWNLOAD_INTERRUPT_REASON_NONE &&
@@ -112,6 +163,12 @@ void DownloadTargetDeterminer::DoLoop() {
       case STATE_DETERMINE_LOCAL_PATH:
         result = DoDetermineLocalPath();
         break;
+      case STATE_DETERMINE_MIME_TYPE:
+        result = DoDetermineMimeType();
+        break;
+      case STATE_DETERMINE_IF_HANDLED_BY_BROWSER:
+        result = DoDetermineIfHandledByBrowser();
+        break;
       case STATE_CHECK_DOWNLOAD_URL:
         result = DoCheckDownloadUrl();
         break;
@@ -305,7 +362,7 @@ DownloadTargetDeterminer::Result
   DCHECK(!virtual_path_.empty());
   DCHECK(local_path_.empty());
 
-  next_state_ = STATE_CHECK_DOWNLOAD_URL;
+  next_state_ = STATE_DETERMINE_MIME_TYPE;
 
   delegate_->DetermineLocalPath(
       download_,
@@ -329,6 +386,68 @@ void DownloadTargetDeterminer::DetermineLocalPathDone(
 }
 
 DownloadTargetDeterminer::Result
+    DownloadTargetDeterminer::DoDetermineMimeType() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!virtual_path_.empty());
+  DCHECK(!local_path_.empty());
+  DCHECK(mime_type_.empty());
+
+  next_state_ = STATE_DETERMINE_IF_HANDLED_BY_BROWSER;
+
+  if (virtual_path_ == local_path_) {
+    delegate_->GetFileMimeType(
+        local_path_,
+        base::Bind(&DownloadTargetDeterminer::DetermineMimeTypeDone,
+                   weak_ptr_factory_.GetWeakPtr()));
+    return QUIT_DOLOOP;
+  }
+  return CONTINUE;
+}
+
+void DownloadTargetDeterminer::DetermineMimeTypeDone(
+    const std::string& mime_type) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DVLOG(20) << "MIME type: " << mime_type;
+  mime_type_ = mime_type;
+  DoLoop();
+}
+
+DownloadTargetDeterminer::Result
+    DownloadTargetDeterminer::DoDetermineIfHandledByBrowser() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!virtual_path_.empty());
+  DCHECK(!local_path_.empty());
+  DCHECK(!is_filetype_handled_securely_);
+
+  next_state_ = STATE_CHECK_DOWNLOAD_URL;
+
+  if (mime_type_.empty())
+    return CONTINUE;
+
+  if (net::IsSupportedMimeType(mime_type_)) {
+    is_filetype_handled_securely_ = true;
+    return CONTINUE;
+  }
+
+  BrowserThread::PostTaskAndReplyWithResult(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&GetPluginListVerifier, mime_type_),
+      base::Bind(&DownloadTargetDeterminer::DetermineIfHandledByBrowserDone,
+                 weak_ptr_factory_.GetWeakPtr()));
+  return QUIT_DOLOOP;
+}
+
+void DownloadTargetDeterminer::DetermineIfHandledByBrowserDone(
+    const base::Callback<bool(Profile*)>& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  is_filetype_handled_securely_ = callback.Run(GetProfile());
+  DVLOG(20) << "Is file type handled securely: "
+            << is_filetype_handled_securely_;
+  DoLoop();
+}
+
+DownloadTargetDeterminer::Result
     DownloadTargetDeterminer::DoCheckDownloadUrl() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!virtual_path_.empty());
@@ -484,15 +603,20 @@ void DownloadTargetDeterminer::ScheduleCallbackAndDeleteSelf() {
             << " Intermediate:" << intermediate_path_.AsUTF8Unsafe()
             << " Should prompt:" << should_prompt_
             << " Danger type:" << danger_type_;
+  scoped_ptr<DownloadTargetInfo> target_info(new DownloadTargetInfo);
+
+  target_info->target_path = local_path_;
+  target_info->target_disposition =
+      (HasPromptedForPath() || should_prompt_
+           ? DownloadItem::TARGET_DISPOSITION_PROMPT
+           : DownloadItem::TARGET_DISPOSITION_OVERWRITE);
+  target_info->danger_type = danger_type_;
+  target_info->intermediate_path = intermediate_path_;
+  target_info->mime_type = mime_type_;
+  target_info->is_filetype_handled_securely = is_filetype_handled_securely_;

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

Long-term thought: One of the goals I had for the refactor that we never got to
was to have sub-structures of DownloadItem "DownloadSourceInfo" (net/url info),
"DownloadTargetInfo" (for file info), + a few others.  If you/we ever do that
refactor, it's possible that the target info struct could be used here.

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

Yup. We can look at it when we revisit the handling of 'Open' and how download
completion is blocked. Currently much of DownloadTargetInfo is concerned with
these.

+
   base::MessageLoop::current()->PostTask(
-      FROM_HERE,
-      base::Bind(completion_callback_,
-                 local_path_,
-                 (HasPromptedForPath() || should_prompt_
-                      ? DownloadItem::TARGET_DISPOSITION_PROMPT
-                      : DownloadItem::TARGET_DISPOSITION_OVERWRITE),
-                 danger_type_,
-                 intermediate_path_));
+      FROM_HERE, base::Bind(completion_callback_, base::Passed(&target_info)));
   completion_callback_.Reset();
   delete this;
 }
@@ -631,12 +755,11 @@ void DownloadTargetDeterminer::OnDownloadDestroyed(
 }
 
 // static
-void DownloadTargetDeterminer::Start(
-    content::DownloadItem* download,
-    const base::FilePath& initial_virtual_path,
-    DownloadPrefs* download_prefs,
-    DownloadTargetDeterminerDelegate* delegate,
-    const content::DownloadTargetCallback& callback) {
+void DownloadTargetDeterminer::Start(content::DownloadItem* download,
+                                     const base::FilePath& initial_virtual_path,
+                                     DownloadPrefs* download_prefs,
+                                     DownloadTargetDeterminerDelegate* delegate,
+                                     const CompletionCallback& callback) {
   // DownloadTargetDeterminer owns itself and will self destruct when the job is
   // complete or the download item is destroyed. The callback is always invoked
   // asynchronously.