Make the logic for stream interception by MimeHandlerViews consistent with starting the plugin

chrome/browser/pdf/pdf_extension_test.cc

Index: chrome/browser/pdf/pdf_extension_test.cc
diff --git a/chrome/browser/pdf/pdf_extension_test.cc b/chrome/browser/pdf/pdf_extension_test.cc
index d6718e79b2ec3f81f4daf928376d5ca16ab31b7c..e23ad1cf4eb533bfe4a7b47170539860be50e585 100644
--- a/chrome/browser/pdf/pdf_extension_test.cc
+++ b/chrome/browser/pdf/pdf_extension_test.cc
@@ -2,20 +2,32 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <vector>
+
 #include "base/base_paths.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/hash.h"
+#include "base/logging.h"
+#include "base/memory/ref_counted.h"
 #include "base/path_service.h"
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/component_loader.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/plugins/plugin_prefs.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_content_client.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/browser_plugin_guest_manager.h"
+#include "content/public/browser/download_item.h"
+#include "content/public/browser/download_manager.h"
 #include "content/public/browser/plugin_service.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/manifest_handlers/mime_types_handler.h"
@@ -23,6 +35,7 @@
 #include "grit/component_extension_resources.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "url/gurl.h"
 
 const int kNumberLoadTestParts = 10;
 
@@ -167,6 +180,55 @@ IN_PROC_BROWSER_TEST_P(PDFExtensionTest, Load) {
   LoadAllPdfsTest("pdf", GetParam());
 }
 
+class TestDownloadObserver : public content::DownloadManager::Observer {
+ public:
+  TestDownloadObserver() {}
+  virtual ~TestDownloadObserver() {}
+
+  GURL GetLastUrl() {

[mmenke, 2015/07/09 20:35:39]

nit:  const GURL& last_url() const

[raymes, 2015/07/17 03:21:00]

Done.

+    return url_;

[mmenke, 2015/07/09 20:35:39]

nit: url_ -> last_url_?

[raymes, 2015/07/17 03:21:00]

Done.

+  }
+
+  // content::DownloadManager::Observer implementation
+  void OnDownloadCreated(content::DownloadManager* manager,
+                         content::DownloadItem* item) override {
+    url_ = item->GetURL();
+  }
+
+ private:
+  GURL url_;
+};
+
+IN_PROC_BROWSER_TEST_F(PDFExtensionTest, DisablePlugin) {
+  // Disable the PDF plugin.
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  content::BrowserContext* browser_context = web_contents->GetBrowserContext();
+  Profile* profile = Profile::FromBrowserContext(browser_context);
+  scoped_refptr<PluginPrefs> prefs(PluginPrefs::GetForProfile(profile));
+  ASSERT_TRUE(prefs.get());
+  prefs->EnablePluginGroup(false,
+      base::UTF8ToUTF16(ChromeContentClient::kPDFPluginName));
+
+  // Register a download observer.
+  content::DownloadManager* download_manager =
+      content::BrowserContext::GetDownloadManager(browser_context);
+  TestDownloadObserver download_observer;
+  download_manager->AddObserver(&download_observer);
+
+  // Navigate to a PDF and test that it is downloaded.
+  GURL url(embedded_test_server()->GetURL("/pdf/test.pdf"));
+  ui_test_utils::NavigateToURL(browser(), url);
+  ASSERT_EQ(url, download_observer.GetLastUrl());

[mmenke, 2015/07/09 20:35:39]

Right...But the download is not considered part of the navigation, so it's not
clear whether or not the navigation stops after the download is created.

It's possible there could be something like:

if (navigation_is_really_download) {
  PostUITask(StopNavigation);
  PostIOTask(ConvertRequestToDownload);
}

(Of course, with the renderer process involved, things get much uglier).  I
don't think one can argue this is non-racy without digging into the download
code, and how the navigation is actually stopped.

[raymes, 2015/07/17 03:21:00]

I agree with you, it's not completely clear whether it should work. I added a
RunLoop as suggested earlier.

+
+  // Cancel the download to shutdown cleanly.
+  download_manager->RemoveObserver(&download_observer);
+  std::vector<content::DownloadItem*> downloads;
+  download_manager->GetAllDownloads(&downloads);
+  ASSERT_EQ(1u, downloads.size());
+  downloads[0]->Cancel(false);
+}
+
 // We break PDFTest.Load up into kNumberLoadTestParts.
 INSTANTIATE_TEST_CASE_P(PDFTestFiles,
                         PDFExtensionTest,