Move instant support to SearchTabHelper.

chrome/browser/ui/search/instant_page.cc

Index: chrome/browser/ui/search/instant_page.cc
diff --git a/chrome/browser/ui/search/instant_page.cc b/chrome/browser/ui/search/instant_page.cc
index ce3fc9f81a55ba64f807a46f56915814a22f15b1..b92c89befe77633ac872613c470ea7fd0f34ef40 100644
--- a/chrome/browser/ui/search/instant_page.cc
+++ b/chrome/browser/ui/search/instant_page.cc
@@ -5,6 +5,9 @@
 #include "chrome/browser/ui/search/instant_page.h"
 
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/search/search.h"
+#include "chrome/browser/ui/search/search_model.h"
+#include "chrome/browser/ui/search/search_tab_helper.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/navigation_controller.h"
@@ -19,6 +22,8 @@ InstantPage::Delegate::~Delegate() {
 }
 
 InstantPage::~InstantPage() {
+  if (extended_enabled_ && contents())

[samarth, 2013/05/10 01:32:38]

No reason to both checking extended_enabled_ now.

[kmadhusu, 2013/05/13 17:13:24]

Done.

+    GetSearchModel()->RemoveObserver(this);
 }
 
 bool InstantPage::IsLocal() const {
@@ -67,16 +72,6 @@ void InstantPage::InitializeFonts() {
       routing_id(), omnibox_font_name, omnibox_font_size));
 }
 
-void InstantPage::DetermineIfPageSupportsInstant() {
-  if (IsLocal()) {
-    // Local pages always support Instant. That's why we keep them around.
-    int page_id = contents()->GetController().GetActiveEntry()->GetPageID();
-    OnInstantSupportDetermined(page_id, true);
-  } else {
-    Send(new ChromeViewMsg_DetermineIfPageSupportsInstant(routing_id()));
-  }
-}
-
 void InstantPage::SendAutocompleteResults(
     const std::vector<InstantAutocompleteResult>& results) {
   Send(new ChromeViewMsg_SearchBoxAutocompleteResults(routing_id(), results));
@@ -121,12 +116,25 @@ void InstantPage::SendMostVisitedItems(
 InstantPage::InstantPage(Delegate* delegate, const std::string& instant_url)
     : delegate_(delegate),
       instant_url_(instant_url),
+      extended_enabled_(chrome::IsInstantExtendedAPIEnabled()),

[samarth, 2013/05/10 01:32:38]

Not necessary anymore not that regular Instant is gone.

[kmadhusu, 2013/05/13 17:13:24]

Done.

       supports_instant_(false),
       instant_support_determined_(false) {
 }
 
 void InstantPage::SetContents(content::WebContents* contents) {
+  if (extended_enabled_ && web_contents())
+    GetSearchModel()->RemoveObserver(this);
+
   Observe(contents);
+
+  if (extended_enabled_ && contents) {
+    SearchModel* model = GetSearchModel();
+    model->AddObserver(this);
+
+    // Already know whether the page supports instant.
+    if (model->instant_support() != INSTANT_SUPPORT_UNKNOWN)
+      SetSupportsInstant(model->instant_support() == INSTANT_SUPPORT_YES);
+  }
 }
 
 bool InstantPage::ShouldProcessRenderViewCreated() {
@@ -162,21 +170,10 @@ void InstantPage::RenderViewCreated(content::RenderViewHost* render_view_host) {
     delegate_->InstantPageRenderViewCreated(contents());
 }
 
-void InstantPage::DidFinishLoad(
-    int64 /* frame_id */,
-    const GURL& /* validated_url */,
-    bool is_main_frame,
-    content::RenderViewHost* /* render_view_host */) {
-  if (is_main_frame && !supports_instant_)
-    DetermineIfPageSupportsInstant();
-}
-
 bool InstantPage::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(InstantPage, message)
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_SetSuggestions, OnSetSuggestions)
-    IPC_MESSAGE_HANDLER(ChromeViewHostMsg_InstantSupportDetermined,
-                        OnInstantSupportDetermined)
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_ShowInstantOverlay,
                         OnShowInstantOverlay)
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_FocusOmnibox, OnFocusOmnibox)
@@ -229,22 +226,20 @@ void InstantPage::DidFailProvisionalLoad(
     delegate_->InstantPageLoadFailed(contents());
 }
 
-void InstantPage::OnSetSuggestions(
-    int page_id,
-    const std::vector<InstantSuggestion>& suggestions) {
-  if (contents()->IsActiveEntry(page_id)) {
-    OnInstantSupportDetermined(page_id, true);
-    if (ShouldProcessSetSuggestions())
-      delegate_->SetSuggestions(contents(), suggestions);
-  }
+void InstantPage::ModelChanged(const SearchModel::State& old_state,
+                               const SearchModel::State& new_state) {
+  if (old_state.instant_support != new_state.instant_support)
+    SetSupportsInstant(new_state.instant_support == INSTANT_SUPPORT_YES);
 }
 
-void InstantPage::OnInstantSupportDetermined(int page_id,
-                                             bool supports_instant) {
-  if (!contents()->IsActiveEntry(page_id) || supports_instant_) {
-    // Nothing to do if the page already supports Instant.
+SearchModel* InstantPage::GetSearchModel() {
+  return contents() ?
+      SearchTabHelper::FromWebContents(contents())->model() : NULL;
+}
+
+void InstantPage::SetSupportsInstant(bool supports_instant) {
+  if (instant_support_determined_ && supports_instant_ == supports_instant)
     return;
-  }
 
   instant_support_determined_ = true;
   supports_instant_ = supports_instant;

[samarth, 2013/05/10 01:32:38]

Currently, once an InstantPage supports instant, we assume it will continue to
for its existence.  We should either:

(1) Keep this assumption (in which case, move that logic to SearchTabHelper).

(2) Fix this assumption.  This is really only an issue for InstantTab, because
with InstantOverlay and InstantNTP, we simply delete the contents and reload new
ones if instant support comes back as false.  But for InstantTab, we can handle
this gracefully because we're already listening to SearchModelChanged in
BrowserInstantController.  So we can have it trigger a ResetInstantTab in
InstantController.

I definitely prefer (2) but Sreeram may have other concerns here, so you should
probably run that by him before doing all that work.

[samarth, 2013/05/10 14:45:56]

Thinking about this some more, doing (2) is still a good idea but will require
some more changes (in particular with how and when we switch between using and
an InstantTab vs an Overlay).  Let's do it in a follow-up CL.

[kmadhusu, 2013/05/13 17:13:24]

ok.

@@ -252,14 +247,26 @@ void InstantPage::OnInstantSupportDetermined(int page_id,
 
   // If the page doesn't support Instant, stop listening to it.
   if (!supports_instant)
-    Observe(NULL);
+    SetContents(NULL);
+}
+
+void InstantPage::OnSetSuggestions(
+    int page_id,
+    const std::vector<InstantSuggestion>& suggestions) {
+  if (contents()->IsActiveEntry(page_id)) {
+    SearchTabHelper::FromWebContents(contents())->
+        OnInstantSupportDeterminedMsgReceived(page_id, true);
+    if (ShouldProcessSetSuggestions())
+      delegate_->SetSuggestions(contents(), suggestions);
+  }
 }
 
 void InstantPage::OnShowInstantOverlay(int page_id,
                                        int height,
                                        InstantSizeUnits units) {
   if (contents()->IsActiveEntry(page_id)) {
-    OnInstantSupportDetermined(page_id, true);
+    SearchTabHelper::FromWebContents(contents())->
+        OnInstantSupportDeterminedMsgReceived(page_id, true);
     if (ShouldProcessShowInstantOverlay())
       delegate_->ShowInstantOverlay(contents(), height, units);
   }
@@ -267,7 +274,8 @@ void InstantPage::OnShowInstantOverlay(int page_id,
 
 void InstantPage::OnFocusOmnibox(int page_id, OmniboxFocusState state) {
   if (contents()->IsActiveEntry(page_id)) {
-    OnInstantSupportDetermined(page_id, true);
+    SearchTabHelper::FromWebContents(contents())->
+        OnInstantSupportDeterminedMsgReceived(page_id, true);
     if (ShouldProcessFocusOmnibox())
       delegate_->FocusOmnibox(contents(), state);
   }
@@ -278,7 +286,8 @@ void InstantPage::OnSearchBoxNavigate(int page_id,
                                       content::PageTransition transition,
                                       WindowOpenDisposition disposition) {
   if (contents()->IsActiveEntry(page_id)) {
-    OnInstantSupportDetermined(page_id, true);
+    SearchTabHelper::FromWebContents(contents())->
+        OnInstantSupportDeterminedMsgReceived(page_id, true);
     if (ShouldProcessNavigateToURL())
       delegate_->NavigateToURL(contents(), url, transition, disposition);
   }