Fix crashes in DownloadRequestLimiter when extension popups/bubbles initiate downloads automatically

chrome/browser/download/download_request_limiter.cc

 // Copyright (c) 2012 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 "chrome/browser/download/download_request_limiter.h"
 
 #include "base/bind.h"
 #include "base/stl_util.h"
 #include "chrome/browser/download/download_request_infobar_delegate.h"
+#include "chrome/browser/download/download_request_limiter_observer.h"
 #include "chrome/browser/infobars/infobar_tab_helper.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/blocked_content/blocked_content_tab_helper.h"
 #include "chrome/browser/ui/blocked_content/blocked_content_tab_helper_delegate.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
@@ skipping 57 matching lines @@
     WebContents* tab,
     const DownloadRequestLimiter::Callback& callback) {
   callbacks_.push_back(callback);
 
   if (is_showing_prompt())
     return;  // Already showing prompt.
 
   if (DownloadRequestLimiter::delegate_) {
     NotifyCallbacks(DownloadRequestLimiter::delegate_->ShouldAllowDownload());
   } else {
-    InfoBarTabHelper* infobar_helper =
-        TabContentsWrapper::GetCurrentWrapperForContents(tab)->
-            infobar_tab_helper();
+    TabContentsWrapper* tab_wrapper =
+        TabContentsWrapper::GetCurrentWrapperForContents(tab);
+    if (!tab_wrapper) {
+      // If |tab| doesn't have a tab_wrapper, then it isn't what a user thinks
+      // of as a tab, it's actually a "raw" WebContents like those used for
+      // extension popups/bubbles and hosted apps etc.
+      // TODO(benjhayden): If this is an automatic download from an extension,
+      // it would be convenient for the extension author if we send a message to
+      // the extension's DevTools console (as we do for CSP) about how
+      // extensions should use chrome.downloads.download() (requires the
+      // "downloads" permission) to automatically download multiple files.
+      Cancel();
+      return;
+    }
+    InfoBarTabHelper* infobar_helper = tab_wrapper->infobar_tab_helper();
     infobar_ = new DownloadRequestInfoBarDelegate(infobar_helper, this);
     infobar_helper->AddInfoBar(infobar_);
   }
 }
 
 void DownloadRequestLimiter::TabDownloadState::Cancel() {
   NotifyCallbacks(false);
 }
 
 void DownloadRequestLimiter::TabDownloadState::Accept() {
@@ skipping 104 matching lines @@
 }
 
 DownloadRequestLimiter::~DownloadRequestLimiter() {
   // All the tabs should have closed before us, which sends notification and
   // removes from state_map_. As such, there should be no pending callbacks.
   DCHECK(state_map_.empty());
 }
 
 DownloadRequestLimiter::DownloadStatus
     DownloadRequestLimiter::GetDownloadStatus(WebContents* tab) {
-  TabDownloadState* state = GetDownloadState(&tab->GetController(), NULL, false);
+  TabDownloadState* state = GetDownloadState(
+      &tab->GetController(), NULL, false);
   return state ? state->download_status() : ALLOW_ONE_DOWNLOAD;
 }
 
 void DownloadRequestLimiter::CanDownloadOnIOThread(
     int render_process_host_id,
     int render_view_id,
     int request_id,
     const std::string& request_method,
     const Callback& callback) {
   // This is invoked on the IO thread. Schedule the task to run on the UI
   // thread so that we can query UI state.
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&DownloadRequestLimiter::CanDownload, this,
                  render_process_host_id, render_view_id, request_id,
                  request_method, callback));
 }
 
 void DownloadRequestLimiter::OnUserGesture(WebContents* tab) {
-  TabDownloadState* state =
-      GetDownloadState(&tab->GetController(), NULL, false);
+  TabDownloadState* state = GetDownloadState(
+      &tab->GetController(), NULL, false);
   if (!state)
     return;
 
   state->OnUserGesture();
 }
 
 // static
 void DownloadRequestLimiter::SetTestingDelegate(TestingDelegate* delegate) {
   delegate_ = delegate;
 }
 
-DownloadRequestLimiter::TabDownloadState* DownloadRequestLimiter::
-    GetDownloadState(NavigationController* controller,
-                     NavigationController* originating_controller,
-                     bool create) {
+DownloadRequestLimiter::TabDownloadState*
+DownloadRequestLimiter::GetDownloadState(
+    NavigationController* controller,
+    NavigationController* originating_controller,
+    bool create) {
   DCHECK(controller);
   StateMap::iterator i = state_map_.find(controller);
   if (i != state_map_.end())
     return i->second;
 
   if (!create)
     return NULL;
 
   TabDownloadState* state =
       new TabDownloadState(this, controller, originating_controller);
   state_map_[controller] = state;
   return state;
 }
 
 void DownloadRequestLimiter::CanDownload(int render_process_host_id,
                                          int render_view_id,
                                          int request_id,
                                          const std::string& request_method,
                                          const Callback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  WebContents* originating_tab =
+  WebContents* originating_contents =
       tab_util::GetWebContentsByID(render_process_host_id, render_view_id);
-  if (!originating_tab) {
+  if (!originating_contents) {
     // The tab was closed, don't allow the download.
     ScheduleNotification(callback, false);
     return;
   }
 
   CanDownloadImpl(
-      TabContentsWrapper::GetCurrentWrapperForContents(originating_tab),
+      originating_contents,
       request_id,
       request_method,
       callback);
 }
 
-void DownloadRequestLimiter::CanDownloadImpl(
-    TabContentsWrapper* originating_tab,
-    int request_id,
-    const std::string& request_method,
-    const Callback& callback) {
-  DCHECK(originating_tab);
+void DownloadRequestLimiter::CanDownloadImpl(WebContents* originating_contents,
+                                             int request_id,
+                                             const std::string& request_method,
+                                             const Callback& callback) {
+  DCHECK(originating_contents);
+
+  if (!ContainsKey(observer_map_, originating_contents)) {
+    // DownloadRequestLimiterObserver is self-deleting. We just remember it so
+    // that we don't create another one for this WebContents*.
+    // DRLO cannot be merged with TabDownloadState because TDS is keyed to a
+    // NavigationController while DRLO is keyed to a WebContents. There may be
+    // many WebContents per NavigationController. A NavigationController is

[Avi (use Gerrit), 2012/05/24 20:48:44]

Whoa. Where did you read that? It hasn't been that way for years.

WebContents are 1:1 with NavControllers. In fact, if you look at
WebContentsImpl, the first sub-object is a NavControllerImpl.

[benjhayden, 2012/05/24 21:01:14]

http://code.google.com/searchframe#OAMlx_jo-ck/src/content/public/browser/nav...
"The NavigationController also owns all WebContents for the tab."

Now that I look at navigation_controller_impl.h, it looks like maybe I can make
TabDownloadState observe the WebContentsImpl directly.

Will a WebContentsImpl ever switch from one NavigationController to another?
Will a NavigationController ever switch from one WebContentsImpl to another?

Why are these separate classes if they're 1:1?

+    // closer to a user's concept of a "tab" than WebContents; we only use
+    // WebContents here to route OnUserGesture() signals. It might be more
+    // "standard" to make TabContentsWrapper create and own DRLO, but there is
+    // no TCW for extension popups/bubbles.
+    observer_map_[originating_contents] = new DownloadRequestLimiterObserver(
+        originating_contents,
+        base::Bind(&DownloadRequestLimiter::RemoveObserver, this,
+                   originating_contents));
+  }
 
   // FYI: Chrome Frame overrides CanDownload in ExternalTabContainer in order
   // to cancel the download operation in chrome and let the host browser
   // take care of it.
-  WebContents* tab = originating_tab->web_contents();
-  if (tab->GetDelegate() && !tab->GetDelegate()->CanDownload(
-      tab->GetRenderViewHost(), request_id, request_method)) {
+  if (originating_contents->GetDelegate() &&
+      !originating_contents->GetDelegate()->CanDownload(
+          originating_contents->GetRenderViewHost(),
+          request_id,
+          request_method)) {
     ScheduleNotification(callback, false);
     return;
   }
 
   // If the tab requesting the download is a constrained popup that is not
   // shown, treat the request as if it came from the parent.
-  TabContentsWrapper* effective_wrapper = originating_tab;
-  if (effective_wrapper->blocked_content_tab_helper()->delegate()) {
-    effective_wrapper = effective_wrapper->blocked_content_tab_helper()->
-        delegate()->GetConstrainingContentsWrapper(effective_wrapper);
+  WebContents* effective_contents = originating_contents;
+  TabContentsWrapper* originating_wrapper =
+     TabContentsWrapper::GetCurrentWrapperForContents(originating_contents);
+  if (originating_wrapper &&
+      originating_wrapper->blocked_content_tab_helper()->delegate()) {
+    effective_contents = originating_wrapper->blocked_content_tab_helper()->
+        delegate()->GetConstrainingContentsWrapper(originating_wrapper)->
+            web_contents();
   }
 
   TabDownloadState* state = GetDownloadState(
-      &effective_wrapper->web_contents()->GetController(),
-      &tab->GetController(), true);
+      &effective_contents->GetController(),
+      &originating_contents->GetController(),
+      true);
   switch (state->download_status()) {
     case ALLOW_ALL_DOWNLOADS:
       if (state->download_count() && !(state->download_count() %
             DownloadRequestLimiter::kMaxDownloadsAtOnce))
         state->set_download_status(PROMPT_BEFORE_DOWNLOAD);
       ScheduleNotification(callback, true);
       state->increment_download_count();
       break;
 
     case ALLOW_ONE_DOWNLOAD:
       state->set_download_status(PROMPT_BEFORE_DOWNLOAD);
       ScheduleNotification(callback, true);
       break;
 
     case DOWNLOADS_NOT_ALLOWED:
       ScheduleNotification(callback, false);
       break;
 
     case PROMPT_BEFORE_DOWNLOAD:
-      state->PromptUserForDownload(effective_wrapper->web_contents(), callback);
+      state->PromptUserForDownload(effective_contents, callback);
       state->increment_download_count();
       break;
 
     default:
       NOTREACHED();
   }
 }
 
 void DownloadRequestLimiter::ScheduleNotification(const Callback& callback,
                                                   bool allow) {
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE, base::Bind(callback, allow));
 }
 
 void DownloadRequestLimiter::Remove(TabDownloadState* state) {
   DCHECK(ContainsKey(state_map_, state->controller()));
   state_map_.erase(state->controller());
   delete state;
 }
 
+void DownloadRequestLimiter::RemoveObserver(
+    WebContents* web_contents,
+    DownloadRequestLimiterObserver* observer) {
+  DCHECK(ContainsKey(observer_map_, web_contents));
+  DCHECK(observer_map_[web_contents] == observer);
+  observer_map_.erase(web_contents);
+}
+
 // static
 DownloadRequestLimiter::TestingDelegate* DownloadRequestLimiter::delegate_ =
     NULL;