InstantExtended: Committed NTP

chrome/browser/instant/instant_controller.h

 // 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.
 
 #ifndef CHROME_BROWSER_INSTANT_INSTANT_CONTROLLER_H_
 #define CHROME_BROWSER_INSTANT_INSTANT_CONTROLLER_H_
 
 #include <map>
 #include <string>
 #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/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/common/page_transition_types.h"
 #include "googleurl/src/gurl.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gfx/rect.h"
 
 struct AutocompleteMatch;
 class AutocompleteProvider;
-class InstantLoader;
+class InstantNTP;
+class InstantOverlay;
 class InstantTab;
 class TemplateURL;
 
 namespace chrome {
 class BrowserInstantController;
 }
 
 namespace content {
 class WebContents;
 }
 
-// InstantController maintains a WebContents that is intended to give a preview
-// of search suggestions and results. InstantController is owned by Browser via
-// BrowserInstantController.
-class InstantController {
+// InstantController drives Chrome Instant, i.e., the browser implementation of
+// the Embedded Search API (see http://dev.chromium.org/embeddedsearch). It
+// maintains a WebContents used to show search suggestions and results in an
+// overlay and a WebContents used for the New Tab Page. InstantController is
+// owned by Browser via BrowserInstantController.
+class InstantController : public InstantPage::Delegate {

[dhollowa, 2013/01/22 22:34:58]

I'm a bit confused here...  The controller is a page-delegate but it sets itself
as the page-delegate for all three of instant-overlay, instant-ntp, and
instant-tab.  How does it know (or does it need to know) which of the three it
is responding too.  Is only on of the three "in effect" at a time?

[samarth, 2013/01/25 21:08:40]

It's responding (potentially) to all three.  That's the reason we send up the
contents in each response, so InstantController can tell who the message came
from.

  public:
   // The URL for the local omnibox popup.
   static const char* kLocalOmniboxPopupURL;
 
   // |use_local_preview_only| will force the use of kLocalOmniboxPopupURL as the
   // instant URL and is only applicable if |extended_enabled| is true.
   InstantController(chrome::BrowserInstantController* browser,
                     bool extended_enabled,
                     bool use_local_preview_only);
   ~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.
+  content::WebContents* ReleaseNTPContents();

[dhollowa, 2013/01/22 22:34:58]

This should use scoped_ptr<contents::WebContents> as the return type.  This
gives compiler support to ownership transfer.  See
https://mail.google.com/mail/u/0/?ui=2&shva=1#search/label%3A*cr-dev+scoped_p...
for details.

[samarth, 2013/01/25 21:08:40]

Ah, didn't know about this.  This is really nice, much better than all the
comment sprinkled about!

Changed to use scoped_ptrs wherever it made sense in this CL and added some
comments for future cleanup in tangentially related code.

+
   // Sets the bounds of the omnibox popup, in screen coordinates.
   void SetPopupBounds(const gfx::Rect& bounds);
 
   // Sets the start and end margins of the omnibox text area.
   void SetMarginSize(int start, int end);
 
   // Send autocomplete results from |providers| to the preview page.
   void HandleAutocompleteResults(
       const std::vector<AutocompleteProvider*>& providers);
 
@@ skipping 13 matching lines @@
   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 loader which will notify the renderer. Create |instant_tab_| if the
+  // 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.
   void SetInstantEnabled(bool instant_enabled);
 
   // The theme has changed. Pass the message to the preview page.
   void ThemeChanged(const ThemeBackgroundInfo& theme_info);
 
   // The theme area height has changed. Pass the message to the preview page.
   void ThemeAreaHeightChanged(int height);
 
   // Returns the transition type of the last AutocompleteMatch passed to Update.
   content::PageTransition last_transition_type() const {
     return last_transition_type_;
   }
 
   const InstantModel* model() const { return &model_; }
 
-  // Invoked by the page when it has suggested text.
-  void SetSuggestions(const content::WebContents* contents,
-                      const std::vector<InstantSuggestion>& suggestions);
-
-  // Invoked by the page when its support for the Instant API is determined.
-  void InstantSupportDetermined(const content::WebContents* contents,
-                                bool supports_instant);
-
-  // Invoked by InstantLoader to request that the preview be shown.
-  void ShowInstantPreview(InstantShownReason reason,
-                          int height,
-                          InstantSizeUnits units);
-
-  // Invoked by InstantLoader to request the browser to start capturing user key
-  // strokes.
-  void StartCapturingKeyStrokes();
-
-  // Invoked by InstantLoader to request the browser to stop capturing user key
-  // strokes.
-  void StopCapturingKeyStrokes();
-
-  // Invoked by InstantLoader when it has swapped a different WebContents into
-  // the preview, usually because a prerendered page was navigated to.
-  void SwappedWebContents();
-
-  // Invoked by InstantLoader when the preview gains focus, usually due to the
-  // user clicking on it.
-  void InstantLoaderContentsFocused();
-
-  // Invoked by the InstantLoader when its RenderView crashes.
-  void InstantLoaderRenderViewGone();
-
-  // Invoked by InstantLoader when the instant page is about to navigate.
-  void InstantLoaderAboutToNavigateMainFrame(const GURL& url);
-
-  // Invoked by the InstantLoader when the instant page wants to navigate to
-  // the speicfied URL.
-  void NavigateToURL(const GURL& url, content::PageTransition transition);
-
  private:
   FRIEND_TEST_ALL_PREFIXES(InstantTest, OmniboxFocusLoadsInstant);
   FRIEND_TEST_ALL_PREFIXES(InstantTest, SetWithTemplateURL);
   FRIEND_TEST_ALL_PREFIXES(InstantTest, NonInstantSearchProvider);
   FRIEND_TEST_ALL_PREFIXES(InstantTest, InstantLoaderRefresh);
   FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, ExtendedModeIsOn);
   FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, OmniboxFocusLoadsInstant);
 
+  // Overridden from InstantPage::Delegate

[dhollowa, 2013/01/22 22:34:58]

nit: Should end with ":".

[samarth, 2013/01/25 21:08:40]

Done.

+  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 ShowInstantPreview(
+      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) OVERRIDE;
+  virtual void OnStalePage(const content::WebContents* contents) OVERRIDE;

[dhollowa, 2013/01/22 22:34:58]

nit: Shouldn't begin with "On".

[samarth, 2013/01/25 21:08:40]

Done.

+  virtual void InstantPageSwappedContents(
+      content::WebContents* new_contents) OVERRIDE;
+  virtual void InstantPageFocused(
+      const content::WebContents* contents) OVERRIDE;
+  virtual void InstantPageClicked(
+      const content::WebContents* contents) OVERRIDE;
+  virtual bool InstantPageAboutToOpenURL(
+      const content::WebContents* contents) OVERRIDE;
+
   // Helper for OmniboxFocusChanged. Commit or discard the preview.
   void OmniboxLostFocus(gfx::NativeView view_gaining_focus);
 
-  // Creates a new loader if necessary, using the instant_url property of the
+  // 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 OnStaleOverlay();

[dhollowa, 2013/01/22 22:34:58]

nit: Shouldn't begin with "On".

[samarth, 2013/01/25 21:08:40]

Done.

+
+  // Creates a new NTP, using the instant_url property of the |template_url|.
+  bool ResetNTP();
+
+  // Creates a new overlay if necessary, using the instant_url property of the
   // |template_url| (for example, if the Instant URL has changed since the last
-  // time the loader was created). If |fallback_to_local| is true will use
+  // time the overlay was created). If |fallback_to_local| is true will use
   // kLocalOmniboxPopupURL as the fallback url (in extended mode) in case
   // the |template_url| doesn't have a valid Instant URL. Returns true if an
   // instant URL could be determined.
-  bool ResetLoader(const TemplateURL* template_url,
-                   const content::WebContents* active_tab,
-                   bool fallback_to_local);
+  bool ResetOverlay(const TemplateURL* template_url,
+                    const content::WebContents* active_tab,
+                    bool fallback_to_local);
 
-  // Ensures that the |loader_| uses the default Instant URL, recreating it if
+  // Ensures that the |overlay_| uses the default Instant URL, recreating it if
   // necessary, and returns true. Returns false if the Instant URL could not be
   // determined or the active tab is NULL (browser is shutting down).
-  bool CreateDefaultLoader();
+  bool CreateDefaultOverlay();
 
-  // Called when the |loader_| might be stale. If it's actually stale, and the
-  // omnibox doesn't have focus, and the preview isn't showing, the |loader_| is
-  // deleted and recreated. Else the refresh is skipped.
-  void OnStaleLoader();
+  // Called by Update() to ensure we have an Instant page that can process
+  // |match|. Returns true if we should continue with the Update().
+  bool ResetOverlayForMatch(const AutocompleteMatch& match);
 
   // 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();
 
-  // Called by Update() to ensure we have an Instant page that can process
-  // |match|. Returns true if we should continue with the Update().
-  bool ResetLoaderForMatch(const AutocompleteMatch& match);
-
   // 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 HideLoader();
+  void HideOverlay();
 
-  // Like HideLoader(), but doesn't call OnStaleLoader(). Use HideLoader()
-  // unless you are going to call loader_.reset() yourself subsequently.
+  // Like HideOverlay(), but doesn't call OnStaleOverlay(). Use HideOverlay()
+  // unless you are going to call overlay_.reset() yourself subsequently.
   void HideInternal();
 
-  // Counterpart to HideLoader(). Asks the |browser_| to display the preview
+  // Counterpart to HideOverlay(). Asks the |browser_| to display the preview
   // with the given |height|.

[dhollowa, 2013/01/22 22:34:58]

nit: "...with the given |height| and |units|."

[samarth, 2013/01/25 21:08:40]

Done.

-  void ShowLoader(InstantShownReason reason,
-                  int height,
-                  InstantSizeUnits units);
+  void ShowOverlay(InstantShownReason reason,
+                   int height,
+                   InstantSizeUnits units);
 
   // Send the omnibox popup bounds to the page.
   void SendPopupBoundsToPage();
 
   // If |template_url| is a valid TemplateURL for use with Instant, fills in
   // |instant_url| and returns true; returns false otherwise.
   // Note: If the command-line switch kInstantURL is set, this method uses its
   // value for |instant_url| and returns true without examining |template_url|.
   bool GetInstantURL(const TemplateURL* template_url,
                      std::string* instant_url) const;
 
+  // Add |url| to the blacklist.
+  void AddToBlacklist(const std::string& url);
+
+  // Remove |url| from the blacklist.
+  void RemoveFromBlacklist(const std::string& url);
+
   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.
   const bool use_local_preview_only_;
 
-  // The state of the preview page, i.e., the page owned by |loader_|. Ignored
+  // The state of the preview page, i.e., the page owned by |overlay_|. Ignored
   // if |instant_tab_| is in use.
   InstantModel model_;
 
-  // The preview WebContents.
-  scoped_ptr<InstantLoader> loader_;
+  // WebContents used to show search suggestions and results in an overlay.
+  scoped_ptr<InstantOverlay> overlay_;

[dhollowa, 2013/01/22 22:34:58]

It would be helpful to articulate the invariants between the |overlay_| the
|ntp_| and the |instant_tab_|.  I.e. What is expected to be NULL and when, any
mutual exclusivity?  And with these invariants are there opportunities to add
DCHECKs in the code?

[samarth, 2013/01/25 21:08:40]

Done.

 
-  // A committed WebContents that supports Instant. If non-NULL, the |loader_|
-  // is guaranteed to be hidden and messages will be sent to this instead.
+  // WebContents used for New Tab Pages.
+  scoped_ptr<InstantNTP> ntp_;
+
+  // A committed WebContents that supports the SearchBoxAPI. If non-NULL, the
+  // |overlay_| is guaranteed to be hidden and messages will be sent to this
+  // instead.
   scoped_ptr<InstantTab> instant_tab_;
 
   // The most recent full_text passed to Update(). If empty, we'll not accept
-  // search suggestions from |loader_| or |instant_tab_|.
+  // 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_;
 
@@ skipping 22 matching lines @@
 
   // Size of the start-edge omnibox text area margin.
   int start_margin_;
 
   // Size of the end-edge omnibox text area margin.
   int end_margin_;
 
   // Timer used to update the bounds of the omnibox popup.
   base::OneShotTimer<InstantController> update_bounds_timer_;
 
-  // Timer used to ensure that the Instant page does not get too stale.
-  base::OneShotTimer<InstantController> stale_loader_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_;
 
   // 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_;
 
   // 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_;
 
   // 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_;
 
   DISALLOW_COPY_AND_ASSIGN(InstantController);
 };
 
 #endif  // CHROME_BROWSER_INSTANT_INSTANT_CONTROLLER_H_