Adds throttling of instant results for sites that don't support

instant.

BUG=55342
TEST=none

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

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

8 files changed, 158 insertions, 43 deletions

diff --git a/chrome/browser/instant/instant_controller.cc b/chrome/browser/instant/instant_controller.cc
index 34c6838..cc4afa4 100644
--- a/chrome/browser/instant/instant_controller.cc
+++ b/chrome/browser/instant/instant_controller.cc
@@ -16,6 +16,9 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
 
+// Number of ms to delay between loading urls.
+static const int kUpdateDelayMS = 200;
+
 // static
 bool InstantController::IsEnabled() {
   static bool enabled = false;
@@ -60,25 +63,17 @@ void InstantController::Update(TabContents* tab_contents,
     return;
   }
 
+  TemplateURLID template_url_id = GetTemplateURLID(match);
+
   if (!loader_manager_.get())
     loader_manager_.reset(new InstantLoaderManager(this));
 
-  const TemplateURL* template_url = match.template_url;
-  if (match.type == AutocompleteMatch::SEARCH_WHAT_YOU_TYPED ||
-      match.type == AutocompleteMatch::SEARCH_HISTORY ||
-      match.type == AutocompleteMatch::SEARCH_SUGGEST) {
-    TemplateURLModel* model = tab_contents->profile()->GetTemplateURLModel();
-    template_url = model ? model->GetDefaultSearchProvider() : NULL;
-  }
-  if (template_url && (!template_url->id() ||
-                       IsBlacklistedFromInstant(template_url->id()) ||
-                       !TemplateURL::SupportsReplacement(template_url))) {
-    template_url = NULL;
+  if (ShouldUpdateNow(template_url_id, match.destination_url)) {
+    UpdateLoader(template_url_id, match.destination_url, match.transition,
+                 user_text, suggested_text);
+  } else {
+    ScheduleUpdate(match.destination_url);
   }
-  TemplateURLID template_url_id = template_url ? template_url->id() : 0;
-
-  UpdateLoader(template_url_id, match.destination_url, match.transition,
-               user_text, template_url, suggested_text);
 }
 
 void InstantController::SetOmniboxBounds(const gfx::Rect& bounds) {
@@ -105,7 +100,8 @@ void InstantController::DestroyPreviewContents() {
 }
 
 bool InstantController::IsCurrent() {
-  return loader_manager_.get() && loader_manager_->active_loader()->ready();
+  return loader_manager_.get() && loader_manager_->active_loader()->ready() &&
+      !update_timer_.IsRunning();
 }
 
 void InstantController::CommitCurrentPreview(InstantCommitType type) {
@@ -136,6 +132,7 @@ TabContents* InstantController::ReleasePreviewContents(InstantCommitType type) {
   omnibox_bounds_ = gfx::Rect();
   commit_on_mouse_up_ = false;
   loader_manager_.reset(NULL);
+  update_timer_.Stop();
   return tab;
 }
 
@@ -190,7 +187,8 @@ void InstantController::CommitInstantLoader(InstantLoader* loader) {
 
 void InstantController::InstantLoaderDoesntSupportInstant(
     InstantLoader* loader,
-    bool needs_reload) {
+    bool needs_reload,
+    const GURL& url_to_load) {
   DCHECK(!loader->ready());  // We better not be showing this loader.
   DCHECK(loader->template_url_id());
 
@@ -205,30 +203,76 @@ void InstantController::InstantLoaderDoesntSupportInstant(
 
     if (needs_reload) {
       string16 suggested_text;
-      loader->Update(tab_contents_, loader->url(), last_transition_type_,
-                     loader->user_text(), NULL, &suggested_text);
+      loader->Update(tab_contents_, 0, url_to_load, last_transition_type_,
+                     loader->user_text(), &suggested_text);
     }
-
   } else {
     loader_manager_->DestroyLoader(loader);
     loader = NULL;
   }
 }
 
+bool InstantController::ShouldUpdateNow(TemplateURLID instant_id,
+                                        const GURL& url) {
+  DCHECK(loader_manager_.get());
+
+  if (instant_id) {
+    // Update sites that support instant immediately, they can do their own
+    // throttling.
+    return true;
+  }
+
+  if (url.SchemeIsFile())
+    return true;  // File urls should load quickly, so don't delay loading them.
+
+  if (loader_manager_->WillUpateChangeActiveLoader(instant_id)) {
+    // If Update would change loaders, update now. This indicates transitioning
+    // from an instant to non-instant loader.
+    return true;
+  }
+
+  InstantLoader* active_loader = loader_manager_->active_loader();
+  // WillUpateChangeActiveLoader should return true if no active loader, so
+  // we know there will be an active loader if we get here.
+  DCHECK(active_loader);
+  // Immediately update if the hosts differ, otherwise we'll delay the update.
+  return active_loader->url().host() != url.host();
+}
+
+void InstantController::ScheduleUpdate(const GURL& url) {
+  scheduled_url_ = url;
+
+  if (update_timer_.IsRunning())
+    update_timer_.Stop();
+  update_timer_.Start(base::TimeDelta::FromMilliseconds(kUpdateDelayMS),
+                      this, &InstantController::ProcessScheduledUpdate);
+}
+
+void InstantController::ProcessScheduledUpdate() {
+  DCHECK(loader_manager_.get());
+
+  // We only delay loading of sites that don't support instant, so we can ignore
+  // suggested_text here.
+  string16 suggested_text;
+  UpdateLoader(0, scheduled_url_, last_transition_type_, string16(),
+               &suggested_text);
+}
+
 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) {
+  update_timer_.Stop();
+
   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);
+  new_loader->Update(tab_contents_, template_url_id, url, transition_type,
+                     user_text, suggested_text);
   if (old_loader != new_loader && new_loader->ready())
     delegate_->ShowInstant(new_loader->preview_contents());
 }
@@ -248,3 +292,20 @@ bool InstantController::IsBlacklistedFromInstant(TemplateURLID id) {
 void InstantController::ClearBlacklist() {
   blacklisted_ids_.clear();
 }
+
+TemplateURLID InstantController::GetTemplateURLID(
+    const AutocompleteMatch& match) {
+  const TemplateURL* template_url = match.template_url;
+  if (match.type == AutocompleteMatch::SEARCH_WHAT_YOU_TYPED ||
+      match.type == AutocompleteMatch::SEARCH_HISTORY ||
+      match.type == AutocompleteMatch::SEARCH_SUGGEST) {
+    TemplateURLModel* model = tab_contents_->profile()->GetTemplateURLModel();
+    template_url = model ? model->GetDefaultSearchProvider() : NULL;
+  }
+  if (template_url && template_url->id() &&
+      !IsBlacklistedFromInstant(template_url->id()) &&
+      TemplateURL::SupportsReplacement(template_url)) {
+    return template_url->id();
+  }
+  return 0;
+}
diff --git a/chrome/browser/instant/instant_controller.h b/chrome/browser/instant/instant_controller.h
index 9032234..992d927 100644
--- a/chrome/browser/instant/instant_controller.h
+++ b/chrome/browser/instant/instant_controller.h
@@ -11,6 +11,7 @@
 #include "base/basictypes.h"
 #include "base/scoped_ptr.h"
 #include "base/string16.h"
+#include "base/timer.h"
 #include "chrome/browser/instant/instant_commit_type.h"
 #include "chrome/browser/instant/instant_loader_delegate.h"
 #include "chrome/browser/search_engines/template_url_id.h"
@@ -109,21 +110,18 @@ class InstantController : public InstantLoaderDelegate {
   virtual bool ShouldCommitInstantOnMouseUp();
   virtual void CommitInstantLoader(InstantLoader* loader);
   virtual void InstantLoaderDoesntSupportInstant(InstantLoader* loader,
-                                                 bool needs_reload);
+                                                 bool needs_reload,
+                                                 const GURL& url_to_load);
 
  private:
-  // Invoked when the page wants to update the suggested text. If |user_text_|
-  // starts with |suggested_text|, then the delegate is notified of the change,
-  // which results in updating the omnibox.
-  void SetCompleteSuggestedText(const string16& suggested_text);
+  // Returns true if we should update immediately.
+  bool ShouldUpdateNow(TemplateURLID instant_id, const GURL& url);
 
-  // Invoked to show the preview. This is invoked in two possible cases: when
-  // the renderer paints, or when an auth dialog is shown. This notifies the
-  // delegate the preview is ready to be shown.
-  void ShowPreview();
+  // Schedules a delayed update to load the specified url.
+  void ScheduleUpdate(const GURL& url);
 
-  // Invoked once the page has finished loading and the script has been sent.
-  void PageFinishedLoading();
+  // Invoked from the timer to process the last scheduled url.
+  void ProcessScheduledUpdate();
 
   // Updates InstantLoaderManager and its current InstantLoader. This is invoked
   // internally from Update.
@@ -131,7 +129,6 @@ class InstantController : public InstantLoaderDelegate {
                     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|.
@@ -147,6 +144,10 @@ class InstantController : public InstantLoaderDelegate {
   // Clears the set of search engines blacklisted.
   void ClearBlacklist();
 
+  // Returns the id of the template url to use for the specified
+  // AutocompleteMatch.
+  TemplateURLID GetTemplateURLID(const AutocompleteMatch& match);
+
   InstantDelegate* delegate_;
 
   // The TabContents last passed to |Update|.
@@ -173,6 +174,11 @@ class InstantController : public InstantLoaderDelegate {
   // reset/commit.
   std::set<TemplateURLID> blacklisted_ids_;
 
+  base::OneShotTimer<InstantController> update_timer_;
+
+  // URL last pased to ScheduleUpdate.
+  GURL scheduled_url_;
+
   DISALLOW_COPY_AND_ASSIGN(InstantController);
 };
 
diff --git a/chrome/browser/instant/instant_loader.cc b/chrome/browser/instant/instant_loader.cc
index dbcbdc2..0ab0731 100644
--- a/chrome/browser/instant/instant_loader.cc
+++ b/chrome/browser/instant/instant_loader.cc
@@ -518,10 +518,10 @@ InstantLoader::~InstantLoader() {
 }
 
 void InstantLoader::Update(TabContents* tab_contents,
+                           TemplateURLID template_url_id,
                            const GURL& url,
                            PageTransition::Type transition_type,
                            const string16& user_text,
-                           const TemplateURL* template_url,
                            string16* suggested_text) {
   if (url_ == url)
     return;
@@ -557,8 +557,8 @@ void InstantLoader::Update(TabContents* tab_contents,
   }
   preview_tab_contents_delegate_->PrepareForNewLoad();
 
-  if (template_url) {
-    DCHECK(template_url_id_ == template_url->id());
+  if (template_url_id) {
+    DCHECK(template_url_id_ == template_url_id);
     if (!created_preview_contents) {
       if (is_waiting_for_load()) {
         // The page hasn't loaded yet. We'll send the script down when it does.
@@ -571,6 +571,7 @@ void InstantLoader::Update(TabContents* tab_contents,
         *suggested_text = complete_suggested_text_.substr(user_text_.size());
       }
     } else {
+      initial_instant_url_ = url;
       preview_contents_->controller().LoadURL(url, GURL(), transition_type);
       frame_load_observer_.reset(new FrameLoadObserver(this, user_text_));
     }
@@ -714,7 +715,14 @@ gfx::Rect InstantLoader::GetOmniboxBoundsInTermsOfPreview() {
 }
 
 void InstantLoader::PageDoesntSupportInstant(bool needs_reload) {
+  GURL url_to_load = url_;
+
+  // Because we didn't process any of the requests to load in Update we're
+  // actually at initial_instant_url_. We need to reset url_ so that callers see
+  // the correct state.
+  url_ = initial_instant_url_;
+
   frame_load_observer_.reset(NULL);
 
-  delegate_->InstantLoaderDoesntSupportInstant(this, needs_reload);
+  delegate_->InstantLoaderDoesntSupportInstant(this, needs_reload, url_to_load);
 }
diff --git a/chrome/browser/instant/instant_loader.h b/chrome/browser/instant/instant_loader.h
index 72ea5e7..ade9018 100644
--- a/chrome/browser/instant/instant_loader.h
+++ b/chrome/browser/instant/instant_loader.h
@@ -38,10 +38,10 @@ 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,
+              TemplateURLID template_url_id,
               const GURL& url,
               PageTransition::Type transition_type,
               const string16& user_text,
-              const TemplateURL* template_url,
               string16* suggested_text);
 
   // Sets the bounds of the omnibox (in screen coordinates). The bounds are
@@ -131,6 +131,9 @@ class InstantLoader {
   // The url we're displaying.
   GURL url_;
 
+  // The URL first used to load instant results.
+  GURL initial_instant_url_;
+
   // Delegate of the preview TabContents. Used to detect when the user does some
   // gesture on the TabContents and the preview needs to be activated.
   scoped_ptr<TabContentsDelegateImpl> preview_tab_contents_delegate_;
diff --git a/chrome/browser/instant/instant_loader_delegate.h b/chrome/browser/instant/instant_loader_delegate.h
index d3938b1..106ebdc 100644
--- a/chrome/browser/instant/instant_loader_delegate.h
+++ b/chrome/browser/instant/instant_loader_delegate.h
@@ -8,6 +8,8 @@
 
 #include "base/string16.h"
 
+class GURL;
+
 namespace gfx {
 class Rect;
 }
@@ -36,9 +38,10 @@ class InstantLoaderDelegate {
   // 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.
+  // has changed since the initial url, and |url_to_load| is that url.
   virtual void InstantLoaderDoesntSupportInstant(InstantLoader* loader,
-                                                 bool needs_reload) = 0;
+                                                 bool needs_reload,
+                                                 const GURL& url_to_load) = 0;
 
  protected:
   virtual ~InstantLoaderDelegate() {}
diff --git a/chrome/browser/instant/instant_loader_manager.cc b/chrome/browser/instant/instant_loader_manager.cc
index 3b5e648..3020f2f 100644
--- a/chrome/browser/instant/instant_loader_manager.cc
+++ b/chrome/browser/instant/instant_loader_manager.cc
@@ -86,6 +86,11 @@ InstantLoader* InstantLoaderManager::UpdateLoader(
   return active_loader();
 }
 
+bool InstantLoaderManager::WillUpateChangeActiveLoader(
+    TemplateURLID instant_id) {
+  return !active_loader() || active_loader()->template_url_id() != instant_id;
+}
+
 void InstantLoaderManager::MakePendingCurrent(
     scoped_ptr<InstantLoader>* old_loader) {
   DCHECK(current_loader_);
diff --git a/chrome/browser/instant/instant_loader_manager.h b/chrome/browser/instant/instant_loader_manager.h
index 9d0f087..f9c0bfd 100644
--- a/chrome/browser/instant/instant_loader_manager.h
+++ b/chrome/browser/instant/instant_loader_manager.h
@@ -43,6 +43,10 @@ class InstantLoaderManager {
   InstantLoader* UpdateLoader(TemplateURLID instant_id,
                               scoped_ptr<InstantLoader>* old_loader);
 
+  // Returns true if invoking |UpdateLoader| with |instant_id| would change the
+  // active loader.
+  bool WillUpateChangeActiveLoader(TemplateURLID instant_id);
+
   // Makes the pending loader the current loader. If ownership of the old
   // loader is to pass to the caller |old_loader| is set appropriately.
   void MakePendingCurrent(scoped_ptr<InstantLoader>* old_loader);
diff --git a/chrome/browser/instant/instant_loader_manager_unittest.cc b/chrome/browser/instant/instant_loader_manager_unittest.cc
index b689e11..d76f8f3 100644
--- a/chrome/browser/instant/instant_loader_manager_unittest.cc
+++ b/chrome/browser/instant/instant_loader_manager_unittest.cc
@@ -31,7 +31,8 @@ class InstantLoaderDelegateImpl : public InstantLoaderDelegate {
   }
 
   virtual void InstantLoaderDoesntSupportInstant(InstantLoader* loader,
-                                                 bool needs_reload) {
+                                                 bool needs_reload,
+                                                 const GURL& url_to_load) {
   }
 
  private:
@@ -249,3 +250,27 @@ TEST_F(InstantLoaderManagerTest, DestroyPendingLoader) {
   EXPECT_EQ(NULL, manager.pending_loader());
   EXPECT_EQ(first_loader, manager.current_loader());
 }
+
+// Makes sure WillUpateChangeActiveLoader works.
+TEST_F(InstantLoaderManagerTest, WillUpateChangeActiveLoader) {
+  InstantLoaderDelegateImpl delegate;
+  InstantLoaderManager manager(&delegate);
+  scoped_ptr<InstantLoader> loader;
+
+  // When there is no loader WillUpateChangeActiveLoader should return true.
+  EXPECT_TRUE(manager.WillUpateChangeActiveLoader(0));
+  EXPECT_TRUE(manager.WillUpateChangeActiveLoader(1));
+
+  // Add a loder with id 0 and test again.
+  manager.UpdateLoader(0, &loader);
+  EXPECT_FALSE(manager.WillUpateChangeActiveLoader(0));
+  EXPECT_TRUE(manager.WillUpateChangeActiveLoader(1));
+  ASSERT_TRUE(manager.active_loader());
+  MarkReady(manager.active_loader());
+
+  // Add a loader with id 1 and test again.
+  manager.UpdateLoader(1, &loader);
+  EXPECT_TRUE(manager.WillUpateChangeActiveLoader(0));
+  EXPECT_FALSE(manager.WillUpateChangeActiveLoader(1));
+}
+