Changes around instant to verify if the page really supports instant

before sending down the instant script. If the page doesn't support
instant we fallback to reloading urls.

BUG=54833
TEST=none

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

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

8 files changed, 324 insertions, 70 deletions

diff --git a/chrome/browser/instant/instant_controller.cc b/chrome/browser/instant/instant_controller.cc
index a095696..34c6838 100644
--- a/chrome/browser/instant/instant_controller.cc
+++ b/chrome/browser/instant/instant_controller.cc
@@ -70,25 +70,15 @@ void InstantController::Update(TabContents* tab_contents,
     TemplateURLModel* model = tab_contents->profile()->GetTemplateURLModel();
     template_url = model ? model->GetDefaultSearchProvider() : NULL;
   }
-  // TODO(sky): remove the id check. It's only necessary because the search
-  // engine saved to prefs doesn't have an id. Jean-luc is fixing separately.
-  if (template_url && (!template_url->supports_instant() ||
-                       !template_url->id() ||
+  if (template_url && (!template_url->id() ||
+                       IsBlacklistedFromInstant(template_url->id()) ||
                        !TemplateURL::SupportsReplacement(template_url))) {
     template_url = NULL;
   }
   TemplateURLID template_url_id = template_url ? template_url->id() : 0;
 
-  InstantLoader* old_loader = loader_manager_->current_loader();
-  scoped_ptr<InstantLoader> owned_loader;
-  InstantLoader* new_loader =
-      loader_manager_->UpdateLoader(template_url_id, &owned_loader);
-
-  new_loader->SetOmniboxBounds(omnibox_bounds_);
-  new_loader->Update(tab_contents, match, user_text, template_url,
-                     suggested_text);
-  if (old_loader != new_loader && new_loader->ready())
-    delegate_->ShowInstant(new_loader->preview_contents());
+  UpdateLoader(template_url_id, match.destination_url, match.transition,
+               user_text, template_url, suggested_text);
 }
 
 void InstantController::SetOmniboxBounds(const gfx::Rect& bounds) {
@@ -96,6 +86,7 @@ void InstantController::SetOmniboxBounds(const gfx::Rect& bounds) {
     return;
 
   if (loader_manager_.get()) {
+    omnibox_bounds_ = bounds;
     if (loader_manager_->current_loader())
       loader_manager_->current_loader()->SetOmniboxBounds(bounds);
     if (loader_manager_->pending_loader())
@@ -113,6 +104,10 @@ void InstantController::DestroyPreviewContents() {
   delete ReleasePreviewContents(INSTANT_COMMIT_DESTROY);
 }
 
+bool InstantController::IsCurrent() {
+  return loader_manager_.get() && loader_manager_->active_loader()->ready();
+}
+
 void InstantController::CommitCurrentPreview(InstantCommitType type) {
   DCHECK(loader_manager_.get());
   DCHECK(loader_manager_->current_loader());
@@ -136,6 +131,7 @@ TabContents* InstantController::ReleasePreviewContents(InstantCommitType type) {
   scoped_ptr<InstantLoader> loader(loader_manager_->ReleaseCurrentLoader());
   TabContents* tab = loader->ReleasePreviewContents(type);
 
+  ClearBlacklist();
   is_active_ = false;
   omnibox_bounds_ = gfx::Rect();
   commit_on_mouse_up_ = false;
@@ -163,7 +159,7 @@ void InstantController::ShowInstantLoader(InstantLoader* loader) {
     loader_manager_->MakePendingCurrent(&old_loader);
     delegate_->ShowInstant(loader->preview_contents());
   } else {
-    NOTREACHED();
+    // The loader supports instant but isn't active yet. Nothing to do.
   }
 }
 
@@ -192,6 +188,63 @@ void InstantController::CommitInstantLoader(InstantLoader* loader) {
   }
 }
 
+void InstantController::InstantLoaderDoesntSupportInstant(
+    InstantLoader* loader,
+    bool needs_reload) {
+  DCHECK(!loader->ready());  // We better not be showing this loader.
+  DCHECK(loader->template_url_id());
+
+  BlacklistFromInstant(loader->template_url_id());
+
+  if (loader_manager_->active_loader() == loader) {
+    // The loader is active. Continue to use it, but make sure it isn't tied to
+    // to the search engine anymore. ClearTemplateURLID ends up showing the
+    // loader.
+    loader_manager_->RemoveLoaderFromInstant(loader);
+    loader->ClearTemplateURLID();
+
+    if (needs_reload) {
+      string16 suggested_text;
+      loader->Update(tab_contents_, loader->url(), last_transition_type_,
+                     loader->user_text(), NULL, &suggested_text);
+    }
+
+  } else {
+    loader_manager_->DestroyLoader(loader);
+    loader = NULL;
+  }
+}
+
+void InstantController::UpdateLoader(TemplateURLID template_url_id,
+                                     const GURL& url,
+                                     PageTransition::Type transition_type,
+                                     const string16& user_text,
+                                     const TemplateURL* template_url,
+                                     string16* suggested_text) {
+  InstantLoader* old_loader = loader_manager_->current_loader();
+  scoped_ptr<InstantLoader> owned_loader;
+  InstantLoader* new_loader =
+      loader_manager_->UpdateLoader(template_url_id, &owned_loader);
+
+  new_loader->SetOmniboxBounds(omnibox_bounds_);
+  new_loader->Update(tab_contents_, url, transition_type, user_text,
+                     template_url, suggested_text);
+  if (old_loader != new_loader && new_loader->ready())
+    delegate_->ShowInstant(new_loader->preview_contents());
+}
+
 bool InstantController::ShouldShowPreviewFor(const GURL& url) {
   return !url.SchemeIs(chrome::kJavaScriptScheme);
 }
+
+void InstantController::BlacklistFromInstant(TemplateURLID id) {
+  blacklisted_ids_.insert(id);
+}
+
+bool InstantController::IsBlacklistedFromInstant(TemplateURLID id) {
+  return blacklisted_ids_.count(id) > 0;
+}
+
+void InstantController::ClearBlacklist() {
+  blacklisted_ids_.clear();
+}
diff --git a/chrome/browser/instant/instant_controller.h b/chrome/browser/instant/instant_controller.h
index 678a9e1..9032234 100644
--- a/chrome/browser/instant/instant_controller.h
+++ b/chrome/browser/instant/instant_controller.h
@@ -6,6 +6,8 @@
 #define CHROME_BROWSER_INSTANT_INSTANT_CONTROLLER_H_
 #pragma once
 
+#include <set>
+
 #include "base/basictypes.h"
 #include "base/scoped_ptr.h"
 #include "base/string16.h"
@@ -20,6 +22,7 @@ struct AutocompleteMatch;
 class InstantDelegate;
 class InstantLoaderManager;
 class TabContents;
+class TemplateURL;
 
 // InstantController maintains a TabContents that is intended to give a preview
 // of a URL. InstantController is owned by Browser.
@@ -55,6 +58,12 @@ class InstantController : public InstantLoaderDelegate {
   // has not been created.
   void DestroyPreviewContents();
 
+  // Returns true if we're showing the last URL passed to |Update|. If this is
+  // false a commit does not result in committing the last url passed to update.
+  // A return value of false happens if we're in the process of determining if
+  // the page supports instant.
+  bool IsCurrent();
+
   // Invoked when the user does some gesture that should trigger making the
   // current previewed page the permanent page.
   void CommitCurrentPreview(InstantCommitType type);
@@ -99,6 +108,8 @@ class InstantController : public InstantLoaderDelegate {
   virtual gfx::Rect GetInstantBounds();
   virtual bool ShouldCommitInstantOnMouseUp();
   virtual void CommitInstantLoader(InstantLoader* loader);
+  virtual void InstantLoaderDoesntSupportInstant(InstantLoader* loader,
+                                                 bool needs_reload);
 
  private:
   // Invoked when the page wants to update the suggested text. If |user_text_|
@@ -114,9 +125,28 @@ class InstantController : public InstantLoaderDelegate {
   // Invoked once the page has finished loading and the script has been sent.
   void PageFinishedLoading();
 
+  // Updates InstantLoaderManager and its current InstantLoader. This is invoked
+  // internally from Update.
+  void UpdateLoader(TemplateURLID template_url_id,
+                    const GURL& url,
+                    PageTransition::Type transition_type,
+                    const string16& user_text,
+                    const TemplateURL* template_url,
+                    string16* suggested_text);
+
   // Returns true if we should show preview for |url|.
   bool ShouldShowPreviewFor(const GURL& url);
 
+  // Marks the specified search engine id as not supporting instant.
+  void BlacklistFromInstant(TemplateURLID id);
+
+  // Returns true if the specified id has been blacklisted from supporting
+  // instant.
+  bool IsBlacklistedFromInstant(TemplateURLID id);
+
+  // Clears the set of search engines blacklisted.
+  void ClearBlacklist();
+
   InstantDelegate* delegate_;
 
   // The TabContents last passed to |Update|.
@@ -137,6 +167,12 @@ class InstantController : public InstantLoaderDelegate {
 
   scoped_ptr<InstantLoaderManager> loader_manager_;
 
+  // The IDs of any search engines that don't support instant. We assume all
+  // search engines support instant, but if we determine an engine doesn't
+  // support instant it is added to this list. The list is cleared out on every
+  // reset/commit.
+  std::set<TemplateURLID> blacklisted_ids_;
+
   DISALLOW_COPY_AND_ASSIGN(InstantController);
 };
 
diff --git a/chrome/browser/instant/instant_loader.cc b/chrome/browser/instant/instant_loader.cc
index 0dfb85f..dbcbdc2 100644
--- a/chrome/browser/instant/instant_loader.cc
+++ b/chrome/browser/instant/instant_loader.cc
@@ -5,11 +5,12 @@
 #include "chrome/browser/instant/instant_loader.h"
 
 #include <algorithm>
+#include <utility>
 
 #include "base/command_line.h"
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
-#include "chrome/browser/autocomplete/autocomplete.h"
+#include "base/values.h"
 #include "chrome/browser/favicon_service.h"
 #include "chrome/browser/history/history_marshaling.h"
 #include "chrome/browser/instant/instant_loader_delegate.h"
@@ -56,6 +57,10 @@ const char kSetOmniboxBoundsScript[] =
     "if (window.chrome.setDropdownDimensions) "
     "window.chrome.setDropdownDimensions($1, $2, $3, $4);";
 
+// Script sent to see if the page really supports instant.
+const char kSupportsInstantScript[] =
+    "if (window.chrome.setDropdownDimensions) true; else false;";
+
 // Escapes quotes in the |text| so that it be passed to JavaScript as a quoted
 // string.
 string16 EscapeUserText(const string16& text) {
@@ -121,7 +126,8 @@ class InstantLoader::FrameLoadObserver : public NotificationObserver {
         tab_contents_(loader->preview_contents()),
         unique_id_(tab_contents_->controller().pending_entry()->unique_id()),
         text_(text),
-        pressed_enter_(false) {
+        initial_text_(text),
+        execute_js_id_(0) {
     registrar_.Add(this, NotificationType::LOAD_COMPLETED_MAIN_FRAME,
                    Source<TabContents>(tab_contents_));
     registrar_.Add(this, NotificationType::TAB_CONTENTS_DESTROYED,
@@ -131,13 +137,6 @@ class InstantLoader::FrameLoadObserver : public NotificationObserver {
   // Sets the text to send to the page.
   void set_text(const string16& text) { text_ = text; }
 
-  // Invoked when the InstantLoader releases ownership of the TabContents and
-  // the page hasn't finished loading.
-  void DetachFromPreview(bool pressed_enter) {
-    loader_ = NULL;
-    pressed_enter_ = pressed_enter;
-  }
-
   // NotificationObserver:
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
@@ -152,20 +151,25 @@ class InstantLoader::FrameLoadObserver : public NotificationObserver {
           return;
         }
 
-        if (loader_) {
-          gfx::Rect bounds = loader_->GetOmniboxBoundsInTermsOfPreview();
-          if (!bounds.IsEmpty())
-            SendOmniboxBoundsScript(tab_contents_, bounds);
-        }
-
-        SendUserInputScript(tab_contents_, text_);
+        DetermineIfPageSupportsInstant();
+        break;
+      }
 
-        if (loader_)
-          loader_->PageFinishedLoading();
-        else
-          SendDoneScript(tab_contents_, text_, pressed_enter_);
+      case NotificationType::EXECUTE_JAVASCRIPT_RESULT: {
+        typedef std::pair<int, Value*> ExecuteDetailType;
+        ExecuteDetailType* result =
+            (static_cast<Details<ExecuteDetailType > >(details)).ptr();
+        if (result->first != execute_js_id_)
+          return;
 
-        delete this;
+        DCHECK(loader_);
+        bool bool_result;
+        if (!result->second || !result->second->IsType(Value::TYPE_BOOLEAN) ||
+            !result->second->GetAsBoolean(&bool_result) || !bool_result) {
+          DoesntSupportInstant();
+          return;
+        }
+        SupportsInstant();
         return;
       }
 
@@ -180,6 +184,43 @@ class InstantLoader::FrameLoadObserver : public NotificationObserver {
   }
 
  private:
+  // Executes the javascript to determine if the page supports script.  The
+  // results are passed back to us by way of NotificationObserver.
+  void DetermineIfPageSupportsInstant() {
+    DCHECK_EQ(0, execute_js_id_);
+
+    RenderViewHost* rvh = tab_contents_->render_view_host();
+    registrar_.Add(this, NotificationType::EXECUTE_JAVASCRIPT_RESULT,
+                   Source<RenderViewHost>(rvh));
+    execute_js_id_ = rvh->ExecuteJavascriptInWebFrameNotifyResult(
+        string16(),
+        ASCIIToUTF16(kSupportsInstantScript));
+  }
+
+  // Invoked when we determine the page doesn't really support instant.
+  void DoesntSupportInstant() {
+    DCHECK(loader_);
+
+    loader_->PageDoesntSupportInstant(text_ != initial_text_);
+
+    // WARNING: we've been deleted.
+  }
+
+  // Invoked when we determine the page really supports instant.
+  void SupportsInstant() {
+    DCHECK(loader_);
+
+    gfx::Rect bounds = loader_->GetOmniboxBoundsInTermsOfPreview();
+    if (!bounds.IsEmpty())
+      SendOmniboxBoundsScript(tab_contents_, bounds);
+
+    SendUserInputScript(tab_contents_, text_);
+
+    loader_->PageFinishedLoading();
+
+    // WARNING: we've been deleted.
+  }
+
   // InstantLoader that created us.
   InstantLoader* loader_;
 
@@ -192,12 +233,16 @@ class InstantLoader::FrameLoadObserver : public NotificationObserver {
   // Text to send down to the page.
   string16 text_;
 
-  // Passed to SendDoneScript.
-  bool pressed_enter_;
+  // Initial text supplied to constructor.
+  const string16 initial_text_;
 
   // Registers and unregisters us for notifications.
   NotificationRegistrar registrar_;
 
+  // ID of the javascript that was executed to determine if the page supports
+  // instant.
+  int execute_js_id_;
+
   DISALLOW_COPY_AND_ASSIGN(FrameLoadObserver);
 };
 
@@ -461,7 +506,8 @@ class InstantLoader::TabContentsDelegateImpl : public TabContentsDelegate {
 InstantLoader::InstantLoader(InstantLoaderDelegate* delegate, TemplateURLID id)
     : delegate_(delegate),
       template_url_id_(id),
-      ready_(false) {
+      ready_(false),
+      last_transition_type_(PageTransition::LINK) {
   preview_tab_contents_delegate_.reset(new TabContentsDelegateImpl(this));
 }
 
@@ -472,16 +518,18 @@ InstantLoader::~InstantLoader() {
 }
 
 void InstantLoader::Update(TabContents* tab_contents,
-                           const AutocompleteMatch& match,
+                           const GURL& url,
+                           PageTransition::Type transition_type,
                            const string16& user_text,
                            const TemplateURL* template_url,
                            string16* suggested_text) {
-  DCHECK(url_ != match.destination_url);
-
-  url_ = match.destination_url;
+  if (url_ == url)
+    return;
 
-  DCHECK(!url_.is_empty() && url_.is_valid());
+  DCHECK(!url.is_empty() && url.is_valid());
 
+  last_transition_type_ = transition_type;
+  url_ = url;
   user_text_ = user_text;
 
   bool created_preview_contents;
@@ -523,18 +571,13 @@ void InstantLoader::Update(TabContents* tab_contents,
         *suggested_text = complete_suggested_text_.substr(user_text_.size());
       }
     } else {
-      // TODO: should we use a different url for instant?
-      GURL url = GURL(template_url->url()->ReplaceSearchTerms(
-          *template_url, std::wstring(),
-          TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring()));
-      // user_text_ is sent once the page finishes loading by FrameLoadObserver.
-      preview_contents_->controller().LoadURL(url, GURL(), match.transition);
+      preview_contents_->controller().LoadURL(url, GURL(), transition_type);
       frame_load_observer_.reset(new FrameLoadObserver(this, user_text_));
     }
   } else {
     DCHECK(template_url_id_ == 0);
     frame_load_observer_.reset(NULL);
-    preview_contents_->controller().LoadURL(url_, GURL(), match.transition);
+    preview_contents_->controller().LoadURL(url_, GURL(), transition_type);
   }
 }
 
@@ -567,13 +610,11 @@ TabContents* InstantLoader::ReleasePreviewContents(InstantCommitType type) {
   if (!preview_contents_.get())
     return NULL;
 
-  if (frame_load_observer_.get()) {
-    frame_load_observer_->DetachFromPreview(
-        type == INSTANT_COMMIT_PRESSED_ENTER);
-    // FrameLoadObserver will delete itself either when the TabContents is
-    // deleted, or when the page finishes loading.
-    FrameLoadObserver* unused ALLOW_UNUSED = frame_load_observer_.release();
-  } else if (type != INSTANT_COMMIT_DESTROY && is_showing_instant()) {
+  // FrameLoadObserver is only used for instant results, and instant results are
+  // only committed if active (when the FrameLoadObserver isn't installed).
+  DCHECK(type == INSTANT_COMMIT_DESTROY || !frame_load_observer_.get());
+
+  if (type != INSTANT_COMMIT_DESTROY && is_showing_instant()) {
     SendDoneScript(preview_contents_.get(),
                    user_text_,
                    type == INSTANT_COMMIT_PRESSED_ENTER);
@@ -606,8 +647,24 @@ void InstantLoader::CommitInstantLoader() {
   delegate_->CommitInstantLoader(this);
 }
 
+void InstantLoader::ClearTemplateURLID() {
+  // This should only be invoked for sites we thought supported instant.
+  DCHECK(template_url_id_);
+
+  // The frame load observer should have completed.
+  DCHECK(!frame_load_observer_.get());
+
+  // We shouldn't be ready.
+  DCHECK(!ready());
+
+  template_url_id_ = 0;
+  ShowPreview();
+}
+
 void InstantLoader::SetCompleteSuggestedText(
     const string16& complete_suggested_text) {
+  ShowPreview();
+
   if (complete_suggested_text == complete_suggested_text_)
     return;
 
@@ -626,7 +683,10 @@ void InstantLoader::SetCompleteSuggestedText(
 }
 
 void InstantLoader::PreviewPainted() {
-  ShowPreview();
+  // If instant is supported then we wait for the first suggest result before
+  // showing the page.
+  if (!template_url_id_)
+    ShowPreview();
 }
 
 void InstantLoader::ShowPreview() {
@@ -637,8 +697,9 @@ void InstantLoader::ShowPreview() {
 }
 
 void InstantLoader::PageFinishedLoading() {
-  // FrameLoadObserver deletes itself after this call.
-  FrameLoadObserver* unused ALLOW_UNUSED = frame_load_observer_.release();
+  frame_load_observer_.reset();
+  // Wait for the user input before showing, this way the page should be up to
+  // date by the time we show it.
 }
 
 gfx::Rect InstantLoader::GetOmniboxBoundsInTermsOfPreview() {
@@ -651,3 +712,9 @@ gfx::Rect InstantLoader::GetOmniboxBoundsInTermsOfPreview() {
                    omnibox_bounds_.width(),
                    omnibox_bounds_.height());
 }
+
+void InstantLoader::PageDoesntSupportInstant(bool needs_reload) {
+  frame_load_observer_.reset(NULL);
+
+  delegate_->InstantLoaderDoesntSupportInstant(this, needs_reload);
+}
diff --git a/chrome/browser/instant/instant_loader.h b/chrome/browser/instant/instant_loader.h
index a5edb35..72ea5e7 100644
--- a/chrome/browser/instant/instant_loader.h
+++ b/chrome/browser/instant/instant_loader.h
@@ -15,7 +15,6 @@
 #include "gfx/rect.h"
 #include "googleurl/src/gurl.h"
 
-struct AutocompleteMatch;
 class InstantLoaderDelegate;
 class InstantLoaderManagerTest;
 class TabContents;
@@ -24,6 +23,13 @@ class TemplateURL;
 // InstantLoader does the loading of a particular URL for InstantController.
 // InstantLoader notifies its delegate, which is typically InstantController, of
 // all interesting events.
+//
+// InstantLoader is created with a TemplateURLID. If non-zero InstantLoader
+// first determines if the site actually supports instant. If it doesn't, the
+// delegate is notified by way of |InstantLoaderDoesntSupportInstant|.
+//
+// If the TemplateURLID supplied to the constructor is zero, then the url is
+// loaded as is.
 class InstantLoader {
  public:
   InstantLoader(InstantLoaderDelegate* delegate, TemplateURLID id);
@@ -32,7 +38,8 @@ class InstantLoader {
   // Invoked to load a URL. |tab_contents| is the TabContents the preview is
   // going to be shown on top of and potentially replace.
   void Update(TabContents* tab_contents,
-              const AutocompleteMatch& match,
+              const GURL& url,
+              PageTransition::Type transition_type,
               const string16& user_text,
               const TemplateURL* template_url,
               string16* suggested_text);
@@ -59,6 +66,10 @@ class InstantLoader {
   bool ShouldCommitInstantOnMouseUp();
   void CommitInstantLoader();
 
+  // Resets the template_url_id_ to zero and shows this loader. This is only
+  // intended to be invoked from InstantLoaderDoesntSupportInstant.
+  void ClearTemplateURLID();
+
   // The preview TabContents; may be null.
   TabContents* preview_contents() const { return preview_contents_.get(); }
 
@@ -73,6 +84,9 @@ class InstantLoader {
   // If we're showing instant this returns non-zero.
   TemplateURLID template_url_id() const { return template_url_id_; }
 
+  // See description above field.
+  const string16& user_text() const { return user_text_; }
+
  private:
   friend class InstantLoaderManagerTest;
   class FrameLoadObserver;
@@ -103,11 +117,16 @@ class InstantLoader {
     return frame_load_observer_.get() != NULL;
   }
 
+  // Invoked if it the page doesn't really support instant when we thought it
+  // did. If |needs_reload| is true, the text changed since the first load and
+  // the page needs to be reloaded.
+  void PageDoesntSupportInstant(bool needs_reload);
+
   InstantLoaderDelegate* delegate_;
 
   // If we're showing instant results this is the ID of the TemplateURL driving
   // the results. A value of 0 means there is no TemplateURL.
-  const TemplateURLID template_url_id_;
+  TemplateURLID template_url_id_;
 
   // The url we're displaying.
   GURL url_;
@@ -133,6 +152,9 @@ class InstantLoader {
 
   scoped_ptr<FrameLoadObserver> frame_load_observer_;
 
+  // Transition type of the match last passed to Update.
+  PageTransition::Type last_transition_type_;
+
   DISALLOW_COPY_AND_ASSIGN(InstantLoader);
 };
 
diff --git a/chrome/browser/instant/instant_loader_delegate.h b/chrome/browser/instant/instant_loader_delegate.h
index b77742d..d3938b1 100644
--- a/chrome/browser/instant/instant_loader_delegate.h
+++ b/chrome/browser/instant/instant_loader_delegate.h
@@ -33,6 +33,13 @@ class InstantLoaderDelegate {
   // Invoked when the the loader should be committed.
   virtual void CommitInstantLoader(InstantLoader* loader) = 0;
 
+  // Invoked if the loader was created with the intention that the site supports
+  // instant, but it turned out the site doesn't support instant. If
+  // |needs_reload| is true, |Update| was invoked on the loader with a url that
+  // has changed since the initial url.
+  virtual void InstantLoaderDoesntSupportInstant(InstantLoader* loader,
+                                                 bool needs_reload) = 0;
+
  protected:
   virtual ~InstantLoaderDelegate() {}
 };
diff --git a/chrome/browser/instant/instant_loader_manager.cc b/chrome/browser/instant/instant_loader_manager.cc
index e807bfc..3b5e648 100644
--- a/chrome/browser/instant/instant_loader_manager.cc
+++ b/chrome/browser/instant/instant_loader_manager.cc
@@ -101,16 +101,37 @@ void InstantLoaderManager::MakePendingCurrent(
 InstantLoader* InstantLoaderManager::ReleaseCurrentLoader() {
   DCHECK(current_loader_);
   InstantLoader* loader = current_loader_;
-  if (current_loader_->template_url_id()) {
-    Loaders::iterator i =
-        instant_loaders_.find(current_loader_->template_url_id());
-    DCHECK(i != instant_loaders_.end());
-    instant_loaders_.erase(i);
-  }
+  RemoveLoaderFromInstant(current_loader_);
   current_loader_ = NULL;
   return loader;
 }
 
+void InstantLoaderManager::DestroyLoader(InstantLoader* loader) {
+  DCHECK(loader == current_loader_ || loader == pending_loader_ ||
+         (loader->template_url_id() &&
+          instant_loaders_.find(loader->template_url_id()) !=
+          instant_loaders_.end()));
+
+  if (current_loader_ == loader)
+    current_loader_ = pending_loader_;
+
+  if (pending_loader_ == loader)
+    pending_loader_ = NULL;
+
+  RemoveLoaderFromInstant(loader);
+
+  delete loader;
+}
+
+void InstantLoaderManager::RemoveLoaderFromInstant(InstantLoader* loader) {
+  if (!loader->template_url_id())
+    return;
+
+  Loaders::iterator i = instant_loaders_.find(loader->template_url_id());
+  DCHECK(i != instant_loaders_.end());
+  instant_loaders_.erase(i);
+}
+
 InstantLoader* InstantLoaderManager::CreateLoader(TemplateURLID id) {
   InstantLoader* loader = new InstantLoader(loader_delegate_, id);
   if (id)
diff --git a/chrome/browser/instant/instant_loader_manager.h b/chrome/browser/instant/instant_loader_manager.h
index efcc6ac..9d0f087 100644
--- a/chrome/browser/instant/instant_loader_manager.h
+++ b/chrome/browser/instant/instant_loader_manager.h
@@ -52,6 +52,12 @@ class InstantLoaderManager {
   // of InstantLoaderManager wants to take ownership of the loader.
   InstantLoader* ReleaseCurrentLoader();
 
+  // Destroyes the specified loader.
+  void DestroyLoader(InstantLoader* loader);
+
+  // If |loader| is in |instant_loaders_| is it removed.
+  void RemoveLoaderFromInstant(InstantLoader* loader);
+
   // Returns the current loader, may be null.
   InstantLoader* current_loader() const { return current_loader_; }
 
diff --git a/chrome/browser/instant/instant_loader_manager_unittest.cc b/chrome/browser/instant/instant_loader_manager_unittest.cc
index 855bf59..b689e11 100644
--- a/chrome/browser/instant/instant_loader_manager_unittest.cc
+++ b/chrome/browser/instant/instant_loader_manager_unittest.cc
@@ -30,6 +30,10 @@ class InstantLoaderDelegateImpl : public InstantLoaderDelegate {
   virtual void CommitInstantLoader(InstantLoader* loader) {
   }
 
+  virtual void InstantLoaderDoesntSupportInstant(InstantLoader* loader,
+                                                 bool needs_reload) {
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(InstantLoaderDelegateImpl);
 };
@@ -207,3 +211,41 @@ TEST_F(InstantLoaderManagerTest, ThreeInstant) {
   EXPECT_NE(instant_loader2, instant_loader3);
   EXPECT_EQ(instant_loader1, manager.current_loader());
 }
+
+// Tests DestroyLoader with an instant loader.
+TEST_F(InstantLoaderManagerTest, DestroyInstantLoader) {
+  InstantLoaderDelegateImpl delegate;
+  InstantLoaderManager manager(&delegate);
+  scoped_ptr<InstantLoader> loader;
+  manager.UpdateLoader(1, &loader);
+  ASSERT_TRUE(manager.current_loader());
+  EXPECT_EQ(1, manager.current_loader()->template_url_id());
+  // Now destroy it.
+  manager.DestroyLoader(manager.current_loader());
+
+  // There should be no current, pending and 0 instant loaders.
+  ASSERT_EQ(NULL, manager.current_loader());
+  ASSERT_EQ(NULL, manager.pending_loader());
+  EXPECT_EQ(0u, manager.num_instant_loaders());
+}
+
+// Tests DestroyLoader when the loader is pending.
+TEST_F(InstantLoaderManagerTest, DestroyPendingLoader) {
+  InstantLoaderDelegateImpl delegate;
+  InstantLoaderManager manager(&delegate);
+  scoped_ptr<InstantLoader> loader;
+  manager.UpdateLoader(1, &loader);
+  InstantLoader* first_loader = manager.active_loader();
+  MarkReady(first_loader);
+
+  // Create another loader.
+  manager.UpdateLoader(0, &loader);
+  InstantLoader* second_loader = manager.pending_loader();
+  ASSERT_TRUE(second_loader);
+  ASSERT_NE(second_loader, first_loader);
+
+  // Destroy it.
+  manager.DestroyLoader(second_loader);
+  EXPECT_EQ(NULL, manager.pending_loader());
+  EXPECT_EQ(first_loader, manager.current_loader());
+}