Fixes bug in instant where we would end up incorrectly using the

chrome/browser/instant/instant_controller.cc

Index: chrome/browser/instant/instant_controller.cc
diff --git a/chrome/browser/instant/instant_controller.cc b/chrome/browser/instant/instant_controller.cc
index 704f82cea608e947f837024d68c887e48337a08a..4b6b50aee01fd673c0e329f6607b14f4af83113f 100644
--- a/chrome/browser/instant/instant_controller.cc
+++ b/chrome/browser/instant/instant_controller.cc
@@ -39,8 +39,8 @@ InstantController::InstantController(Profile* profile,
                                      InstantDelegate* delegate)
     : delegate_(delegate),
       tab_contents_(NULL),
-      is_active_(false),
       is_displayable_(false),
+      is_out_of_date_(false),

[sreeram, 2011/10/14 20:07:32]

Should we start with is_out_of_date_ being true? Consider that the
OnAutocompleteGotFocus() method creates a loader, but doesn't set this to false.

       commit_on_mouse_up_(false),
       last_transition_type_(content::PAGE_TRANSITION_LINK),
       ALLOW_THIS_IN_INITIALIZER_LIST(destroy_factory_(this)) {
@@ -162,7 +162,7 @@ bool InstantController::Update(TabContentsWrapper* tab_contents,
   last_user_text_ = user_text;
 
   if (!ShouldUseInstant(match)) {
-    DestroyPreviewContentsAndLeaveActive();
+    Hide();
     return false;
   }
 
@@ -170,7 +170,6 @@ bool InstantController::Update(TabContentsWrapper* tab_contents,
   DCHECK(template_url);  // ShouldUseInstant returns false if no turl.
   if (!loader_.get())
     loader_.reset(new InstantLoader(this, template_url->id()));
-  is_active_ = true;
 
   // In some rare cases (involving group policy), Instant can go from the field
   // trial to normal mode, with no intervening call to DestroyPreviewContents()
@@ -208,15 +207,12 @@ void InstantController::DestroyPreviewContents() {
     return;
   }
 
-  // ReleasePreviewContents sets is_active_ to false, but we need to set it
-  // before notifying the delegate, otherwise if the delegate asks for the state
-  // we'll still be active.
-  is_active_ = false;
   delegate_->HideInstant();
   delete ReleasePreviewContents(INSTANT_COMMIT_DESTROY);
 }
 
-void InstantController::DestroyPreviewContentsAndLeaveActive() {
+void InstantController::Hide() {
+  is_out_of_date_ = true;
   commit_on_mouse_up_ = false;
   if (is_displayable_) {
     is_displayable_ = false;
@@ -243,6 +239,8 @@ bool InstantController::PrepareForCommit() {
     return false;
 
   const TemplateURL* template_url = model->GetDefaultSearchProvider();
+  // For the HIDDEN field trial we ignore |is_out_of_date_| as we're going to
+  // update the loader below with the correct value.

[sreeram, 2011/10/14 20:07:32]

Actually, we do care about is_out_of_date_ for the HIDDEN trial as well. At the
time of commit, we want to make sure that whatever we have stored in
last_user_text_ is still the current autocomplete match. Which is why if there
had been an intervening call to Update() with something that we shouldn't be
using Instant for (such as a URL), we would've cleared last_user_text_ in
Hide().

In short, "if (last_user_text_.empty())" serves the same purpose as "if
(is_out_of_date_)". Since you've now added is_out_of_date_, we might as well
stop mucking with last_user_text_ and just check is_out_of_date_ directly. But
of course, you'll need to set it to true in the if() block on lines 178-181
above (in addition to doing it in UpdateLoader()).

   if (last_user_text_.empty() ||
       !IsValidInstantTemplateURL(template_url) ||
       !loader_.get() ||
@@ -393,7 +391,6 @@ TabContentsWrapper* InstantController::ReleasePreviewContents(
 
   TabContentsWrapper* tab = loader_->ReleasePreviewContents(type);
   ClearBlacklist();
-  is_active_ = false;
   is_displayable_ = false;
   commit_on_mouse_up_ = false;
   omnibox_bounds_ = gfx::Rect();
@@ -464,7 +461,8 @@ void InstantController::SwappedTabContents(InstantLoader* loader) {
 
 void InstantController::UpdateIsDisplayable() {
   bool displayable =
-      (loader_.get() && loader_->ready() && loader_->http_status_ok());
+      (!is_out_of_date_ && loader_.get() && loader_->ready() &&
+       loader_->http_status_ok());
   if (displayable == is_displayable_)
     return;
 
@@ -486,6 +484,7 @@ void InstantController::UpdateLoader(const TemplateURL* template_url,
                                      const string16& user_text,
                                      bool verbatim,
                                      string16* suggested_text) {
+  is_out_of_date_ = false;
   loader_->SetOmniboxBounds(omnibox_bounds_);
   loader_->Update(tab_contents_, template_url, url, transition_type, user_text,
                   verbatim, suggested_text);