InstantExtended: Transient naventry for preview.

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 6314a47f70ccc5f00ee9621f207d5c4be214c077..c7f442f58a353dfe2a0b064196a2bc6236f83225 100644
--- a/chrome/browser/instant/instant_controller.cc
+++ b/chrome/browser/instant/instant_controller.cc
@@ -180,7 +180,8 @@ InstantController::InstantController(chrome::BrowserInstantController* browser,
       omnibox_focus_state_(OMNIBOX_FOCUS_NONE),
       start_margin_(0),
       end_margin_(0),
-      allow_preview_to_show_search_suggestions_(false) {
+      allow_preview_to_show_search_suggestions_(false),
+      instant_set_transient_entry_(false) {
 }
 
 InstantController::~InstantController() {
@@ -689,8 +690,15 @@ void InstantController::ActiveTabChanged() {
 
 void InstantController::TabDeactivated(content::WebContents* contents) {
   DVLOG(1) << "TabDeactivated";
-  if (extended_enabled_ && !contents->IsBeingDestroyed())
-    CommitIfPossible(INSTANT_COMMIT_FOCUS_LOST);
+  if (extended_enabled_) {
+    if (!contents->IsBeingDestroyed()) {
+      CommitIfPossible(INSTANT_COMMIT_FOCUS_LOST);
+    } else {
+      // The tab is going away. Trying to restore its original transient entry
+      // would affect the wrong tab.
+      saved_transient_entry_.reset();

[sreeram, 2013/01/25 21:28:02]

Doesn't this break tab-restore? Quoting myself from earlier:
"""
Note that we can't just clear the state variables in InstantController alone. We
need to restore any previous transient entry (such as an SSL interstitial) of
the closing tab, to make sure tab-restore does the right thing.
"""

Test this scenario:
1. Have at least two tabs.
2. On tab A, navigate to ianfette.org. This should cause a malware interstitial.
3. Focus the omnibox and type a query (no Enter). Wait for the preview to
appear.
4. Close the tab.
5. Ctrl-Shift-T to restore the tab.

I'd expect that with the CL as it is, it would try to restore the Instant
overlay. This may seem okay, but would actually lead to trouble further down.
Because now, the restored tab has a transient Instant overlay, but not one that
InstantController knows about. So if you focus the omnibox and start typing
again, we'll end up treating the existing transient entry (Instant-created) as
something like the SSL interstitial, and replace it with yet another
Instant-created entry. Maybe it can all work, but it makes my head hurt.

So, I'd settle for having tab-restore the malware interstitial seen at step 2.

+    }
+  }
 }
 
 void InstantController::SetInstantEnabled(bool instant_enabled) {
@@ -909,6 +917,8 @@ void InstantController::InstantLoaderAboutToNavigateMainFrame(const GURL& url) {
   if (model_.mode().is_ntp() ||
       (url.host() != instant_url.host() || url.path() != instant_url.path())) {
     CommitIfPossible(INSTANT_COMMIT_NAVIGATED);
+  } else {
+    UpdateTransientHistoryEntry(url);
   }
 }
 
@@ -1088,6 +1098,8 @@ void InstantController::HideInternal() {
     loader_->Update(string16(), 0, 0, true);
   }
 
+  ResetTransientHistoryEntry();
+
   // Clear the first interaction timestamp for later use.
   first_interaction_time_ = base::Time();
 }
@@ -1145,6 +1157,51 @@ void InstantController::ShowLoader(InstantShownReason reason,
   // overlay (because we're not full height).
   if (reason == INSTANT_SHOWN_CLICKED_QUERY_SUGGESTION)
     CommitIfPossible(INSTANT_COMMIT_CLICKED_QUERY_SUGGESTION);
+  else if (extended_enabled_ && IsFullHeight(model_))
+    SetTransientHistoryEntry();
+}
+
+void InstantController::SetTransientHistoryEntry() {
+  content::WebContents* active_tab = browser_->GetActiveWebContents();
+  // Save any transient entry already associated with the active tab.
+  content::NavigationEntry* old_transient_entry =
+      active_tab->GetController().GetTransientEntry();
+  if (old_transient_entry && !instant_set_transient_entry_)
+    saved_transient_entry_.reset(
+        content::NavigationEntry::Create(*old_transient_entry));
+  // The loader's base navigation entry should describe the search provider
+  // without reference to a specific query. The alternative, updating the query
+  // in the tab title as the user types, would be distracting.
+  const content::NavigationEntry* base_entry = loader_->base_navigation_entry();
+  DCHECK(base_entry != NULL);
+  content::NavigationEntry* new_transient_entry =
+      content::NavigationEntry::Create(*base_entry);
+  active_tab->GetController().AddTransientEntry(new_transient_entry);
+  instant_set_transient_entry_ = true;
+}
+
+void InstantController::UpdateTransientHistoryEntry(const GURL& url) {
+  // URL should update so that if the user presses reload, we reload the page
+  // for the current query, but title remains unchanged so it doesn't flicker.
+  if (instant_set_transient_entry_) {
+    content::WebContents* active_tab = browser_->GetActiveWebContents();
+    content::NavigationEntry* transient_entry =
+        active_tab->GetController().GetTransientEntry();
+    transient_entry->SetURL(url);
+    transient_entry->SetVirtualURL(url);
+  }
+}
+
+void InstantController::ResetTransientHistoryEntry() {
+  content::NavigationEntry* old_transient_entry =
+      saved_transient_entry_.release();
+  if (old_transient_entry) {
+    content::WebContents* active_tab = browser_->GetActiveWebContents();
+    active_tab->GetController().AddTransientEntry(old_transient_entry);
+    chrome::search::SearchTabHelper::FromWebContents(active_tab)->
+        NavigationEntryUpdated();
+  }
+  instant_set_transient_entry_ = false;
 }
 
 void InstantController::SendPopupBoundsToPage() {