Fix to allow browser close after download initiated from chrome:// url.

R=brettw
BUG=12745

Review URL: http://codereview.chromium.org/150122

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@19752 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 71 insertions, 1 deletions

diff --git a/chrome/browser/renderer_host/render_view_host_manager.cc b/chrome/browser/renderer_host/render_view_host_manager.cc
index 7c4d219..a1bf00e 100644
--- a/chrome/browser/renderer_host/render_view_host_manager.cc
+++ b/chrome/browser/renderer_host/render_view_host_manager.cc
@@ -33,6 +33,7 @@ RenderViewHostManager::RenderViewHostManager(
       render_view_delegate_(render_view_delegate),
       render_view_host_(NULL),
       pending_render_view_host_(NULL),
+      pending_renderer_aborted_(false),
       interstitial_page_(NULL) {
   registrar_.Add(this, NotificationType::RENDER_VIEW_HOST_DELETED,
                  NotificationService::AllSources());
@@ -215,11 +216,21 @@ void RenderViewHostManager::RendererAbortedProvisionalLoad(
   // navigation events.  (That's necessary to support onunload anyway.)  Once
   // we've made that change, we won't create a pending renderer until we know
   // the response is not a download.
+
+  // There is one instance where we must be able to pre-emptively clean up a
+  // pending renderer: If a cross-site download is initiated from a chrome://
+  // url, and the browser then wants to close.
+  if (pending_render_view_host_) {
+    pending_renderer_aborted_ = true;
+  }
 }
 
 void RenderViewHostManager::ShouldClosePage(bool proceed) {
   // Should only see this while we have a pending renderer.  Otherwise, we
   // should ignore.
+  if (pending_render_view_host_ && pending_renderer_aborted_)
+    CancelPending();
+
   if (!pending_render_view_host_) {
     bool proceed_to_fire_unload;
     delegate_->BeforeUnloadFiredFromRenderManager(proceed,
@@ -589,7 +600,7 @@ void RenderViewHostManager::CancelPending() {
   RenderViewHost* pending_render_view_host = pending_render_view_host_;
   pending_render_view_host_ = NULL;
   pending_render_view_host->Shutdown();
-
+  pending_renderer_aborted_ = false;
   pending_dom_ui_.reset();
 }
 
diff --git a/chrome/browser/renderer_host/render_view_host_manager.h b/chrome/browser/renderer_host/render_view_host_manager.h
index 0cc1504..7e2a126 100644
--- a/chrome/browser/renderer_host/render_view_host_manager.h
+++ b/chrome/browser/renderer_host/render_view_host_manager.h
@@ -244,6 +244,10 @@ class RenderViewHostManager : public NotificationObserver {
   RenderViewHost* pending_render_view_host_;
   scoped_ptr<DOMUI> pending_dom_ui_;
 
+  // Records whether the navigation was cancelled, but we are leaving the
+  // pending_render_view_host_ around in consideration of the download system.
+  bool pending_renderer_aborted_;
+
   // The intersitial page currently shown if any, not own by this class
   // (the InterstitialPage is self-owned, it deletes itself when hidden).
   InterstitialPage* interstitial_page_;
diff --git a/chrome/browser/renderer_host/render_view_host_manager_browsertest.cc b/chrome/browser/renderer_host/render_view_host_manager_browsertest.cc
index a0fb805..8252040 100755
--- a/chrome/browser/renderer_host/render_view_host_manager_browsertest.cc
+++ b/chrome/browser/renderer_host/render_view_host_manager_browsertest.cc
@@ -6,6 +6,11 @@
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/notification_details.h"
+#include "chrome/common/notification_observer.h"
+#include "chrome/common/notification_registrar.h"
+#include "chrome/common/notification_service.h"
+#include "chrome/common/notification_type.h"
 #include "chrome/common/extensions/extension_error_reporter.h"
 #include "chrome/test/in_process_browser_test.h"
 #include "chrome/test/ui_test_utils.h"
@@ -45,3 +50,53 @@ IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest, ChromeURLAfterDownload) {
       &domui_responded));
   EXPECT_TRUE(domui_responded);
 }
+
+class BrowserClosedObserver : public NotificationObserver {
+ public:
+  BrowserClosedObserver(Browser* browser) {
+    registrar_.Add(this, NotificationType::BROWSER_CLOSED,
+        Source<Browser>(browser));
+    ui_test_utils::RunMessageLoop();
+  }
+  
+  // NotificationObserver
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details) {
+    switch (type.value) {
+      case NotificationType::BROWSER_CLOSED:
+        MessageLoopForUI::current()->Quit();
+        break;
+    }
+  }
+
+ private:
+  NotificationRegistrar registrar_;
+};
+
+// Test for crbug.com/12745. This tests that if a download is initiated from
+// a chrome:// page that has registered and onunload handler, the browser
+// will be able to close.
+IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest, BrowserCloseAfterDownload) {
+  GURL downloads_url("chrome://downloads");
+  FilePath zip_download;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &zip_download));
+  zip_download = zip_download.AppendASCII("zip").AppendASCII("test.zip");
+  ASSERT_TRUE(file_util::PathExists(zip_download));
+  GURL zip_url = net::FilePathToFileURL(zip_download);
+
+  ui_test_utils::NavigateToURL(browser(), downloads_url);
+  TabContents *contents = browser()->GetSelectedTabContents();
+  ASSERT_TRUE(contents);
+  bool result = false;
+  EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
+      contents,
+      L"",
+      L"window.onunload = function() { var do_nothing = 0; }; "
+      L"window.domAutomationController.send(true);",
+      &result));
+  EXPECT_TRUE(result);
+  ui_test_utils::NavigateToURL(browser(), zip_url);
+  browser()->CloseWindow();
+  BrowserClosedObserver observe(browser());
+}