Reverting 14005.

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

2 files changed, 28 insertions, 20 deletions

diff --git a/chrome/browser/download/download_request_manager.cc b/chrome/browser/download/download_request_manager.cc
index 6078fce..98efb19 100644
--- a/chrome/browser/download/download_request_manager.cc
+++ b/chrome/browser/download/download_request_manager.cc
@@ -177,7 +177,7 @@ DownloadRequestManager::~DownloadRequestManager() {
 
 DownloadRequestManager::DownloadStatus
     DownloadRequestManager::GetDownloadStatus(TabContents* tab) {
-  TabDownloadState* state = GetDownloadState(&tab->controller(), NULL, false);
+  TabDownloadState* state = GetDownloadState(tab->controller(), NULL, false);
   return state ? state->download_status() : ALLOW_ONE_DOWNLOAD;
 }
 
@@ -193,7 +193,13 @@ void DownloadRequestManager::CanDownloadOnIOThread(int render_process_host_id,
 }
 
 void DownloadRequestManager::OnUserGesture(TabContents* tab) {
-  TabDownloadState* state = GetDownloadState(&tab->controller(), NULL, false);
+  NavigationController* controller = tab->controller();
+  if (!controller) {
+    NOTREACHED();
+    return;
+  }
+
+  TabDownloadState* state = GetDownloadState(controller, NULL, false);
   if (!state)
     return;
 
@@ -250,8 +256,10 @@ void DownloadRequestManager::CanDownloadImpl(
         effective_tab->delegate()->GetConstrainingContents(effective_tab);
   }
 
+  NavigationController* controller = effective_tab->controller();
+  DCHECK(controller);
   TabDownloadState* state = GetDownloadState(
-      &effective_tab->controller(), &originating_tab->controller(), true);
+      controller, originating_tab->controller(), true);
   switch (state->download_status()) {
     case ALLOW_ALL_DOWNLOADS:
       ScheduleNotification(callback, true);
diff --git a/chrome/browser/download/download_request_manager_unittest.cc b/chrome/browser/download/download_request_manager_unittest.cc
index a8ece7a..943b439 100644
--- a/chrome/browser/download/download_request_manager_unittest.cc
+++ b/chrome/browser/download/download_request_manager_unittest.cc
@@ -38,7 +38,7 @@ class DownloadRequestManagerTest
 
   void CanDownload() {
     download_request_manager_->CanDownloadImpl(
-        controller().tab_contents(), this);
+        controller()->tab_contents(), this);
   }
 
   bool ShouldAllowDownload() {
@@ -81,13 +81,13 @@ TEST_F(DownloadRequestManagerTest, Allow) {
   // All tabs should initially start at ALLOW_ONE_DOWNLOAD.
   ASSERT_EQ(DownloadRequestManager::ALLOW_ONE_DOWNLOAD,
             download_request_manager_->GetDownloadStatus(
-                controller().tab_contents()));
+                controller()->tab_contents()));
 
   // Ask if the tab can do a download. This moves to PROMPT_BEFORE_DOWNLOAD.
   CanDownload();
   ASSERT_EQ(DownloadRequestManager::PROMPT_BEFORE_DOWNLOAD,
             download_request_manager_->GetDownloadStatus(
-                controller().tab_contents()));
+                controller()->tab_contents()));
   // We should have been told we can download.
   ASSERT_EQ(1, continue_count_);
   ASSERT_EQ(0, cancel_count_);
@@ -102,7 +102,7 @@ TEST_F(DownloadRequestManagerTest, Allow) {
   ask_allow_count_ = 0;
   ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS,
             download_request_manager_->GetDownloadStatus(
-                controller().tab_contents()));
+                controller()->tab_contents()));
   // We should have been told we can download.
   ASSERT_EQ(1, continue_count_);
   ASSERT_EQ(0, cancel_count_);
@@ -114,7 +114,7 @@ TEST_F(DownloadRequestManagerTest, Allow) {
   ASSERT_EQ(0, ask_allow_count_);
   ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS,
             download_request_manager_->GetDownloadStatus(
-                controller().tab_contents()));
+                controller()->tab_contents()));
   // We should have been told we can download.
   ASSERT_EQ(1, continue_count_);
   ASSERT_EQ(0, cancel_count_);
@@ -131,7 +131,7 @@ TEST_F(DownloadRequestManagerTest, ResetOnNavigation) {
   ask_allow_count_ = continue_count_ = cancel_count_ = 0;
   ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS,
             download_request_manager_->GetDownloadStatus(
-                controller().tab_contents()));
+                controller()->tab_contents()));
 
   // Navigate to a new URL with the same host, which shouldn't reset the allow
   // all state.
@@ -143,20 +143,20 @@ TEST_F(DownloadRequestManagerTest, ResetOnNavigation) {
   ask_allow_count_ = continue_count_ = cancel_count_ = 0;
   ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS,
             download_request_manager_->GetDownloadStatus(
-                controller().tab_contents()));
+                controller()->tab_contents()));
 
   // Do a user gesture, because we're at allow all, this shouldn't change the
   // state.
-  download_request_manager_->OnUserGesture(controller().tab_contents());
+  download_request_manager_->OnUserGesture(controller()->tab_contents());
   ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS,
             download_request_manager_->GetDownloadStatus(
-                controller().tab_contents()));
+                controller()->tab_contents()));
 
   // Navigate to a completely different host, which should reset the state.
   NavigateAndCommit(GURL("http://fooey.com"));
   ASSERT_EQ(DownloadRequestManager::ALLOW_ONE_DOWNLOAD,
             download_request_manager_->GetDownloadStatus(
-                controller().tab_contents()));
+                controller()->tab_contents()));
 }
 
 TEST_F(DownloadRequestManagerTest, ResetOnUserGesture) {
@@ -167,13 +167,13 @@ TEST_F(DownloadRequestManagerTest, ResetOnUserGesture) {
   ask_allow_count_ = continue_count_ = cancel_count_ = 0;
   ASSERT_EQ(DownloadRequestManager::PROMPT_BEFORE_DOWNLOAD,
             download_request_manager_->GetDownloadStatus(
-                controller().tab_contents()));
+                controller()->tab_contents()));
 
   // Do a user gesture, which should reset back to allow one.
-  download_request_manager_->OnUserGesture(controller().tab_contents());
+  download_request_manager_->OnUserGesture(controller()->tab_contents());
   ASSERT_EQ(DownloadRequestManager::ALLOW_ONE_DOWNLOAD,
             download_request_manager_->GetDownloadStatus(
-                controller().tab_contents()));
+                controller()->tab_contents()));
 
   // Ask twice, which triggers calling the delegate. Don't allow the download
   // so that we end up with not allowed.
@@ -182,13 +182,13 @@ TEST_F(DownloadRequestManagerTest, ResetOnUserGesture) {
   CanDownload();
   ASSERT_EQ(DownloadRequestManager::DOWNLOADS_NOT_ALLOWED,
             download_request_manager_->GetDownloadStatus(
-                controller().tab_contents()));
+                controller()->tab_contents()));
 
   // A user gesture now should NOT change the state.
-  download_request_manager_->OnUserGesture(controller().tab_contents());
+  download_request_manager_->OnUserGesture(controller()->tab_contents());
   ASSERT_EQ(DownloadRequestManager::DOWNLOADS_NOT_ALLOWED,
             download_request_manager_->GetDownloadStatus(
-                controller().tab_contents()));
+                controller()->tab_contents()));
   // And make sure we really can't download.
   ask_allow_count_ = continue_count_ = cancel_count_ = 0;
   CanDownload();
@@ -198,5 +198,5 @@ TEST_F(DownloadRequestManagerTest, ResetOnUserGesture) {
   // And the state shouldn't have changed.
   ASSERT_EQ(DownloadRequestManager::DOWNLOADS_NOT_ALLOWED,
             download_request_manager_->GetDownloadStatus(
-                controller().tab_contents()));
+                controller()->tab_contents()));
 }