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

chrome/browser/pdf/pdf_extension_test.cc

 // Copyright 2014 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 <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"
 #include "extensions/test/result_catcher.h"
 #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;
 
 class PDFExtensionTest : public ExtensionApiTest,
                          public testing::WithParamInterface<int> {
  public:
   ~PDFExtensionTest() override {}
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitch(switches::kDisablePdfMaterialUI);
@@ skipping 124 matching lines @@
 
 IN_PROC_BROWSER_TEST_P(PDFExtensionTest, Load) {
 #if defined(GOOGLE_CHROME_BUILD)
   // Load private PDFs.
   LoadAllPdfsTest("pdf_private", GetParam());
 #endif
   // Load public PDFs.
   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,
                         testing::Range(0, kNumberLoadTestParts));
 
 IN_PROC_BROWSER_TEST_F(PDFExtensionTest, Basic) {
   RunTestsInFile("basic_test.js", "test.pdf");
 }
 
 IN_PROC_BROWSER_TEST_F(PDFExtensionTest, BasicPlugin) {
@@ skipping 46 matching lines @@
   RunTestsInFile("navigator_test.js", "test.pdf");
 }
 
 IN_PROC_BROWSER_TEST_F(MaterialPDFExtensionTest, ParamsParser) {
   RunTestsInFile("params_parser_test.js", "test.pdf");
 }
 
 IN_PROC_BROWSER_TEST_F(MaterialPDFExtensionTest, ZoomManager) {
   RunTestsInFile("zoom_manager_test.js", "test.pdf");
 }