This CL makes the safe browsing interstitial page support multiple unsafe resources in one page.

We had a bug when an interstitial was showing and another unsafe resource was detected. We would show another interstitial on top of the original one, causing the DontProceed method to be invoked several times. That would cause us to remove more than once an entry from the navigation controller and cause crashers.

With this new CL, if an interstitial shows and a new resource is flagged as bad, the SafeBrowsingBlockingPage will queue that notification. If the user decides to proceed through the interstitial, we'll create another interstitial warning about all the unsafe resources we have received so far.

This CL also contains a fix for a crasher that would happen when closing a tab with a safe browsing interstitial.

BUG=5916,6207,6306
TEST=Test all actions in the interstitial you get when opening pages with the followin scenarios:
- Main page is malware
- Main page is fishing
- Main page is OK contains resources (images,iframes...) which are malware
- Main page is OK contains resources (images,iframes...) which are phishing
- Main page is OK contains resources (images,iframes...) some of them phishing, some of then malware.
Note that when there are more than one bad resource, it is normal to see a 1st interstitial, then another one listing all the other bad resources.
(ex of malware site http://ianfette.org, phishing site http://cvisit.tripod.com)
Review URL: http://codereview.chromium.org/18346

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

2 files changed, 51 insertions, 11 deletions

diff --git a/chrome/browser/tab_contents/interstitial_page.cc b/chrome/browser/tab_contents/interstitial_page.cc
index bc65504..942c126 100644
--- a/chrome/browser/tab_contents/interstitial_page.cc
+++ b/chrome/browser/tab_contents/interstitial_page.cc
@@ -21,11 +21,12 @@ namespace {
 
 class ResourceRequestTask : public Task {
  public:
-  ResourceRequestTask(RenderViewHost* render_view_host,
+  ResourceRequestTask(int process_id,
+                      int render_view_host_id,
                       ResourceRequestAction action)
       : action_(action),
-        process_id_(render_view_host->process()->host_id()),
-        render_view_host_id_(render_view_host->routing_id()),
+        process_id_(process_id),
+        render_view_host_id_(render_view_host_id),
         resource_dispatcher_host_(
             g_browser_process->resource_dispatcher_host()) {
   }
@@ -72,7 +73,10 @@ InterstitialPage::InterstitialPage(WebContents* tab,
       action_taken_(false),
       enabled_(true),
       new_navigation_(new_navigation),
+      original_rvh_process_id_(tab->render_view_host()->process()->host_id()),
+      original_rvh_id_(tab->render_view_host()->routing_id()),
       render_view_host_(NULL),
+      resource_dispatcher_host_notified_(false),
       should_revert_tab_title_(false),
       ui_loop_(MessageLoop::current()) {
   InitInterstitialPageMap();
@@ -157,16 +161,26 @@ void InterstitialPage::Observe(NotificationType type,
                                const NotificationDetails& details) {
   switch (type) {
     case NOTIFY_NAV_ENTRY_PENDING:
-      // We are navigating away from the interstitial.  Make sure clicking on
-      // the interstitial will have no effect.
+      // We are navigating away from the interstitial (the user has typed a URL
+      // in the location bar or clicked a bookmark).  Make sure clicking on the
+      // interstitial will have no effect.  Also cancel any blocked requests
+      // on the ResourceDispatcherHost.  Note that when we get this notification
+      // the RenderViewHost has not yet navigated so we'll unblock the
+      // RenderViewHost before the resource request for the new page we are
+      // navigating arrives in the ResourceDispatcherHost.  This ensures that
+      // request won't be blocked if the same RenderViewHost was used for the
+      // new navigation.
       Disable();
+      DCHECK(!resource_dispatcher_host_notified_);
+      TakeActionOnResourceDispatcher(CANCEL);
       break;
     case NOTIFY_RENDER_WIDGET_HOST_DESTROYED:
       if (!action_taken_) {
         // The RenderViewHost is being destroyed (as part of the tab being
         // closed), make sure we clear the blocked requests.
-        DCHECK(Source<RenderViewHost>(source).ptr() ==
-               tab_->render_view_host());
+        RenderViewHost* rvh = Source<RenderViewHost>(source).ptr();
+        DCHECK(rvh->process()->host_id() == original_rvh_process_id_ &&
+               rvh->routing_id() == original_rvh_id_);
         TakeActionOnResourceDispatcher(CANCEL);
       }
       break;
@@ -210,7 +224,10 @@ RenderViewHost* InterstitialPage::CreateRenderViewHost() {
 }
 
 void InterstitialPage::Proceed() {
-  DCHECK(!action_taken_);
+  if (action_taken_) {
+    NOTREACHED();
+    return;
+  }
   Disable();
   action_taken_ = true;
 
@@ -235,7 +252,10 @@ void InterstitialPage::Proceed() {
 }
 
 void InterstitialPage::DontProceed() {
-  DCHECK(!action_taken_);
+  if (action_taken_) {
+    NOTREACHED();
+    return;
+  }
   Disable();
   action_taken_ = true;
 
@@ -326,14 +346,25 @@ void InterstitialPage::TakeActionOnResourceDispatcher(
     ResourceRequestAction action) {
   DCHECK(MessageLoop::current() == ui_loop_) << 
       "TakeActionOnResourceDispatcher should be called on the main thread.";
+
+  if (action == CANCEL || action == RESUME) {
+    if (resource_dispatcher_host_notified_)
+      return;
+    resource_dispatcher_host_notified_ = true;
+  }
+
   // The tab might not have a render_view_host if it was closed (in which case,
   // we have taken care of the blocked requests when processing
   // NOTIFY_RENDER_WIDGET_HOST_DESTROYED.
   // Also we need to test there is an IO thread, as when unit-tests we don't
   // have one.
-  if (tab_->render_view_host() && g_browser_process->io_thread()) {
+  RenderViewHost* rvh = RenderViewHost::FromID(original_rvh_process_id_,
+                                               original_rvh_id_);
+  if (rvh && g_browser_process->io_thread()) {
     g_browser_process->io_thread()->message_loop()->PostTask(
-        FROM_HERE, new ResourceRequestTask(tab_->render_view_host(), action));
+        FROM_HERE, new ResourceRequestTask(original_rvh_process_id_,
+                                           original_rvh_id_,
+                                           action));
   }
 }
 
@@ -354,3 +385,4 @@ InterstitialPage* InterstitialPage::GetInterstitialPage(
 
   return iter->second;
 }
+
diff --git a/chrome/browser/tab_contents/interstitial_page.h b/chrome/browser/tab_contents/interstitial_page.h
index 755ec65..eb18b60 100644
--- a/chrome/browser/tab_contents/interstitial_page.h
+++ b/chrome/browser/tab_contents/interstitial_page.h
@@ -150,10 +150,18 @@ class InterstitialPage : public NotificationObserver,
   // The RenderViewHost displaying the interstitial contents.
   RenderViewHost* render_view_host_;
 
+  // The IDs for the RenderViewHost hidden by this interstitial.
+  int original_rvh_process_id_;
+  int original_rvh_id_;
+
   // Whether or not we should change the title of the tab when hidden (to revert
   // it to its original value).
   bool should_revert_tab_title_;
 
+  // Whether the ResourceDispatcherHost has been notified to cancel/resume the
+  // resource requests blocked for the RenderViewHost.
+  bool resource_dispatcher_host_notified_;
+
   // The original title of the tab that should be reverted to when the
   // interstitial is hidden.
   std::wstring original_tab_title_;