Instant: Use only one hidden WebContents per profile.

chrome/browser/instant/instant_controller.h

Index: chrome/browser/instant/instant_controller.h
diff --git a/chrome/browser/instant/instant_controller.h b/chrome/browser/instant/instant_controller.h
index 562089f3df5fd63d39b68f57bcdd3bd72a3b8bfa..c31fabf3492ef8b37cfb6c76ee9ceaa105cd65f8 100644
--- a/chrome/browser/instant/instant_controller.h
+++ b/chrome/browser/instant/instant_controller.h
@@ -5,444 +5,135 @@
 #ifndef CHROME_BROWSER_INSTANT_INSTANT_CONTROLLER_H_
 #define CHROME_BROWSER_INSTANT_INSTANT_CONTROLLER_H_
 
-#include <list>
-#include <map>
-#include <string>
-#include <utility>
 #include <vector>
 
 #include "base/basictypes.h"
-#include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/string16.h"
-#include "base/time.h"
-#include "base/timer.h"
-#include "chrome/browser/history/history_types.h"
 #include "chrome/browser/instant/instant_commit_type.h"
-#include "chrome/browser/instant/instant_model.h"
 #include "chrome/browser/instant/instant_page.h"
 #include "chrome/browser/ui/omnibox/omnibox_edit_model.h"
-#include "chrome/common/instant_types.h"
-#include "chrome/common/search_types.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
-#include "content/public/common/page_transition_types.h"
-#include "googleurl/src/gurl.h"
-#include "ui/base/window_open_disposition.h"
 #include "ui/gfx/native_widget_types.h"
-#include "ui/gfx/rect.h"
 
 struct AutocompleteMatch;
 class AutocompleteProvider;
-class InstantNTP;
-class InstantOverlay;
-class InstantTab;
-class TemplateURL;
+class InstantModel;
+class InstantModelObserver;
+class Profile;
 
 namespace chrome {
 class BrowserInstantController;
 }
 
+namespace chrome {
+namespace search {
+struct Mode;
+}
+}
+
 namespace content {
 class WebContents;
 }
 
-// Macro used for logging debug events. |message| should be a std::string.
-#define LOG_INSTANT_DEBUG_EVENT(controller, message) \
-    controller->LogDebugEvent(message)
+namespace gfx {
+class Rect;
+}
 
 // InstantController drives Chrome Instant, i.e., the browser implementation of
 // the Embedded Search API (see http://dev.chromium.org/embeddedsearch).
 //
-// In extended mode, InstantController maintains and coordinates three
-// instances of InstantPage:
-//  (1) An InstantOverlay instance that is used to show search suggestions and
-//      results in an overlay over a non-search page.
-//  (2) An InstantNTP instance which is a preloaded search page that will be
-//      swapped-in the next time the user navigates to the New Tab Page. It is
-//      never shown to the user in an uncommitted state.
-//  (3) An InstantTab instance which points to the currently active tab, if it
-//      supports the Embedded Search API.
+// There are two concrete modes of operation: Regular (InstantControllerImpl)
+// and Extended (InstantControllerExtendedImpl).
 //
-// All three are backed by a WebContents. InstantOverlay and InstantNTP own
-// their corresponding WebContents; InstantTab does not. In non-extended mode,
-// only an InstantOverlay instance is kept.
+// In both modes, InstantController can show an overlay WebContents as the user
+// types in the omnibox. In extended mode, InstantController can also use the
+// current active tab to drive the display of search suggestions and results.
 //
 // InstantController is owned by Browser via BrowserInstantController.
-class InstantController : public InstantPage::Delegate,
-                          public content::NotificationObserver {
+class InstantController : public InstantPage::Delegate {
  public:
-  InstantController(chrome::BrowserInstantController* browser,
-                    bool extended_enabled);
+  static scoped_ptr<InstantController> GetInstance(

[samarth, 2013/03/01 17:59:53]

GetInstance makes me think of singletons.  How about just Create()?

[samarth, 2013/03/01 17:59:53]

Also comments here.

[sreeram, 2013/03/07 18:18:46]

Done.

[sreeram, 2013/03/07 18:18:46]

Done. Create() along with the scoped_ptr<> return value seems self-evident, so
I'm not sure my comment adds much value. Let me know if I misunderstood you.

[samarth, 2013/03/11 19:03:24]

Not sure what I had in mind when I wrote that :) Agreed that the current comment
isn't particularly useful so feel free to take it out.

+      chrome::BrowserInstantController* browser,
+      Profile* profile);
+
+  InstantController();
   virtual ~InstantController();
 
-  // Invoked as the user types into the omnibox. |user_text| is what the user
-  // has typed. |full_text| is what the omnibox is showing. These may differ if
-  // the user typed only some text, and the rest was inline autocompleted. If
-  // |verbatim| is true, search results are shown for the exact omnibox text,
-  // rather than the best guess as to what the user means. Returns true if the
-  // update is accepted (i.e., if |match| is a search rather than a URL).
-  // |is_keyword_search| is true if keyword searching is in effect.
-  bool Update(const AutocompleteMatch& match,
-              const string16& user_text,
-              const string16& full_text,
-              size_t selection_start,
-              size_t selection_end,
-              bool verbatim,
-              bool user_input_in_progress,
-              bool omnibox_popup_is_open,
-              bool escape_pressed,
-              bool is_keyword_search);
-
-  // Releases and returns the NTP WebContents. May be NULL. Loads a new
-  // WebContents for the NTP.
-  scoped_ptr<content::WebContents> ReleaseNTPContents() WARN_UNUSED_RESULT;
-
-  // Sets the bounds of the omnibox popup, in screen coordinates.
-  void SetPopupBounds(const gfx::Rect& bounds);
-
-  // Sets the stored start-edge margin and width of the omnibox.
-  void SetOmniboxBounds(const gfx::Rect& bounds);
-
-  // Send autocomplete results from |providers| to the preview page.
-  void HandleAutocompleteResults(
-      const std::vector<AutocompleteProvider*>& providers);
-
-  // Called when the user presses up or down. |count| is a repeat count,
-  // negative for moving up, positive for moving down. Returns true if Instant
-  // handled the key press.
-  bool OnUpOrDownKeyPressed(int count);
-
-  // Called when the user has arrowed into the suggestions but wants to cancel,
-  // typically by pressing ESC. The omnibox text is expected to have been
-  // reverted to |full_text| by the OmniboxEditModel prior to calling this.
-  // |match| is the match reverted to.
-  void OnCancel(const AutocompleteMatch& match,
-                const string16& full_text);
-
-  // The preview WebContents. May be NULL. InstantController retains ownership.
-  content::WebContents* GetPreviewContents() const;
-
-  // Returns true if the Instant overlay is showing a search results preview.
-  bool IsPreviewingSearchResults() const;
-
-  // If the preview is showing search results, commits the preview, calling
-  // CommitInstant() on the browser, and returns true. Else, returns false.
-  bool CommitIfPossible(InstantCommitType type);
-
-  // Called to indicate that the omnibox focus state changed with the given
-  // |reason|. If |focus_state| is FOCUS_NONE, |view_gaining_focus| is set to
-  // the view gaining focus.
-  void OmniboxFocusChanged(OmniboxFocusState focus_state,
-                           OmniboxFocusChangeReason reason,
-                           gfx::NativeView view_gaining_focus);
-
-  // The search mode in the active tab has changed. Pass the message down to
-  // the overlay which will notify the renderer. Create |instant_tab_| if the
-  // |new_mode| reflects an Instant search results page.
-  void SearchModeChanged(const chrome::search::Mode& old_mode,
-                         const chrome::search::Mode& new_mode);
-
-  // The user switched tabs. Hide the preview. Create |instant_tab_| if the
-  // newly active tab is an Instant search results page.
-  void ActiveTabChanged();
-
-  // The user is about to switch tabs. Commit the preview if needed.
-  void TabDeactivated(content::WebContents* contents);
-
-  // Sets whether Instant should show result previews. |use_local_preview_only|
-  // will force the use of kLocalOmniboxPopupURL as the Instant URL and is only
-  // applicable if |extended_enabled_| is true.
-  void SetInstantEnabled(bool instant_enabled, bool use_local_preview_only);
-
-  // The theme has changed. Pass the message to the preview page.
-  void ThemeChanged(const ThemeBackgroundInfo& theme_info);
-
-  // Called when someone else swapped in a different contents in the |overlay_|.
-  void SwappedOverlayContents();
-
-  // Called when contents for |overlay_| received focus.
-  void FocusedOverlayContents();
-
-  // Called when the |overlay_| might be stale. If it's actually stale, and the
-  // omnibox doesn't have focus, and the preview isn't showing, the |overlay_|
-  // is deleted and recreated. Else the refresh is skipped.
-  void ReloadOverlayIfStale();
-
-  // Adds a new event to |debug_events_| and also DVLOG's it. Ensures that
-  // |debug_events_| doesn't get too large.
-  void LogDebugEvent(const std::string& info) const;
-
-  // See comments for |debug_events_| below.
-  const std::list<std::pair<int64, std::string> >& debug_events() {
-    return debug_events_;
-  }
-
-  // Returns the transition type of the last AutocompleteMatch passed to Update.
-  content::PageTransition last_transition_type() const {
-    return last_transition_type_;
-  }
-
-  // Non-const for Add/RemoveObserver only.  Other model changes should only
-  // happen through the InstantController interface.
-  InstantModel* model() { return &model_; }
+  // --------------------------------------------------------------------------
+  // The OmniboxEditModel delivers the following events as the user interacts
+  // with the omnibox.
 
- private:
-  FRIEND_TEST_ALL_PREFIXES(InstantTest, OmniboxFocusLoadsInstant);
-  FRIEND_TEST_ALL_PREFIXES(InstantTest, SetWithTemplateURL);
-  FRIEND_TEST_ALL_PREFIXES(InstantTest, NonInstantSearchProvider);
-  FRIEND_TEST_ALL_PREFIXES(InstantTest, InstantOverlayRefresh);
-  FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, ExtendedModeIsOn);
-  FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, MostVisited);
-  FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, OmniboxFocusLoadsInstant);
-  FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest,
-                           OmniboxTextUponFocusedCommittedSERP);
-  FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest,
-                           MiddleClickOnSuggestionOpensInNewTab);
-  FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, NTPIsPreloaded);
-  FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, PreloadedNTPIsUsedInNewTab);
-  FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, PreloadedNTPIsUsedInSameTab);
-  FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, ProcessIsolation);
-  FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, UnrelatedSiteInstance);
-  FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest,
-      OmniboxCommitsWhenShownFullHeight);
-
-  // Overridden from content::NotificationObserver:
-  virtual void Observe(int type,
-                       const content::NotificationSource& source,
-                       const content::NotificationDetails& details) OVERRIDE;
-
-  // Overridden from InstantPage::Delegate:
-  // TODO(shishir): We assume that the WebContent's current RenderViewHost is
-  // the RenderViewHost being created which is not always true. Fix this.
-  virtual void InstantPageRenderViewCreated(
-      const content::WebContents* contents) OVERRIDE;
-  virtual void InstantSupportDetermined(
-      const content::WebContents* contents,
-      bool supports_instant) OVERRIDE;
-  virtual void InstantPageRenderViewGone(
-      const content::WebContents* contents) OVERRIDE;
-  virtual void InstantPageAboutToNavigateMainFrame(
-      const content::WebContents* contents,
-      const GURL& url) OVERRIDE;
-  virtual void SetSuggestions(
-      const content::WebContents* contents,
-      const std::vector<InstantSuggestion>& suggestions) OVERRIDE;
-  virtual void ShowInstantOverlay(
-      const content::WebContents* contents,
-      InstantShownReason reason,
-      int height,
-      InstantSizeUnits units) OVERRIDE;
-  virtual void StartCapturingKeyStrokes(
-      const content::WebContents* contents) OVERRIDE;
-  virtual void StopCapturingKeyStrokes(content::WebContents* contents) OVERRIDE;
-  virtual void NavigateToURL(
-      const content::WebContents* contents,
-      const GURL& url,
-      content::PageTransition transition,
-      WindowOpenDisposition disposition) OVERRIDE;
-
-  // Invoked by the InstantLoader when the instant page wants to delete a
-  // Most Visited item.
-  virtual void DeleteMostVisitedItem(const GURL& url) OVERRIDE;
-
-  // Invoked by the InstantLoader when the instant page wants to undo a
-  // Most Visited deletion.
-  virtual void UndoMostVisitedDeletion(const GURL& url) OVERRIDE;
-
-  // Invoked by the InstantLoader when the instant page wants to undo all
-  // Most Visited deletions.
-  virtual void UndoAllMostVisitedDeletions() OVERRIDE;
-
-  // Helper for OmniboxFocusChanged. Commit or discard the preview.
-  void OmniboxLostFocus(gfx::NativeView view_gaining_focus);
-
-  // Creates a new NTP, using the instant_url property of the default
-  // TemplateURL.
-  void ResetNTP();
-
-  // Ensures that |overlay_| uses the Instant URL returned by GetInstantURL(),
-  // creating a new overlay if necessary. In extended mode, will fallback to
-  // using the kLocalOmniboxPopupURL as the Instant URL in case GetInstantURL()
-  // returns false. Returns true if an Instant URL could be determined.
-  // For |ignore_blacklist| look at comments in |GetInstantURL|.
-  bool EnsureOverlayIsCurrent(bool ignore_blacklist);
-
-  // Recreates the |overlay_| with |instant_url|. Note that |overlay_| is
-  // deleted in this call.
-  void CreateOverlay(const std::string& instant_url,
-                     const content::WebContents* active_tab);
-
-  // If the |overlay_| being used is in fallback mode, it will be switched back
-  // to the remote overlay if the overlay is not showing and the omnibox does
-  // not have focus.
-  void MaybeSwitchToRemoteOverlay();
-
-  // If the active tab is an Instant search results page, sets |instant_tab_| to
-  // point to it. Else, deletes any existing |instant_tab_|.
-  void ResetInstantTab();
-
-  // Hide the preview. Also sends an onchange event (with blank query) to the
-  // preview, telling it to clear out results for any old queries.
-  void HideOverlay();
-
-  // Like HideOverlay(), but doesn't call OnStaleOverlay(). Use HideOverlay()
-  // unless you are going to call overlay_.reset() yourself subsequently.
-  void HideInternal();
-
-  // Counterpart to HideOverlay(). Asks the |browser_| to display the preview
-  // with the given |height| in |units|.
-  void ShowOverlay(InstantShownReason reason,
-                   int height,
-                   InstantSizeUnits units);
-
-  // Send the omnibox popup bounds to the page.
-  void SendPopupBoundsToPage();
-
-  // Determines the Instant URL based on a number of factors:
-  // If |extended_enabled_|:
-  //   - If |use_local_preview_only_| is true return kLocalOmniboxPopupURL, else
-  //   - If the Instant URL is specified by command line, returns it, else
-  //   - If the default Instant URL is present returns it.
-  // If !|extended_enabled_|:
-  //   - If the Instant URL is specified by command line, returns it, else
-  //   - If the default Instant URL is present returns it.
-  //
-  // If |ignore_blacklist| is set to true, Instant URLs are not filtered through
-  // the blacklist.
-  //
-  // Returns true if a valid Instant URL could be found that is not blacklisted.
-  bool GetInstantURL(Profile* profile,
-                     bool ignore_blacklist,
-                     std::string* instant_url) const;
-
-  // Adds the URL for the page to the blacklist. Deletes the contents held and
-  // recreates a new page.
-  void BlacklistAndResetOverlay();
-  void BlacklistAndResetNTP();
-
-  // Removes |url| from the blacklist.
-  void RemoveFromBlacklist(const std::string& url);
-
-  InstantOverlay* overlay() const { return overlay_.get(); }
-  InstantTab* instant_tab() const { return instant_tab_.get(); }
-  InstantNTP* ntp() const { return ntp_.get(); }
-
-  // Begin listening to change notifications from TopSites and fire off an
-  // initial request for most visited items.
-  void StartListeningToMostVisitedChanges();
-
-  // Fire off an async request for most visited items to the TopNav code.
-  void RequestMostVisitedItems();
-
-  // Called when we get new most visited items from the TopNav code,
-  // registered as an async callback.  Parses them and sends them to the
-  // renderer via SendMostVisitedItems.
-  void OnMostVisitedItemsReceived(const history::MostVisitedURLList& data);
-
-  // Sends a collection of MostVisitedItems to the renderer process via
-  // the appropriate InstantPage subclass.
-  void SendMostVisitedItems(const std::vector<MostVisitedItem>& items);
-
-  chrome::BrowserInstantController* const browser_;
-
-  // Whether the extended API and regular API are enabled. If both are false,
-  // Instant is effectively disabled.
-  const bool extended_enabled_;
-  bool instant_enabled_;
-
-  // If true, the instant URL is set to kLocalOmniboxPopupURL.
-  bool use_local_preview_only_;
-
-  // The state of the preview page, i.e., the page owned by |overlay_|. Ignored
-  // if |instant_tab_| is in use.
-  InstantModel model_;
-
-  // The three instances of InstantPage maintained by InstantController as
-  // described above. All three may be non-NULL in extended mode.  If
-  // |instant_tab_| is not NULL, then |overlay_| is guaranteed to be hidden and
-  // messages will be sent to |instant_tab_| instead.
-  //
-  // In non-extended mode, only |overlay_| is ever non-NULL.
-  scoped_ptr<InstantOverlay> overlay_;
-  scoped_ptr<InstantNTP> ntp_;
-  scoped_ptr<InstantTab> instant_tab_;
-
-  // The most recent full_text passed to Update(). If empty, we'll not accept
-  // search suggestions from |overlay_| or |instant_tab_|.
-  string16 last_omnibox_text_;
-
-  // True if the last Update() had an inline autocompletion. Used only to make
-  // sure that we don't accidentally suggest gray text suggestion in that case.
-  bool last_omnibox_text_has_inline_autocompletion_;
-
-  // The most recent verbatim passed to Update(). Used only to ensure that we
-  // don't accidentally suggest an inline autocompletion.
-  bool last_verbatim_;
-
-  // The most recent suggestion received from the page, minus any prefix that
-  // the user has typed.
-  InstantSuggestion last_suggestion_;
-
-  // See comments on the getter above.
-  content::PageTransition last_transition_type_;
-
-  // True if the last match passed to Update() was a search (versus a URL).
-  // Used to ensure that the preview page is committable.
-  bool last_match_was_search_;
-
-  // Omnibox focus state.
-  OmniboxFocusState omnibox_focus_state_;
-
-  // The search model mode for the active tab.
-  chrome::search::Mode search_mode_;
-
-  // Current omnibox popup bounds.
-  gfx::Rect popup_bounds_;
-
-  // Last popup bounds passed to the page.
-  gfx::Rect last_popup_bounds_;
-
-  // The start-edge margin and width of the omnibox, used by the page to align
-  // its suggestions with the omnibox.
-  gfx::Rect omnibox_bounds_;
-
-  // Timer used to update the bounds of the omnibox popup.
-  base::OneShotTimer<InstantController> update_bounds_timer_;
-
-  // For each key K => value N, the map says that we found that the search
-  // engine identified by Instant URL K didn't support the Instant API, or
-  // caused RenderView crashes in each of the last N times that we loaded it.
-  // If an Instant URL isn't present in the map at all or has a value 0,
-  // it means that search engine supports the Instant API (or we assume it does,
-  // since we haven't determined it doesn't) and it did not cause a crash.
-  std::map<std::string, int> blacklisted_urls_;
+  // Called in most cases where the omnibox text has changed.
+  virtual bool Update(const AutocompleteMatch& match,
+                      const string16& user_text,
+                      const string16& full_text,
+                      size_t selection_start,
+                      size_t selection_end,
+                      bool verbatim,
+                      bool user_input_in_progress,
+                      bool omnibox_popup_is_open,
+                      bool escape_pressed) = 0;
+
+  // Called when the autocomplete system has new results for the current text.
+  virtual void HandleAutocompleteResults(
+      const std::vector<AutocompleteProvider*>& providers) = 0;
+
+  // Called when the user wants to traverse the suggestions list. |count| is
+  // positive to indicate moving down, negative for moving up.
+  virtual bool OnUpOrDownKeyPressed(int count) = 0;
+
+  // Called when the user hits Escape after traversing the suggestions list.
+  virtual void OnCancel(const AutocompleteMatch& match,
+                        const string16& full_text) = 0;
+
+  // --------------------------------------------------------------------------
+  // Methods dealing with the overlay, its model and the NTP.
+
+  virtual const InstantModel* model() const = 0;
+
+  virtual bool IsOverlayingSearchResults() const = 0;

[samarth, 2013/03/01 17:59:53]

Search-and-replace fail? :)

How about IsOverlayShowingSearchResults?

[sreeram, 2013/03/07 18:18:46]

Sorry, this ship has sailed. I don't think it's unreasonable to say something
like "is the controller overlaying search results?"

 
-  // Search terms extraction (for autocomplete history matches) doesn't work
-  // on Instant URLs. So, whenever the user commits an Instant search, we add
-  // an equivalent non-Instant search URL to history, so that the search shows
-  // up in autocomplete history matches.
-  // TODO(sreeram): Remove when http://crbug.com/155373 is fixed.
-  GURL url_for_history_;
+  virtual content::WebContents* GetOverlayContents() const = 0;
 
-  // The timestamp at which query editing began. This value is used when the
-  // preview is showed and cleared when the preview is hidden.
-  base::Time first_interaction_time_;
+  virtual bool CommitIfPossible(InstantCommitType type) = 0;
 
-  // Whether to allow the preview to show search suggestions. In general, the
-  // preview is allowed to show search suggestions whenever |search_mode_| is
-  // MODE_SEARCH_SUGGESTIONS, except in those cases where this is false.
-  bool allow_preview_to_show_search_suggestions_;
+  virtual scoped_ptr<content::WebContents> ReleaseNTPContents() = 0;
 
-  // List of events and their timestamps, useful in debugging Instant behaviour.
-  mutable std::list<std::pair<int64, std::string> > debug_events_;
+  virtual void AddModelObserver(InstantModelObserver* observer) = 0;
 
-  // Used for Top Sites async retrieval.
-  base::WeakPtrFactory<InstantController> weak_ptr_factory_;
+  virtual void RemoveModelObserver(InstantModelObserver* observer) = 0;
 
-  // Used to get notifications about Most Visted changes.
-  content::NotificationRegistrar registrar_;
+  // --------------------------------------------------------------------------
+  // Information about the omnibox and bookmark bar.
 
+  virtual void SetPopupBounds(const gfx::Rect& bounds) = 0;
+
+  virtual void SetOmniboxBounds(const gfx::Rect& bounds) = 0;
+
+  virtual void BookmarkBarStateChanged(bool is_detached) = 0;
+
+  // --------------------------------------------------------------------------
+  // Events sent as the user stops (or starts) interacting with the omnibox.
+
+  virtual void OmniboxFocusChanged(OmniboxFocusState state,
+                                   OmniboxFocusChangeReason reason,
+                                   gfx::NativeView view_gaining_focus) = 0;
+
+  virtual void TabDeactivated(content::WebContents* contents) = 0;
+
+  virtual void SearchModeChanged(const chrome::search::Mode& old_mode,
+                                 const chrome::search::Mode& new_mode) = 0;
+
+  virtual void ActiveTabChanged() = 0;
+
+  // --------------------------------------------------------------------------
+  // Calls from InstantOverlay. Not for general use.
+
+  virtual void SwappedOverlayContents() = 0;
+
+  virtual void FocusedOverlayContents() = 0;
+
+ private:
   DISALLOW_COPY_AND_ASSIGN(InstantController);
 };