Persist the Instant API to committed search result pages.

chrome/browser/instant/instant_controller.cc

 // Copyright 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/instant/instant_controller.h"
 
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
@@ skipping 158 matching lines @@
 
   DVLOG(1) << "Update: " << AutocompleteMatch::TypeToString(match.type)
            << " user_text='" << user_text << "' full_text='" << full_text << "'"
            << " verbatim=" << verbatim << " typing=" << user_input_in_progress
            << " popup=" << omnibox_popup_is_open;
 
   // If the popup is open, the user has to be typing.
   DCHECK(!omnibox_popup_is_open || user_input_in_progress);
 
   // If the popup is closed, there should be no inline autocompletion.
-  DCHECK(omnibox_popup_is_open || user_text == full_text) << user_text << "|"
-                                                          << full_text;
+  DCHECK(omnibox_popup_is_open || user_text.empty() || user_text == full_text)
+      << user_text << "|" << full_text;
 
   // If there's inline autocompletion, the query has to be verbatim.
-  DCHECK(user_text == full_text || verbatim) << user_text << "|" << full_text;
+  DCHECK(user_text.empty() || user_text == full_text || verbatim)
+      << user_text << "|" << full_text;
 
   // If there's no text in the omnibox, the user can't have typed any.
   DCHECK(!full_text.empty() || user_text.empty()) << user_text;
 
   // If the user isn't typing, and the popup is closed, there can't be any
   // user-typed text.
   DCHECK(user_input_in_progress || omnibox_popup_is_open || user_text.empty())
       << user_text;
 
   // In non-extended mode, SearchModeChanged() is never called, so fake it. The
   // mode is set to "disallow suggestions" here, so that if one of the early
   // "return false" conditions is hit, suggestions will be disallowed. If the
   // query is sent to the loader, the mode is set to "allow" further below.
   if (!extended_enabled_)
     search_mode_.mode = chrome::search::Mode::MODE_DEFAULT;
 
   // If there's no active tab, the browser is closing.
   const TabContents* const active_tab = browser_->GetActiveTabContents();
   if (!active_tab) {
     Hide(true);
     return false;
   }
 
+  // Ensure we have a loader that can process this match. First, try to use the
+  // TemplateURL of the |match|. If that's invalid, in non-extended mode, stop.
+  // In extended mode, try using the default search engine, but only when the
+  // match is for a URL (i.e., not some other kind of non-Instant search).
+  // A completely blank query shows up as a search, and we do want to allow
+  // that, hence the "!user_text.empty()" clause.
+  Profile* const profile = active_tab->profile();
+  const bool match_is_search = AutocompleteMatch::IsSearchType(match.type);
+  if (!ResetLoader(match.GetTemplateURL(profile, false), active_tab) &&

[Jered, 2012/11/27 19:21:57]

Why reset the loader here if client_ is set?

[sreeram, 2012/11/29 07:33:19]

Done.

+      (!extended_enabled_ || (match_is_search && !user_text.empty()) ||
+       (!client_ && !CreateDefaultLoader()))) {
+    Hide(true);
+    return false;
+  }
+
   // Legend: OPIO == |omnibox_popup_is_open|, UIIP = |user_input_in_progress|.
   //
   //  #  OPIO UIIP full_text  Notes
   //  -  ---- ---- ---------  -----
   //  1   no   no    blank    } Navigation, or user hit Escape. |full_text| is
   //  2   no   no  non-blank  } blank if the page is NTP, non-blank otherwise.
   //
   //  3   no  yes    blank    User backspaced away all omnibox text.
   //
   //  4   no  yes  non-blank  User switched to a tab with a partial query.
   //
   //  5  yes   no    blank    } Impossible. DCHECK()ed above.
   //  6  yes   no  non-blank  }
   //
   //  7  yes  yes    blank    User typed a "?" into the omnibox.
   //
   //  8  yes  yes  non-blank  User typed text into the omnibox.
   //
   //  In non-extended mode, #1 to #7 call Hide(). #8 calls loader_->Update().
   //
   //  In extended mode, #2 and #4 call Hide(). #1 doesn't Hide() as the preview
   //  may be showing custom NTP content, but doesn't Update() either. #3 and #7
-  //  don't Hide(), but send a blank query to Update(). #8 calls Update().
+  //  don't Hide(), and send a blank query to Update(). #8 calls Update().
 
   if (extended_enabled_) {
     if (!omnibox_popup_is_open) {
-      if (!full_text.empty()) {
-        Hide(true);
-        return false;
+      if (!user_input_in_progress) {
+        if (!full_text.empty()) {
+          Hide(true);  // #2

[samarth, 2012/11/27 18:09:43]

Mostly for my understanding: Hide() will be a no-op here if client_ is set,
right? If so, it might be clearer to make that explicit by checking if
(!client_) before calling it.

[sreeram, 2012/11/29 07:33:19]

No, Hide() won't be a no-op. For one, it will actually hide the preview (i.e.,
owned by |loader_|). It will also reset last_full_text_, which also isn't a
no-op.

+          // If the user hit Escape when on a search results page, restore the
+          // original results by resending the query. If the user was not on a
+          // search results page, SearchModeChanged() would've caught it, and
+          // |client_| will be NULL, so we won't accidentally send non-query
+          // |full_text| to |client_|. Except if the user just switched tabs.
+          // Hence the comparison of WebContents, to catch that case as well.
+          if (client_ && client_->contents() == active_tab->web_contents())
+            client_->Update(full_text, true);
+        }
+        return false;  // #1
+      } else {
+        if (!full_text.empty()) {
+          Hide(true);  // #4
+          if (match_is_search) {
+            // The user just switched to a tab with a partial query already in
+            // the omnibox. This tab may or may not be a search results page
+            // (SearchModeChanged() hasn't been called yet). So, assume that it
+            // could be, and store |full_text| so that if the user then hits
+            // Enter, we'll send the correct query in client_->Submit().
+            last_full_text_ = full_text;
+            last_match_was_search_ = true;
+          }
+          return false;
+        }
       }
-      if (!user_input_in_progress && full_text.empty())
-        return false;
     }
   } else if (!omnibox_popup_is_open || full_text.empty()) {
     // Update() can be called if the user clicks the preview while composing
     // text with an IME. If so, we should commit on mouse up, so don't Hide().
-    if (!GetPreviewContents() || !loader_->IsPointerDownFromActivate())
+    if (!loader_ || !loader_->IsPointerDownFromActivate())
       Hide(true);
     return false;
   }
 
-  // Ensure we have a loader that can process this match. First, try to use the
-  // TemplateURL of the |match|. If that's invalid, in non-extended mode, stop.
-  // In extended mode, try using the default search engine, but only when the
-  // match is for a URL (i.e., not some other kind of non-Instant search).
-  // A completely blank query shows up as a search, and we do want to allow
-  // that, hence the "!full_text.empty()" clause.
-  Profile* const profile = active_tab->profile();
-  const bool match_is_search = AutocompleteMatch::IsSearchType(match.type);
-  if (!ResetLoader(match.GetTemplateURL(profile, false), active_tab) &&
-      (!extended_enabled_ || (match_is_search && !full_text.empty()) ||
-       !CreateDefaultLoader())) {
-    Hide(true);
-    return false;
-  }
-
   // If the user continues typing the same query as the suggested text is
   // showing, reuse the suggestion (but only for INSTANT_COMPLETE_NEVER).
   bool reused_suggestion = false;
   if (last_suggestion_.behavior == INSTANT_COMPLETE_NEVER) {
     if (StartsWith(last_user_text_, user_text, false) && !user_text.empty()) {
       // The user is backspacing away characters.
       last_suggestion_.text.insert(0, last_user_text_, user_text.size(),
                                    last_user_text_.size() - user_text.size());
       reused_suggestion = true;
     } else if (StartsWith(user_text, last_user_text_, false)) {
       // The user is typing forward. Normalize any added characters.
       reused_suggestion = NormalizeAndStripPrefix(&last_suggestion_.text,
           string16(user_text, last_user_text_.size()));
     }
   }
 
   last_user_text_ = user_text;
   last_full_text_ = full_text;
   last_verbatim_ = verbatim;
 
   if (!reused_suggestion)
     last_suggestion_ = InstantSuggestion();
 
   last_transition_type_ = match.transition;
   last_match_was_search_ = match_is_search;
   url_for_history_ = match.destination_url;
 
   // Store the first interaction time for use with latency histograms.
-  if (first_interaction_time_.is_null())
+  if (loader_ == CurrentClient() && first_interaction_time_.is_null())

[Jered, 2012/11/27 19:21:57]

Please extract a method like IsCommitted() for loader_ == CurrentClient() and
client_ == CurrentClient().

[sreeram, 2012/11/29 07:33:19]

I've gone with the simpler "if (instant_tab_)" idiom everywhere. I think this
reads reasonably well ("if [there's] an instant tab, ...").

     first_interaction_time_ = base::Time::Now();
 
   // Allow search suggestions. In extended mode, SearchModeChanged() will set
   // this, but it's not called in non-extended mode, so fake it.
   if (!extended_enabled_)
     search_mode_.mode = chrome::search::Mode::MODE_SEARCH_SUGGESTIONS;
 
-  loader_->Update(extended_enabled_ ? user_text : full_text, verbatim);
+  CurrentClient()->Update(extended_enabled_ ? user_text : full_text, verbatim);
+
+  if (loader_ == CurrentClient())
+    loader_->DidNavigate(history::HistoryAddPageArgs());
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_INSTANT_CONTROLLER_UPDATED,
       content::Source<InstantController>(this),
       content::NotificationService::NoDetails());
 
   // We don't have new suggestions yet, but we can either reuse the existing
   // suggestion or reset the existing "gray text".
   browser_->SetInstantSuggestion(last_suggestion_);
 
   // Though we may have handled a URL match above, we return false here, so that
   // omnibox prerendering can kick in. TODO(sreeram): Remove this (and always
   // return true) once we are able to commit URLs as well.
   return match_is_search;
 }
 
 // TODO(tonyg): This method only fires when the omnibox bounds change. It also
 // needs to fire when the preview bounds change (e.g.: open/close info bar).
 void InstantController::SetOmniboxBounds(const gfx::Rect& bounds) {
   if (!extended_enabled_ && !instant_enabled_)
     return;
 
   if (omnibox_bounds_ == bounds)

[samarth, 2012/11/28 06:31:15]

This no longer works correctly.  Well, we don't care about these bounds for
extended mode, but in general, we can no longer keep a copy of state here to
compare against since we now care about what was the last value sent to
CurrentClient().

One option is to stashed the cached values either in InstantLoader or in per-tab
profile data.  For committed tabs, you can check for changes and dispatch the
appropriate events when ResetClient is called. 

[sreeram, 2012/11/29 07:33:19]

As discussed offline, this does work correctly (because the bounds are always
sent to |loader_|). Further CLs, such as @melevin's, that need to send to either
|instant_tab_| or |loader_| should just send to both, and make sure that the
state is sent again when they are recreated.

     return;
 
   omnibox_bounds_ = bounds;
   if (omnibox_bounds_.height() > last_omnibox_bounds_.height()) {
     update_bounds_timer_.Stop();
     SendBoundsToPage();
   } else if (!update_bounds_timer_.IsRunning()) {
     update_bounds_timer_.Start(FROM_HERE,
         base::TimeDelta::FromMilliseconds(kUpdateBoundsDelayMS), this,
         &InstantController::SendBoundsToPage);
   }
 }
 
 void InstantController::HandleAutocompleteResults(
     const std::vector<AutocompleteProvider*>& providers) {
   if (!extended_enabled_)
     return;
 

[sky, 2012/11/27 01:07:51]

Shouldn't we early out if no client?

[sreeram, 2012/11/29 07:33:19]

Done.

-  if (!GetPreviewContents())
-    return;
-
   DVLOG(1) << "AutocompleteResults:";
   std::vector<InstantAutocompleteResult> results;
   for (ACProviders::const_iterator provider = providers.begin();
        provider != providers.end(); ++provider) {
     for (ACMatches::const_iterator match = (*provider)->matches().begin();
          match != (*provider)->matches().end(); ++match) {
       InstantAutocompleteResult result;
       result.provider = UTF8ToUTF16((*provider)->GetName());
       result.type = UTF8ToUTF16(AutocompleteMatch::TypeToString(match->type));
       result.description = match->description;
       result.destination_url = UTF8ToUTF16(match->destination_url.spec());
       result.relevance = match->relevance;
       DVLOG(1) << "    " << result.relevance << " " << result.type << " "
                << result.provider << " " << result.destination_url << " '"
                << result.description << "'";
       results.push_back(result);
     }
   }
 
-  loader_->SendAutocompleteResults(results);
+  if (InstantClient* client = CurrentClient())

[samarth, 2012/11/27 18:09:43]

nit: does the Chrome style guide allow this? I haven't seen assignment in
conditionals elsewhere.

[sreeram, 2012/11/29 07:33:19]

The style guide doesn't mention it. @sky hates it. I like this style, but am
cognizant that I might be overdoing it. I think it helps in some cases; for
example, see InstantLoader::SetupPreviewContents().

+    client->SendAutocompleteResults(results);
 }
 
 bool InstantController::OnUpOrDownKeyPressed(int count) {
   if (!extended_enabled_)
     return false;
 
-  if (!GetPreviewContents())
-    return false;
+  if (InstantClient* client = CurrentClient()) {
+    client->UpOrDownKeyPressed(count);
+    return true;
+  }
 
-  loader_->OnUpOrDownKeyPressed(count);
-  return true;
+  return false;
 }
 
 TabContents* InstantController::GetPreviewContents() const {
   return loader_ ? loader_->preview_contents() : NULL;
 }
 
 bool InstantController::IsCurrent() const {

[samarth, 2012/11/27 18:09:43]

What should IsCurrent return if client_ is set? For instance, part of the check
you have below for client_ in CommitIfCurrent could be moved here.

The only other place where IsCurrent is used is in OmniboxLostFocus. Is there
any reason that isn't just a no-op (beyond updating if_focused) when client_ is
set?

[sreeram, 2012/11/29 07:33:19]

If |instant_tab_| is non-NULL, IsCurrent() should be false, since the preview
should have been hidden by then. I don't see a need to check instant_tab_ here
explicitly.
Not sure what you mean, but OmniboxLostFocus() could lead to the |loader_|
getting refreshed if IsCurrent() is false (among other things).

   return model_.mode().is_search_suggestions() && last_match_was_search_;
 }
 
 bool InstantController::CommitIfCurrent(InstantCommitType type) {
   if (!extended_enabled_ && !instant_enabled_)
     return false;
 
+  if (client_ == CurrentClient() && last_match_was_search_ &&

[Jered, 2012/11/27 19:21:57]

I am very confused. Why doesn't this cancel sometimes? What is the business
about loader_ below?

[sreeram, 2012/11/29 07:33:19]

Cancel() isn't applicable in the case of a committed tab. I've added a short
comment about why this block exists here; let me know if it is still unclear.

+      type == INSTANT_COMMIT_PRESSED_ENTER) {
+    client_->Submit(last_full_text_);
+    return true;
+  }
+
   if (!IsCurrent())
     return false;
 
   DVLOG(1) << "CommitIfCurrent";
-  TabContents* preview = loader_->ReleasePreviewContents(type, last_full_text_);
+
+  if (type == INSTANT_COMMIT_FOCUS_LOST)
+    loader_->Cancel(last_full_text_);
+  else
+    loader_->Submit(last_full_text_);
+  loader_->CleanupPreviewContents();
+
+  TabContents* preview = loader_->release_preview_contents();
 
   if (extended_enabled_) {
     // Consider what's happening:
     //   1. The user has typed a query in the omnibox and committed it (either
     //      by pressing Enter or clicking on the preview).
     //   2. We commit the preview to the tab strip, and tell the page.
     //   3. The page will update the URL hash fragment with the query terms.
     // After steps 1 and 3, the omnibox will show the query terms. However, if
     // the URL we are committing at step 2 doesn't already have query terms, it
     // will flash for a brief moment as a plain URL. So, avoid that flicker by
@@ skipping 83 matching lines @@
   if (!IsCurrent() || !GetPreviewContents()) {
     OnStaleLoader();
     return;
   }
 
 #if defined(OS_MACOSX)
   if (!loader_->IsPointerDownFromActivate())
     Hide(true);
 #else
   if (IsViewInContents(GetViewGainingFocus(view_gaining_focus),
-                       GetPreviewContents()->web_contents()))
+                       loader_->contents()))
     CommitIfCurrent(INSTANT_COMMIT_FOCUS_LOST);
   else
     Hide(true);
 #endif
 }
 
 void InstantController::OmniboxGotFocus() {
   DVLOG(1) << "OmniboxGotFocus";
   is_omnibox_focused_ = true;
 
   if (!extended_enabled_ && !instant_enabled_)
     return;
 
-  if (!GetPreviewContents())
+  if (!client_ && !loader_)
     CreateDefaultLoader();
 }
 
 void InstantController::SearchModeChanged(
     const chrome::search::Mode& old_mode,
     const chrome::search::Mode& new_mode) {
   if (!extended_enabled_)
     return;
 
   DVLOG(1) << "SearchModeChanged: [origin:mode] " << old_mode.origin << ":"
            << old_mode.mode << " to " << new_mode.origin << ":"
            << new_mode.mode;
   search_mode_ = new_mode;
 
   if (new_mode.is_search_suggestions()) {

[samarth, 2012/11/27 18:09:43]

Is this code path relevant when client_ is set? In general, I think we should be
more explicit here about only doing things that are necessary when using client_
or using loader_. Even if it's functionally equivalent, it makes it harder to
read the code because you can't tell what matters and what doesn't.

[sreeram, 2012/11/29 07:33:19]

I think there's a balance to be struck here. I don't want to see "if
(instant_tab_) return;" all over the place. I think you feel this way because
instant_tab_ is a new piece being introduced. Once it has been around a while, I
think we'll get used to the idea.

For example, why do we not have "if (!loader_) return;" or "if
(model_.mode().is_default()) return;" all over the place too? In many places, we
go ahead with the logic, end up calling Hide(), knowing that it might be
redundant - the preview may already be hidden, or worse still, we may not even
have a loader.

     // The preview is showing NTP content, but it's not appropriate anymore.
     if (model_.mode().is_ntp() && !new_mode.is_origin_ntp())
       Hide(false);
   } else {
     Hide(true);
   }
 
-  if (GetPreviewContents())
+  if (loader_)
     loader_->SearchModeChanged(new_mode);
+
+  ResetClient();
 }
 
 void InstantController::ActiveTabChanged() {
   if (!extended_enabled_ && !instant_enabled_)
     return;
 
   DVLOG(1) << "ActiveTabChanged";
 
   // By this time, SearchModeChanged() should've been called, so we only need to
   // handle the case when the search mode does NOT change, as in the case of
   // going from search_suggestions to search_suggestions (i.e., partial queries
   // on both old and new tabs).
   if (search_mode_.is_search_suggestions() &&
       model_.mode().is_search_suggestions())
     Hide(false);
+
+  if (extended_enabled_)
+    ResetClient();
 }
 
 void InstantController::SetInstantEnabled(bool instant_enabled) {
   instant_enabled_ = instant_enabled;
   if (!extended_enabled_ && !instant_enabled_)
     DeleteLoader();
 }
 
 void InstantController::ThemeChanged(const ThemeBackgroundInfo& theme_info) {
   if (!extended_enabled_)
     return;
 
-  if (GetPreviewContents())
+  if (loader_)
     loader_->SendThemeBackgroundInfo(theme_info);
 }
 
 void InstantController::ThemeAreaHeightChanged(int height) {
   if (!extended_enabled_)
     return;
 
-  if (GetPreviewContents())
+  if (loader_)
     loader_->SendThemeAreaHeight(height);
 }
 
 void InstantController::SetSuggestions(
-    InstantLoader* loader,
+    InstantClient* client,
     const std::vector<InstantSuggestion>& suggestions) {
   DVLOG(1) << "SetSuggestions";
-  if (loader_ != loader || !search_mode_.is_search_suggestions())
+  if (client != CurrentClient() || !search_mode_.is_search_suggestions())
     return;
 
   InstantSuggestion suggestion;
   if (!suggestions.empty())
     suggestion = suggestions[0];
 
   if (suggestion.behavior == INSTANT_COMPLETE_REPLACE) {
     // We don't get an Update() when changing the omnibox due to a REPLACE
     // suggestion (so that we don't inadvertently cause the preview to change
     // what it's showing, as the user arrows up/down through the page-provided
@@ skipping 62 matching lines @@
 }
 
 void InstantController::ShowInstantPreview(InstantLoader* loader,
                                            InstantShownReason reason,
                                            int height,
                                            InstantSizeUnits units) {
   if (loader_ == loader && extended_enabled_)
     Show(reason, height, units);
 }
 
-void InstantController::InstantSupportDetermined(InstantLoader* loader,
+void InstantController::InstantSupportDetermined(InstantClient* client,
                                                  bool supports_instant) {
+  if (client_ == client) {
+    if (!supports_instant) {
+      // TODO(sreeram): This should be scheduled for later deletion, but we need
+      // to disconnect the observer before doing so.
+      client_.reset();
+    }
+    return;
+  }
+
+  InstantLoader* loader = static_cast<InstantLoader*>(client);
   if (supports_instant) {
     blacklisted_urls_.erase(loader->instant_url());
   } else {
     ++blacklisted_urls_[loader->instant_url()];
     if (loader_ == loader)
       DeleteLoader();
   }
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_INSTANT_SUPPORT_DETERMINED,
       content::Source<InstantController>(this),
       content::NotificationService::NoDetails());
 }
 
 void InstantController::SwappedTabContents(InstantLoader* loader) {
   if (loader_ == loader)
     model_.SetPreviewContents(GetPreviewContents());
 }
 
 void InstantController::InstantLoaderContentsFocused(InstantLoader* loader) {
 #if defined(USE_AURA)
   // On aura the omnibox only receives a focus lost if we initiate the focus
   // change. This does that.
   if (loader_ == loader && !model_.mode().is_default())
     browser_->InstantPreviewFocused();
 #endif
 }
 
+void InstantController::ResetClient() {
+  if (search_mode_.is_origin_search()) {
+    content::WebContents* contents =
+        browser_->GetActiveTabContents()->web_contents();
+    if (!client_ || contents != client_->contents()) {
+      client_.reset(new InstantClient(this, contents));
+      client_->DetermineIfPageSupportsInstant();
+    }
+    // We are now using |client_| instead of |loader_|, so Hide() the latter. We
+    // want to call Hide(true) to clear old query results on the |loader_|, but
+    // that would also clear |last_full_text_|, which is bad if the user then
+    // immediately tries to commit the query on |client_|.
+    Hide(!search_mode_.is_search_suggestions());
+  } else {
+    client_.reset();
+  }
+}
+
 bool InstantController::ResetLoader(const TemplateURL* template_url,
                                     const TabContents* active_tab) {
   std::string instant_url;
   if (!GetInstantURL(template_url, &instant_url))
     return false;
 
-  if (GetPreviewContents() && loader_->instant_url() != instant_url)
+  if (loader_ && loader_->instant_url() != instant_url)
     DeleteLoader();
 
-  if (!GetPreviewContents()) {
+  if (!loader_) {
     loader_.reset(new InstantLoader(this, instant_url, active_tab));
     loader_->Init();
 
     // Ensure the searchbox API has the correct theme-related info and context.
     if (extended_enabled_) {
       browser_->UpdateThemeInfoForPreview();
       loader_->SearchModeChanged(search_mode_);
     }
 
     // Reset the loader timer.
@@ skipping 16 matching lines @@
       TemplateURLServiceFactory::GetForProfile(active_tab->profile())->
                                  GetDefaultSearchProvider();
 
   return ResetLoader(template_url, active_tab);
 }
 
 void InstantController::OnStaleLoader() {
   // If the preview is showing or the omnibox has focus, don't delete the
   // loader. It will get refreshed the next time the preview is hidden or the
   // omnibox loses focus.
-  if (!stale_loader_timer_.IsRunning() && !is_omnibox_focused_ &&
+  if (!stale_loader_timer_.IsRunning() && !is_omnibox_focused_ && !client_ &&
       model_.mode().is_default()) {
     DeleteLoader();
     CreateDefaultLoader();
   }
 }
 
 void InstantController::DeleteLoader() {
   // Clear all state, except |last_transition_type_| as it's used during commit.
   last_user_text_.clear();
   last_full_text_.clear();
@@ skipping 40 matching lines @@
     last_user_text_.clear();
     last_full_text_.clear();
   }
 
   OnStaleLoader();
 }
 
 void InstantController::Show(InstantShownReason reason,
                              int height,
                              InstantSizeUnits units) {
+  if (client_ == CurrentClient())
+    return;
+
   DVLOG(1) << "Show: reason=" << reason << " height=" << height << " units="
            << units;
 
   // Must be on NTP to show NTP content.
   if (reason == INSTANT_SHOWN_CUSTOM_NTP_CONTENT && !search_mode_.is_ntp())
     return;
 
   // Must have updated omnibox after most recent Hide() to show suggestions.
   if (reason == INSTANT_SHOWN_QUERY_SUGGESTIONS &&
       !search_mode_.is_search_suggestions())
     return;
 
   // If the preview is being shown because of the first set of suggestions to
   // arrive for this query editing session, record a histogram value.
   if (!first_interaction_time_.is_null() && model_.mode().is_default()) {
     base::TimeDelta delta = base::Time::Now() - first_interaction_time_;
     UMA_HISTOGRAM_TIMES("Instant.TimeToFirstShow", delta);
   }
 
   model_.SetPreviewState(search_mode_, height, units);
 }
 
 void InstantController::SendBoundsToPage() {
-  if (last_omnibox_bounds_ == omnibox_bounds_ ||
-      !GetPreviewContents() || loader_->IsPointerDownFromActivate())
+  if (last_omnibox_bounds_ == omnibox_bounds_ || !loader_ ||
+      loader_->IsPointerDownFromActivate())
     return;
 
   last_omnibox_bounds_ = omnibox_bounds_;
   gfx::Rect preview_bounds = browser_->GetInstantBounds();
   gfx::Rect intersection = gfx::IntersectRects(omnibox_bounds_, preview_bounds);
 
   // Translate into window coordinates.
   if (!intersection.IsEmpty()) {
     intersection.Offset(-preview_bounds.origin().x(),
                         -preview_bounds.origin().y());
@@ skipping 55 matching lines @@
   }
 
   std::map<std::string, int>::const_iterator iter =
       blacklisted_urls_.find(*instant_url);
   if (iter != blacklisted_urls_.end() &&
       iter->second > kMaxInstantSupportFailures)
     return false;
 
   return true;
 }
+
+InstantClient* InstantController::CurrentClient() const {
+  return client_ ? client_.get() : loader_.get();
+}