Instant Extended: Fallback to local preview if the remote instant page is not ready on user input.

chrome/browser/instant/instant_loader.h

Index: chrome/browser/instant/instant_loader.h
diff --git a/chrome/browser/instant/instant_loader.h b/chrome/browser/instant/instant_loader.h
index 185d2ff53e9317c3b7838c66ac3adf826f2e5c61..869abf0104fd81aeef9819416a14003951385ad5 100644
--- a/chrome/browser/instant/instant_loader.h
+++ b/chrome/browser/instant/instant_loader.h
@@ -82,6 +82,10 @@ class InstantLoader : public InstantClient::Delegate,
     return last_navigation_;
   }
 
+  // Tells the loader that it is in fallback mode.
+  void set_is_in_fallback_mode(bool value);
+  bool is_in_fallback_mode() const { return is_in_fallback_mode_; }

[sreeram, 2013/01/22 03:58:03]

Do we really need this? Can't we just use IsUsingLocalPreview()? If you are
thinking about incognito mode, we can surely adjust for it by also checking
use_local_preview_only_ appropriately.

[Shishir, 2013/01/25 22:11:26]

There are a couple of reasons why I think its better to have this. 

1. Using use_local_preview_only_and IsUsingLocalPreview() will not help us
distinguish the case where we are using the local preview because the instant
page is temporarily slow vs the case where we are using it becuase all other
instant urls are blacklisted (unless we want to call GetInstantURL).

2. We need EnsureLoaderIsCurrent to not delete the loader if it is in fallback
mode and delete it once MaybeSwitchLocalLoader is called.

I will send out an alternate patch that works around 1 and 2 but I am not sure
it is the better approach.

[sreeram, 2013/01/29 17:02:21]

Re: 1, I don't see the need to distinguish the cases. So yes, we should just use
GetInstantURL() and use it if it's not blacklisted.

Re: 2, Why not? In all the cases I see, EnsureLoaderIsCurrent() is only being
called when the preview (whether local or remote) is not being used. So, it
should go ahead and delete the preview and try the remote URL if possible.

[Shishir, 2013/01/29 20:06:31]

As discussed, we want to keep the EnsureLoaderIsCurrent call in Update. Made the
change to check fallback mode only in update.

+
   // Called by the history tab helper with information that it would have added
   // to the history service had this WebContents not been used for Instant.
   void DidNavigate(const history::HistoryAddPageArgs& add_page_args);
@@ -99,6 +103,7 @@ class InstantLoader : public InstantClient::Delegate,
   void Cancel(const string16& text);
   void SetPopupBounds(const gfx::Rect& bounds);
   void SetMarginSize(int start, int end);
+  void InitializeFonts();

[sreeram, 2013/01/22 03:58:03]

I realize that the reason why this was being done in InstantSupportDetermined
previously was so that the page was guaranteed to get an event with the font
information. I think that's wrong. The page should just check the font name and
size when it loads. If they are non-blank, it should use that information right
away. Otherwise, when they become available, the page will get an event, as
expected. In other words, the font information should be treated exactly the
same way searchbox.value is treated.

We should convey this to the google.com folks internally, so they know what to
do if stuff breaks due to this CL.

[Shishir, 2013/01/25 22:11:26]

As we discussed, the font information will now always be available as we
initialize it in RenderViewCreated.

   void SendAutocompleteResults(
       const std::vector<InstantAutocompleteResult>& results);
   void UpOrDownKeyPressed(int count);
@@ -148,6 +153,10 @@ class InstantLoader : public InstantClient::Delegate,
   bool is_pointer_down_from_activate_;
   history::HistoryAddPageArgs last_navigation_;
 
+  // Indicates if this loader is running as a fallback loader using the local
+  // preview because the actual instant page was too slow to load.
+  bool is_in_fallback_mode_;
+
   // Used to get notifications about renderers coming and going.
   content::NotificationRegistrar registrar_;