Windows: Add an "Open in Adobe Reader" menu item for PDF files in the download shelf.

chrome/browser/download/download_prefs.cc

Index: chrome/browser/download/download_prefs.cc
diff --git a/chrome/browser/download/download_prefs.cc b/chrome/browser/download/download_prefs.cc
index 750e11b5737877a6676eaba00ab0f35cd1a5b396..20235e03571bfe5c3f829e8a7c71db7b7b3dc760 100644
--- a/chrome/browser/download/download_prefs.cc
+++ b/chrome/browser/download/download_prefs.cc
@@ -37,6 +37,10 @@
 #include "chrome/browser/chromeos/file_manager/path_util.h"
 #endif
 
+#if defined(OS_WIN)
+#include "chrome/browser/ui/pdf/adobe_reader_info_win.h"
+#endif
+
 using content::BrowserContext;
 using content::BrowserThread;
 using content::DownloadManager;
@@ -91,9 +95,18 @@ class DefaultDownloadDirectory {
   DISALLOW_COPY_AND_ASSIGN(DefaultDownloadDirectory);
 };
 
-static base::LazyInstance<DefaultDownloadDirectory>
+base::LazyInstance<DefaultDownloadDirectory>
     g_default_download_directory = LAZY_INSTANCE_INITIALIZER;
 
+#if defined(OS_WIN)
+bool g_adobe_reader_up_to_date = false;
+
+// Callback when whether Adobe Reader is up to date is known.
+void OnGotAdobeReaderInfo(bool up_to_date) {
+  g_adobe_reader_up_to_date = up_to_date;
+}
+#endif
+
 }  // namespace
 
 DownloadPrefs::DownloadPrefs(Profile* profile) : profile_(profile) {
@@ -120,12 +133,25 @@ DownloadPrefs::DownloadPrefs(Profile* profile) : profile_(profile) {
   }
 
   // Ensure that the default download directory exists.
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
+  BrowserThread::PostBlockingPoolTask(

[asanka, 2014/06/25 03:55:08]

Shall we leave this on the FILE thread?

The sequencing of the creation on FILE thread is, I believe, intentional. The
download related set of objects are instantiated when the first time something
touches the download system (e.g. because the user started a new download or
because the user opened the chrome://downloads page). Sequencing on the FILE
thread ensures that the tasks that will be queued later will be executed after
this one (e.g. opening the downloads folder) and thus see the default download
folder as existing. Posting on the blocking pool doesn't give us that sequence
restriction. Thus it is easier to reason about things if we stick to the FILE
thread for now.

This predates the blocking pool, and there were plans to consider moving to
sequenced worker pool, but there's no cycles to do the transition.

That said, I'm not sure why this is done for Chrome OS. Maybe the Files app
misbehaves if there's no download folder? It's certainly not a requirement on
other OSs.

[Lei Zhang, 2014/06/26 08:06:12]

Reverted. I generally like to do cleanups along the way, one of which is to
ever-so-slowly get rid of the FILE thread.

+      FROM_HERE,
       base::Bind(base::IgnoreResult(&base::CreateDirectory),
                  GetDefaultDownloadDirectoryForProfile()));
 #endif  // defined(OS_CHROMEOS)
 
+#if defined(OS_WIN)
+  if (IsAdobeReaderDefaultPDFViewer()) {
+    base::PostTaskAndReplyWithResult(
+        BrowserThread::GetBlockingPool(),

[asanka, 2014/06/25 03:55:08]

This also makes the extremely likely but still non-deterministic assumption that
the value would be available by the time we hit
CDMD::IsOpenInBrowserPreferredForFile().

Have you looked at DownloadTargetDeterminer? It is intentionally designed to
take these thread jumps into consideration and put all the cross-thread data
gathering into a predictable sequence. For example, we have a check in there for
looking up the plugin that would handle the target file if it was opened inside
the browser (DownloadTargetDeterminer::DoDetermineIfHandledSafely()). We can
move this logic there as well so that this check is performed deterministically
before its result is used.

[Lei Zhang, 2014/06/26 08:06:12]

Thanks for the suggestion, done.

Yes, I was very sad that I couldn't find the full version number using only the
registry, so I was hoping to do this early on and have the info ready by the
time the user hits the context menu 99% of the time.

+        FROM_HERE,
+        base::Bind(&::IsAdobeReaderUpToDate),
+        base::Bind(&OnGotAdobeReaderInfo));
+  }
+
+  should_open_pdf_in_adobe_reader_ =
+      prefs->GetBoolean(prefs::kOpenPdfDownloadInAdobeReader);
+#endif
+
   // If the download path is dangerous we forcefully reset it. But if we do
   // so we set a flag to make sure we only do it once, to avoid fighting
   // the user if he really wants it on an unsafe place such as the desktop.
@@ -194,8 +220,21 @@ void DownloadPrefs::RegisterProfilePrefs(
       prefs::kSaveFileDefaultDirectory,
       default_download_path,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+#if defined(OS_WIN)
+  registry->RegisterBooleanPref(
+      prefs::kOpenPdfDownloadInAdobeReader,
+      false,
+      user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+#endif
 }
 
+#if defined(OS_WIN)
+// static
+bool DownloadPrefs::IsAdobeReaderUpToDate() {
+  return g_adobe_reader_up_to_date;
+}
+#endif
+
 base::FilePath DownloadPrefs::GetDefaultDownloadDirectoryForProfile() const {
 #if defined(OS_CHROMEOS)
   return file_manager::util::GetDownloadsFolderForProfile(profile_);
@@ -268,6 +307,10 @@ bool DownloadPrefs::IsDownloadPathManaged() const {
 }
 
 bool DownloadPrefs::IsAutoOpenUsed() const {
+#if defined(OS_WIN)
+  if (ShouldOpenPdfInAdobeReader())
+    return true;
+#endif
   return !auto_open_.empty();
 }
 
@@ -278,6 +321,11 @@ bool DownloadPrefs::IsAutoOpenEnabledBasedOnExtension(
     return false;
   DCHECK(extension[0] == base::FilePath::kExtensionSeparator);
   extension.erase(0, 1);
+#if defined(OS_WIN)
+  if (extension == FILE_PATH_LITERAL("pdf") &&
+      g_adobe_reader_up_to_date && ShouldOpenPdfInAdobeReader())
+    return true;
+#endif

[asanka, 2014/06/25 03:55:08]

What should happen here if the Adobe Reader is not up-to-date and .pdf is listed
as an auto open file type?

[Lei Zhang, 2014/06/26 08:06:12]

We currently ignore it because IsOpenInBrowserPreferreredForFile() will return
true for PDFs, right? I imagine we'll continue to do so if Adobe Reader is not
up-to-date.

   return auto_open_.find(extension) != auto_open_.end();
 }
 
@@ -305,7 +353,24 @@ void DownloadPrefs::DisableAutoOpenBasedOnExtension(
   SaveAutoOpenState();
 }
 
+#if defined(OS_WIN)
+void DownloadPrefs::SetShouldOpenPdfInAdobeReader(bool should_open) {
+  if (should_open_pdf_in_adobe_reader_ == should_open)
+    return;
+  should_open_pdf_in_adobe_reader_ = should_open;
+  profile_->GetPrefs()->SetBoolean(prefs::kOpenPdfDownloadInAdobeReader,
+                                   should_open);
+}
+
+bool DownloadPrefs::ShouldOpenPdfInAdobeReader() const {
+  return should_open_pdf_in_adobe_reader_;
+}
+#endif
+
 void DownloadPrefs::ResetAutoOpen() {
+#if defined(OS_WIN)
+  SetShouldOpenPdfInAdobeReader(false);
+#endif
   auto_open_.clear();
   SaveAutoOpenState();
 }