Persist the Instant API to committed search result pages.

chrome/browser/instant/instant_loader.cc

 // 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.
 
 #include "chrome/browser/instant/instant_loader.h"
 
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/instant/instant_controller.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/thumbnails/thumbnail_tab_helper.h"
@@ skipping 29 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(InstantLoaderUserData);
 };
 
 }
 
 // WebContentsDelegateImpl -----------------------------------------------------
 
 class InstantLoader::WebContentsDelegateImpl
     : public ConstrainedWindowTabHelperDelegate,
       public CoreTabHelperDelegate,
-      public content::WebContentsDelegate,
-      public content::WebContentsObserver {
+      public content::WebContentsDelegate {
  public:
   explicit WebContentsDelegateImpl(InstantLoader* loader);
 
   bool is_pointer_down_from_activate() const {
     return is_pointer_down_from_activate_;
   }
 
-  // Start observing |web_contents| instead of whatever is currently being
-  // observed. If |web_contents| is NULL, effectively stops observing.
-  void ObserveContents(content::WebContents* web_contents);
-
  private:
   // Overridden from ConstrainedWindowTabHelperDelegate:
   virtual bool ShouldFocusConstrainedWindow() OVERRIDE;
 
   // Overridden from CoreTabHelperDelegate:
   virtual void SwapTabContents(content::WebContents* old_contents,
                                content::WebContents* new_contents) OVERRIDE;
 
   // Overridden from content::WebContentsDelegate:
   virtual bool ShouldSuppressDialogs() OVERRIDE;
   virtual bool ShouldFocusPageAfterCrash() OVERRIDE;
   virtual void LostCapture() OVERRIDE;
   virtual void WebContentsFocused(content::WebContents* contents) OVERRIDE;
   virtual bool CanDownload(content::RenderViewHost* render_view_host,
                            int request_id,
                            const std::string& request_method) OVERRIDE;
   virtual void HandleMouseDown() OVERRIDE;
   virtual void HandleMouseUp() OVERRIDE;
   virtual void HandlePointerActivate() OVERRIDE;
   virtual void HandleGestureEnd() OVERRIDE;
   virtual void DragEnded() OVERRIDE;
   virtual bool OnGoToEntryOffset(int offset) OVERRIDE;
 
-  // Overridden from content::WebContentsObserver:
-  virtual void DidFinishLoad(
-      int64 frame_id,
-      const GURL& validated_url,
-      bool is_main_frame,
-      content::RenderViewHost* render_view_host) OVERRIDE;
-  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
-
-  // Message from renderer indicating the page has suggestions.
-  void OnSetSuggestions(int page_id,
-                        const std::vector<InstantSuggestion>& suggestions);
-
-  // Message from the renderer determining whether it supports the Instant API.
-  void OnInstantSupportDetermined(int page_id, bool result);
-
-  // Message from the renderer requesting the preview be shown.
-  void OnShowInstantPreview(int page_id,
-                            InstantShownReason reason,
-                            int height,
-                            InstantSizeUnits units);
-
   void CommitFromPointerReleaseIfNecessary();
-  void MaybeSetAndNotifyInstantSupportDetermined(bool supports_instant);
 
   InstantLoader* const loader_;
 
   // True if the mouse or a touch pointer is down from an activate.
   bool is_pointer_down_from_activate_;
 
   DISALLOW_COPY_AND_ASSIGN(WebContentsDelegateImpl);
 };
 
 InstantLoader::WebContentsDelegateImpl::WebContentsDelegateImpl(
     InstantLoader* loader)
     : loader_(loader),
       is_pointer_down_from_activate_(false) {
 }
 
-void InstantLoader::WebContentsDelegateImpl::ObserveContents(
-    content::WebContents* web_contents) {
-  Observe(web_contents);
-}
-
 bool InstantLoader::WebContentsDelegateImpl::ShouldFocusConstrainedWindow() {
   // Return false so that constrained windows are not initially focused. If we
   // did otherwise the preview would prematurely get committed when focus goes
   // to the constrained window.
   return false;
 }
 
 void InstantLoader::WebContentsDelegateImpl::SwapTabContents(
     content::WebContents* old_contents,
     content::WebContents* new_contents) {
@@ skipping 48 matching lines @@
   // If the user drags, we won't get a mouse up (at least on Linux). Commit the
   // Instant result when the drag ends, so that during the drag the page won't
   // move around.
   CommitFromPointerReleaseIfNecessary();
 }
 
 bool InstantLoader::WebContentsDelegateImpl::OnGoToEntryOffset(int offset) {
   return false;
 }
 
-void InstantLoader::WebContentsDelegateImpl::DidFinishLoad(
-    int64 frame_id,
-    const GURL& validated_url,
-    bool is_main_frame,
-    content::RenderViewHost* render_view_host) {
-  if (is_main_frame && !loader_->supports_instant_)
-    Send(new ChromeViewMsg_DetermineIfPageSupportsInstant(routing_id()));
-}
-
-bool InstantLoader::WebContentsDelegateImpl::OnMessageReceived(
-    const IPC::Message& message) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(WebContentsDelegateImpl, message)
-    IPC_MESSAGE_HANDLER(ChromeViewHostMsg_SetSuggestions, OnSetSuggestions)
-    IPC_MESSAGE_HANDLER(ChromeViewHostMsg_InstantSupportDetermined,
-                        OnInstantSupportDetermined)
-    IPC_MESSAGE_HANDLER(ChromeViewHostMsg_ShowInstantPreview,
-                        OnShowInstantPreview);
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
-}
-
-void InstantLoader::WebContentsDelegateImpl::OnSetSuggestions(
-    int page_id,
-    const std::vector<InstantSuggestion>& suggestions) {
-  DCHECK(loader_->preview_contents());
-  DCHECK(loader_->preview_contents_->web_contents());
-  content::NavigationEntry* entry = loader_->preview_contents_->web_contents()->
-                                        GetController().GetActiveEntry();
-  if (entry && page_id == entry->GetPageID()) {
-    MaybeSetAndNotifyInstantSupportDetermined(true);
-    loader_->controller_->SetSuggestions(loader_, suggestions);
-  }
-}
-
-void InstantLoader::WebContentsDelegateImpl::OnInstantSupportDetermined(
-    int page_id,
-    bool result) {
-  DCHECK(loader_->preview_contents());
-  DCHECK(loader_->preview_contents_->web_contents());
-  content::NavigationEntry* entry = loader_->preview_contents_->web_contents()->
-                                        GetController().GetActiveEntry();
-  if (entry && page_id == entry->GetPageID())
-    MaybeSetAndNotifyInstantSupportDetermined(result);
-}
-
-void InstantLoader::WebContentsDelegateImpl::OnShowInstantPreview(
-    int page_id,
-    InstantShownReason reason,
-    int height,
-    InstantSizeUnits units) {
-  DCHECK(loader_->preview_contents());
-  DCHECK(loader_->preview_contents_->web_contents());
-  content::NavigationEntry* entry = loader_->preview_contents_->web_contents()->
-                                        GetController().GetActiveEntry();
-  if (entry && page_id == entry->GetPageID()) {
-    MaybeSetAndNotifyInstantSupportDetermined(true);
-    loader_->controller_->ShowInstantPreview(loader_, reason, height, units);
-  }
-}
-
 void InstantLoader::WebContentsDelegateImpl
     ::CommitFromPointerReleaseIfNecessary() {
   if (is_pointer_down_from_activate_) {
     is_pointer_down_from_activate_ = false;
     loader_->controller_->CommitInstantLoader(loader_);
   }
 }
 
-void InstantLoader::WebContentsDelegateImpl
-    ::MaybeSetAndNotifyInstantSupportDetermined(bool supports_instant) {
-  // If we already determined that the loader supports Instant, nothing to do.
-  if (!loader_->supports_instant_) {
-    loader_->supports_instant_ = supports_instant;
-    loader_->controller_->InstantSupportDetermined(loader_, supports_instant);
-  }
-}
-
 // InstantLoader ---------------------------------------------------------------
 
 // static
 InstantLoader* InstantLoader::FromWebContents(
     content::WebContents* web_contents) {
   InstantLoaderUserData* data = static_cast<InstantLoaderUserData*>(
       web_contents->GetUserData(&kUserDataKey));
   return data ? data->loader() : NULL;
 }
 
 InstantLoader::InstantLoader(InstantController* controller,
                              const std::string& instant_url,
                              const TabContents* tab_contents)
-    : controller_(controller),
+    : InstantClient(controller, NULL),

[sky, 2012/11/27 01:07:51]

I get why you subclassed here, but it feels a bit iffy. It seems like
InstantLoader and what is InstantClient want to have an InstantClient rather
than be InstantClient.

[samarth, 2012/11/27 18:09:43]

This might be what sky is already saying, but what about having InstantClient
(or some other name) be in charge of multiplexing between the active tab and the
preview contents? That has two benefits:
1) You don't need to split up the loader functionality into two places (which
will make things harder to find and understand later)
2) We can reduce the burden on InstantController which is already handling tons
of state and tricky logic.

[sreeram, 2012/11/27 18:41:33]

I understand the style guide's preference for composition over inheritance. If
we do that here, I think we'll have something like this (say the new stuff is
called InstantTab; and the object that both InstantTab and InstantLoader hold is
called InstantClient):

void InstantController::SetSuggestions(InstantClient* client, ...) {
  if (CurrentClient()->client() != client || ...)
    return;

I.e., when calling back from the loader to InstantController, we'll have to pass
in something that's common to both InstantTab and InstantLoader, and then do
this roundabout check to see which one actually called the InstantController.
("roundabout" because there's really no reason for InstantTab/InstantLoader to
expose their client() to InstantController otherwise.)

The above is a somewhat small point, but overall I think inheritance makes the
code leaner in this case. I think composition will just give us a whole bunch of
shim methods for no good reason.

To samarth's point: I don't think we can do the multiplexing outside the
InstantController easily. The logic is quite complex, and depends on
InstantController state (such as search_mode_ and is_focused_ and such), so we'd
have to expose all of that as well.

[sreeram, 2012/11/29 07:33:19]

Done. Composition wins.

       preview_delegate_(new WebContentsDelegateImpl(
           ALLOW_THIS_IN_INITIALIZER_LIST(this))),
       preview_contents_(
           TabContents::Factory::CreateTabContents(
               content::WebContents::CreateWithSessionStorage(
                   tab_contents->profile(), NULL, MSG_ROUTING_NONE,
                   tab_contents->web_contents(),
                   tab_contents->web_contents()->GetController().
                                           GetSessionStorageNamespaceMap()))),
-      supports_instant_(false),
       instant_url_(instant_url) {
 }
 
 InstantLoader::~InstantLoader() {
 }
 
 void InstantLoader::Init() {
   SetupPreviewContents();
   // This HTTP header and value are set on loads that originate from instant.
   const char kInstantHeader[] = "X-Purpose: Instant";
   DVLOG(1) << "LoadURL: " << instant_url_;
-  preview_contents_->web_contents()->GetController().LoadURL(GURL(instant_url_),
-      content::Referrer(), content::PAGE_TRANSITION_GENERATED, kInstantHeader);
-  preview_contents_->web_contents()->WasHidden();
+  contents()->GetController().LoadURL(GURL(instant_url_), content::Referrer(),
+      content::PAGE_TRANSITION_GENERATED, kInstantHeader);
+  contents()->WasHidden();
 }
 
-void InstantLoader::Update(const string16& user_text, bool verbatim) {
-  // TODO: Support real cursor position.
-  last_navigation_ = history::HistoryAddPageArgs();
-  content::RenderViewHost* rvh =
-      preview_contents_->web_contents()->GetRenderViewHost();
-  rvh->Send(new ChromeViewMsg_SearchBoxChange(rvh->GetRoutingID(), user_text,
-                    verbatim, user_text.size(), user_text.size()));
+void InstantLoader::Cancel(const string16& text) {
+  Send(new ChromeViewMsg_SearchBoxCancel(routing_id(), text));
 }
 
 void InstantLoader::SetOmniboxBounds(const gfx::Rect& bounds) {
-  content::RenderViewHost* rvh =
-      preview_contents_->web_contents()->GetRenderViewHost();
-  rvh->Send(new ChromeViewMsg_SearchBoxResize(rvh->GetRoutingID(), bounds));
-}
-
-void InstantLoader::SendAutocompleteResults(
-    const std::vector<InstantAutocompleteResult>& results) {
-  content::RenderViewHost* rvh =
-      preview_contents_->web_contents()->GetRenderViewHost();
-  rvh->Send(new ChromeViewMsg_SearchBoxAutocompleteResults(rvh->GetRoutingID(),
-                                                           results));
+  Send(new ChromeViewMsg_SearchBoxResize(routing_id(), bounds));
 }
 
 void InstantLoader::SendThemeBackgroundInfo(
       const ThemeBackgroundInfo& theme_info) {
-  content::RenderViewHost* rvh =
-      preview_contents_->web_contents()->GetRenderViewHost();
-  rvh->Send(new ChromeViewMsg_SearchBoxThemeChanged(rvh->GetRoutingID(),
-                                                    theme_info));
+  Send(new ChromeViewMsg_SearchBoxThemeChanged(routing_id(), theme_info));
 }
 
 void InstantLoader::SendThemeAreaHeight(int height) {
-  content::RenderViewHost* rvh =
-      preview_contents_->web_contents()->GetRenderViewHost();
-  rvh->Send(new ChromeViewMsg_SearchBoxThemeAreaHeightChanged(
-      rvh->GetRoutingID(), height));
-}
-
-void InstantLoader::OnUpOrDownKeyPressed(int count) {
-  content::RenderViewHost* rvh =
-      preview_contents_->web_contents()->GetRenderViewHost();
-  rvh->Send(new ChromeViewMsg_SearchBoxUpOrDownKeyPressed(rvh->GetRoutingID(),
-                                                          count));
+  Send(new ChromeViewMsg_SearchBoxThemeAreaHeightChanged(routing_id(), height));
 }
 
 void InstantLoader::SearchModeChanged(const chrome::search::Mode& mode) {
-  content::RenderViewHost* rvh =
-      preview_contents_->web_contents()->GetRenderViewHost();
-  rvh->Send(new ChromeViewMsg_SearchBoxModeChanged(rvh->GetRoutingID(), mode));
+  Send(new ChromeViewMsg_SearchBoxModeChanged(routing_id(), mode));
 }
 
 void InstantLoader::DidNavigate(
     const history::HistoryAddPageArgs& add_page_args) {
   last_navigation_ = add_page_args;
 }
 
-TabContents* InstantLoader::ReleasePreviewContents(InstantCommitType type,
-                                                   const string16& text) {
-  content::RenderViewHost* rvh =
-      preview_contents_->web_contents()->GetRenderViewHost();
-  if (type == INSTANT_COMMIT_FOCUS_LOST)
-    rvh->Send(new ChromeViewMsg_SearchBoxCancel(rvh->GetRoutingID(), text));
-  else
-    rvh->Send(new ChromeViewMsg_SearchBoxSubmit(rvh->GetRoutingID(), text));
-  CleanupPreviewContents();
-  return preview_contents_.release();
-}
+void InstantLoader::CleanupPreviewContents() {
+  contents()->RemoveUserData(&kUserDataKey);
+  contents()->SetDelegate(NULL);
 
-void InstantLoader::CleanupPreviewContents() {
-  content::WebContents* old_contents = preview_contents_->web_contents();
-  old_contents->RemoveUserData(&kUserDataKey);
-  old_contents->SetDelegate(NULL);
-  preview_delegate_->ObserveContents(NULL);
-
-  BlockedContentTabHelper::FromWebContents(old_contents)->
+  BlockedContentTabHelper::FromWebContents(contents())->
       SetAllContentsBlocked(false);
-  ConstrainedWindowTabHelper::FromWebContents(old_contents)->set_delegate(NULL);
-  TabSpecificContentSettings::FromWebContents(old_contents)->
+  ConstrainedWindowTabHelper::FromWebContents(contents())->set_delegate(NULL);
+  TabSpecificContentSettings::FromWebContents(contents())->
       SetPopupsBlocked(false);
-  CoreTabHelper::FromWebContents(old_contents)->set_delegate(NULL);
+  CoreTabHelper::FromWebContents(contents())->set_delegate(NULL);
   if (ThumbnailTabHelper* thumbnail_tab_helper =
-          ThumbnailTabHelper::FromWebContents(old_contents))
+          ThumbnailTabHelper::FromWebContents(contents()))
     thumbnail_tab_helper->set_enabled(true);
 
 #if defined(OS_MACOSX)
   if (content::RenderWidgetHostView* rwhv =
-          old_contents->GetRenderWidgetHostView())
+          contents()->GetRenderWidgetHostView())
     rwhv->SetTakesFocusOnlyOnMouseDown(false);
   registrar_.Remove(this, content::NOTIFICATION_RENDER_VIEW_HOST_CHANGED,
                     content::Source<content::NavigationController>(
-                        &old_contents->GetController()));
+                        &contents()->GetController()));
 #endif
+
+  content::WebContentsObserver::Observe(NULL);
 }
 
 bool InstantLoader::IsPointerDownFromActivate() const {
   return preview_delegate_->is_pointer_down_from_activate();
 }
 
+bool InstantLoader::OnMessageReceived(const IPC::Message& message) {
+  if (InstantClient::OnMessageReceived(message))
+    return true;
+
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(InstantLoader, message)
+    IPC_MESSAGE_HANDLER(ChromeViewHostMsg_ShowInstantPreview,
+                        OnShowInstantPreview);
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
 void InstantLoader::Observe(int type,
                             const content::NotificationSource& source,
                             const content::NotificationDetails& details) {
 #if defined(OS_MACOSX)
   if (type == content::NOTIFICATION_RENDER_VIEW_HOST_CHANGED) {
     if (content::RenderWidgetHostView* rwhv =
-            preview_contents_->web_contents()->GetRenderWidgetHostView())
+            contents()->GetRenderWidgetHostView())
       rwhv->SetTakesFocusOnlyOnMouseDown(true);
     return;
   }
   NOTREACHED();
 #endif
 }
 
 void InstantLoader::SetupPreviewContents() {
-  content::WebContents* new_contents = preview_contents_->web_contents();
-  new_contents->SetUserData(&kUserDataKey, new InstantLoaderUserData(this));
-  WebContentsDelegateImpl* new_delegate = preview_delegate_.get();
-  new_contents->SetDelegate(new_delegate);
-  new_delegate->ObserveContents(new_contents);
+  content::WebContentsObserver::Observe(preview_contents_->web_contents());
+
+  contents()->SetUserData(&kUserDataKey, new InstantLoaderUserData(this));
+  contents()->SetDelegate(preview_delegate_.get());
 
   // Disable popups and such (mainly to avoid losing focus and reverting the
   // preview prematurely).
-  BlockedContentTabHelper::FromWebContents(new_contents)->
+  BlockedContentTabHelper::FromWebContents(contents())->
       SetAllContentsBlocked(true);
-  ConstrainedWindowTabHelper::FromWebContents(new_contents)->
-      set_delegate(new_delegate);
-  TabSpecificContentSettings::FromWebContents(new_contents)->
+  ConstrainedWindowTabHelper::FromWebContents(contents())->
+      set_delegate(preview_delegate_.get());
+  TabSpecificContentSettings::FromWebContents(contents())->
       SetPopupsBlocked(true);
-  CoreTabHelper::FromWebContents(new_contents)->set_delegate(new_delegate);
+  CoreTabHelper::FromWebContents(contents())->
+      set_delegate(preview_delegate_.get());
   if (ThumbnailTabHelper* thumbnail_tab_helper =
-          ThumbnailTabHelper::FromWebContents(new_contents))
+          ThumbnailTabHelper::FromWebContents(contents()))
     thumbnail_tab_helper->set_enabled(false);
 
 #if defined(OS_MACOSX)
   // If |preview_contents_| does not currently have a RWHV, we will call
   // SetTakesFocusOnlyOnMouseDown() as a result of the RENDER_VIEW_HOST_CHANGED
   // notification.
   if (content::RenderWidgetHostView* rwhv =
-          new_contents->GetRenderWidgetHostView())
+          contents()->GetRenderWidgetHostView())
     rwhv->SetTakesFocusOnlyOnMouseDown(true);
   registrar_.Add(this, content::NOTIFICATION_RENDER_VIEW_HOST_CHANGED,
                  content::Source<content::NavigationController>(
-                     &new_contents->GetController()));
+                     &contents()->GetController()));
 #endif
 }
 
 void InstantLoader::ReplacePreviewContents(content::WebContents* old_contents,
                                            content::WebContents* new_contents) {
-  DCHECK_EQ(old_contents, preview_contents_->web_contents());
+  DCHECK_EQ(old_contents, contents());
   CleanupPreviewContents();
   // We release here without deleting so that the caller still has the
   // responsibility for deleting the TabContents.
   ignore_result(preview_contents_.release());
   preview_contents_.reset(TabContents::FromWebContents(new_contents));
   SetupPreviewContents();
   controller_->SwappedTabContents(this);
 }
+
+void InstantLoader::OnShowInstantPreview(int page_id,
+                                         InstantShownReason reason,
+                                         int height,
+                                         InstantSizeUnits units) {
+  content::NavigationEntry* entry =
+      contents()->GetController().GetActiveEntry();
+  if (entry && page_id == entry->GetPageID()) {
+    NotifyInstantSupportDetermined(true);
+    controller_->ShowInstantPreview(this, reason, height, units);
+  }
+}