Make the FindInPageController implement its own delegate interface for

RenderViewHost so that we can get rid of the pass-throughs in WebContents.

I removed some redundant checks in WebContents when calling view() for
render_view_host() since that internally checks the null-ness of
render_view_host().

BUG=1323267


git-svn-id: svn://svn.chromium.org/chrome/trunk/src@881 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 91 insertions, 114 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index a634a38..818c321 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -1688,6 +1688,14 @@ void AutomationProvider::HandleFindInPageRequest(
   find_in_page_observer_.reset(new
       FindInPageNotificationObserver(this, tab_contents, message.routing_id()));
 
+  // The find in page dialog must be up for us to get the notification that the
+  // find was complete
+  if (tab_contents->AsWebContents()) {
+    NavigationController* tab = tab_tracker_->GetResource(handle);
+    Browser* browser = Browser::GetBrowserForController(tab, NULL);
+    tab_contents->AsWebContents()->OpenFindInPageWindow(*browser);
+  }
+
   // The explicit comparison to TRUE avoids a warning (C4800).
   tab_contents->StartFinding(
       FindInPageNotificationObserver::kFindInPageRequestId,
diff --git a/chrome/browser/find_in_page_controller.cc b/chrome/browser/find_in_page_controller.cc
index 9e2fd42..4d5c8ef 100644
--- a/chrome/browser/find_in_page_controller.cc
+++ b/chrome/browser/find_in_page_controller.cc
@@ -91,12 +91,6 @@ FindInPageController::FindInPageController(TabContents* parent_tab,
   // Start the process of animating the opening of the window.
   animation_.reset(new SlideAnimation(this));
   animation_->Show();
-
-  // We need to be notified about when results from a find operation are
-  // available.
-  NotificationService::current()->
-      AddObserver(this, NOTIFY_FIND_RESULT_AVAILABLE,
-                        Source<TabContents>(parent_tab));
 }
 
 FindInPageController::~FindInPageController() {
@@ -302,7 +296,7 @@ void FindInPageController::StartFinding(bool forward_direction) {
 }
 
 void FindInPageController::StopFinding(bool clear_selection) {
-  last_find_string_ = L"";
+  last_find_string_.clear();
   parent_tab_->StopFinding(clear_selection);
 }
 
@@ -364,10 +358,6 @@ void FindInPageController::OnFinalMessage(HWND window) {
   // destroyed resulting in the focus tracker trying to reference a deleted
   // focus manager.
   focus_tracker_.reset(NULL);
-
-  NotificationService::current()->
-      RemoveObserver(this, NOTIFY_FIND_RESULT_AVAILABLE,
-                     Source<TabContents>(parent_tab_));
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -442,6 +432,41 @@ void FindInPageController::AnimationEnded(
   }
 }
 
+void FindInPageController::FindReply(int request_id,
+                                     int number_of_matches,
+                                     const gfx::Rect& selection_rect,
+                                     int active_match_ordinal,
+                                     bool final_update) {
+  // Ignore responses for requests other than the one we have most recently
+  // issued. That way we won't act on stale results when the user has
+  // already typed in another query.
+  if (view_ && request_id == current_request_id_) {
+    view_->UpdateMatchCount(number_of_matches, final_update);
+    view_->UpdateActiveMatchOrdinal(active_match_ordinal);
+    view_->UpdateResultLabel();
+
+    // We now need to check if the window is obscuring the search results.
+    if (!selection_rect.IsEmpty())
+      MoveWindowIfNecessary(selection_rect);
+
+    // Once we find a match we no longer want to keep track of what had
+    // focus. EndFindSession will then set the focus to the page content.
+    if (number_of_matches > 0)
+      focus_tracker_.reset(NULL);
+  }
+
+  // Notify all observers of this notification, such as the automation
+  // providers which do UI tests for find in page.
+  FindNotificationDetails detail(request_id,
+                                 number_of_matches,
+                                 selection_rect,
+                                 active_match_ordinal,
+                                 final_update);
+  NotificationService::current()->
+      Notify(NOTIFY_FIND_RESULT_AVAILABLE, Source<TabContents>(parent_tab_),
+             Details<FindNotificationDetails>(&detail));
+}
+
 void FindInPageController::GetDialogBounds(gfx::Rect* bounds) {
   DCHECK(bounds);
 
@@ -680,35 +705,3 @@ void FindInPageController::UnregisterEscAccelerator() {
   if (current_target == this)
     focus_manager_->RegisterAccelerator(escape, old_accel_target_for_esc_);
 }
-
-void FindInPageController::Observe(NotificationType type,
-                                   const NotificationSource& source,
-                                   const NotificationDetails& details) {
-  switch (type) {
-    case NOTIFY_FIND_RESULT_AVAILABLE: {
-      Details<FindNotificationDetails> find_details(details);
-      // Ignore responses for requests other than the one we have most recently
-      // issued. That way we won't act on stale results when the user has
-      // already typed in another query.
-      if (view_ && find_details->request_id() == current_request_id_) {
-        view_->UpdateMatchCount(find_details->number_of_matches(),
-                                find_details->final_update());
-        view_->UpdateActiveMatchOrdinal(find_details->active_match_ordinal());
-        view_->UpdateResultLabel();
-
-        // We now need to check if the window is obscuring the search results.
-        if (!find_details->selection_rect().IsEmpty())
-          MoveWindowIfNecessary(find_details->selection_rect());
-
-        // Once we find a match we no longer want to keep track of what had
-        // focus. EndFindSession will then set the focus to the page content.
-        if (find_details->number_of_matches() > 0)
-          focus_tracker_.reset(NULL);
-      }
-      break;
-    }
-    default:
-      NOTREACHED() << L"Notification not handled";
-      return;
-  }
-}
diff --git a/chrome/browser/find_in_page_controller.h b/chrome/browser/find_in_page_controller.h
index 4848cd4..1ee56d4 100644
--- a/chrome/browser/find_in_page_controller.h
+++ b/chrome/browser/find_in_page_controller.h
@@ -31,8 +31,8 @@
 #define CHROME_BROWSER_FIND_IN_PAGE_CONTROLLER_H__
 
 #include "base/gfx/size.h"
+#include "chrome/browser/render_view_host_delegate.h"
 #include "chrome/common/slide_animation.h"
-#include "chrome/common/notification_service.h"
 #include "chrome/views/hwnd_view_container.h"
 
 class FindInPageView;
@@ -56,7 +56,7 @@ namespace ChromeViews {
 // We create one controller per tab and remember each search query per tab.
 //
 ////////////////////////////////////////////////////////////////////////////////
-class FindInPageController : public NotificationObserver,
+class FindInPageController : public RenderViewHostDelegate::FindInPage,
                              public ChromeViews::FocusChangeListener,
                              public ChromeViews::HWNDViewContainer,
                              public AnimationDelegate {
@@ -141,7 +141,15 @@ class FindInPageController : public NotificationObserver,
   // AnimationDelegate implementation:
   virtual void AnimationProgressed(const Animation* animation);
   virtual void AnimationEnded(const Animation* animation);
+
  private:
+  // RenderViewHostDelegate::FindInPage implementation.
+  virtual void FindReply(int request_id,
+                         int number_of_matches,
+                         const gfx::Rect& selection_rect,
+                         int active_match_ordinal,
+                         bool final_update);
+
   // Retrieves the boundaries that the FindInPage dialog has to work with within
   // the Chrome frame window. The resulting rectangle will be a rectangle that
   // overlaps the bottom of the Chrome toolbar by one pixel (so we can create
@@ -193,12 +201,6 @@ class FindInPageController : public NotificationObserver,
   // also: SetFocusChangeListener().
   void UnregisterEscAccelerator();
 
-  // Listens for notifications to know when search results are available, or
-  // when user selected a new tab (at which point we need to show/hide the
-  // FindInPage window depending on what the user selected).
-  virtual void Observe(NotificationType type, const NotificationSource& source,
-                       const NotificationDetails& details);
-
   // The tab we are associated with.
   TabContents* parent_tab_;
 
diff --git a/chrome/browser/render_view_host.cc b/chrome/browser/render_view_host.cc
index 00116b4..c37db34 100644
--- a/chrome/browser/render_view_host.cc
+++ b/chrome/browser/render_view_host.cc
@@ -339,10 +339,6 @@ void RenderViewHost::StopFinding(bool clear_selection) {
   Send(new ViewMsg_StopFinding(routing_id_, clear_selection));
 }
 
-void RenderViewHost::SendFindReplyAck() {
-  Send(new ViewMsg_FindReplyACK(routing_id_));
-}
-
 void RenderViewHost::AlterTextSize(text_zoom::TextSize size) {
   Send(new ViewMsg_AlterTextSize(routing_id_, size));
 }
@@ -936,9 +932,20 @@ void RenderViewHost::OnMsgFindReply(int request_id,
                                     const gfx::Rect& selection_rect,
                                     int active_match_ordinal,
                                     bool final_update) {
-  delegate_->FindReply(request_id, number_of_matches,
-                       selection_rect, active_match_ordinal, final_update);
-  SendFindReplyAck();
+  RenderViewHostDelegate::FindInPage* delegate =
+      delegate_->GetFindInPageDelegate();
+  if (!delegate)
+    return;
+  delegate->FindReply(request_id, number_of_matches, selection_rect,
+                      active_match_ordinal, final_update);
+
+  // Send a notification to the renderer that we are ready to receive more
+  // results from the scoping effort of the Find operation. The FindInPage
+  // scoping is asynchronous and periodically sends results back up to the
+  // browser using IPC. In an effort to not spam the browser we have the
+  // browser send an ACK for each FindReply message and have the renderer
+  // queue up the latest status message while waiting for this ACK.
+  Send(new ViewMsg_FindReplyACK(routing_id_));
 }
 
 void RenderViewHost::OnMsgUpdateFavIconURL(int32 page_id,
diff --git a/chrome/browser/render_view_host.h b/chrome/browser/render_view_host.h
index a05659a..7fdbfe0 100644
--- a/chrome/browser/render_view_host.h
+++ b/chrome/browser/render_view_host.h
@@ -34,6 +34,7 @@
 #include <vector>
 
 #include "base/scoped_handle.h"
+#include "chrome/browser/render_view_host_delegate.h"
 #include "chrome/browser/render_widget_host.h"
 #include "webkit/glue/password_form_dom_manager.h"
 
@@ -209,14 +210,6 @@ class RenderViewHost : public RenderWidgetHost {
   // clear the selection on the focused frame.
   void StopFinding(bool clear_selection);
 
-  // Sends a notification to the renderer that we are ready to receive more
-  // results from the scoping effort of the Find operation. The FindInPage
-  // scoping is asynchronous and periodically sends results back up to the
-  // browser using IPC. In an effort to not spam the browser we have the
-  // browser send an ACK for each FindReply message and have the renderer
-  // queue up the latest status message while waiting for this ACK.
-  void SendFindReplyAck();
-
   // Change the text size of the page.
   void AlterTextSize(text_zoom::TextSize size);
 
diff --git a/chrome/browser/render_view_host_delegate.h b/chrome/browser/render_view_host_delegate.h
index b80d328..959f427 100644
--- a/chrome/browser/render_view_host_delegate.h
+++ b/chrome/browser/render_view_host_delegate.h
@@ -72,6 +72,19 @@ enum LoadState;
 //
 class RenderViewHostDelegate {
  public:
+  class FindInPage {
+   public:
+    // A find operation in the current page completed.
+    virtual void FindReply(int request_id,
+                           int number_of_matches,
+                           const gfx::Rect& selection_rect,
+                           int active_match_ordinal,
+                           bool final_update) = 0;
+  };
+
+  // Returns the current find in page delegate, if any.
+  virtual FindInPage* GetFindInPageDelegate() { return NULL; }
+
   // Retrieves the profile to be used.
   virtual Profile* GetProfile() const = 0;
 
@@ -167,21 +180,10 @@ class RenderViewHostDelegate {
       const GURL& url,
       bool showing_repost_interstitial) { }
 
-  // A find operation in the current page completed.
-  virtual void FindReply(int request_id,
-                         int number_of_matches,
-                         const gfx::Rect& selection_rect,
-                         int active_match_ordinal,
-                         bool final_update) { }
-
   // The URL for the FavIcon of a page has changed.
   virtual void UpdateFavIconURL(RenderViewHost* render_view_host,
                                 int32 page_id, const GURL& icon_url) { }
 
-  // The FavIcon for a page needs to be downloaded.
-  virtual void DownloadFavIcon(RenderViewHost* render_view_host,
-                               int32 page_id) { }
-
   // An image that was requested to be downloaded by DownloadImage has
   // completed.
   virtual void DidDownloadImage(RenderViewHost* render_view_host,
diff --git a/chrome/browser/tab_contents.h b/chrome/browser/tab_contents.h
index e83cd72..5a56f5e 100644
--- a/chrome/browser/tab_contents.h
+++ b/chrome/browser/tab_contents.h
@@ -263,10 +263,6 @@ class TabContents : public PageNavigator,
   void AddConstrainedPopup(TabContents* new_contents,
                            const gfx::Rect& initial_pos);
 
-  // Find functions: subclasses should override to implement "Find" in a
-  // sensible way for their content types
-  virtual bool CanFind() const { return false; }
-
   // An asynchronous call to trigger the string search in the page.
   // It sends an IPC message to the Renderer that handles the string
   // search, selecting the matches and setting the caret positions.
diff --git a/chrome/browser/web_contents.cc b/chrome/browser/web_contents.cc
index 1774c1d..3732714 100644
--- a/chrome/browser/web_contents.cc
+++ b/chrome/browser/web_contents.cc
@@ -672,7 +672,7 @@ void WebContents::Stop() {
 void WebContents::DidBecomeSelected() {
   TabContents::DidBecomeSelected();
 
-  if (render_view_host() && view())
+  if (view())
     view()->DidBecomeSelected();
 
   CacheManagerHost::GetInstance()->ObserveActivity(process()->host_id());
@@ -685,7 +685,7 @@ void WebContents::WasHidden() {
     // is because closing the tab calls WebContents::Destroy(), which removes
     // the |render_view_host()|; then when we actually destroy the window,
     // OnWindowPosChanged() notices and calls HideContents() (which calls us).
-    if (render_view_host() && view())
+    if (view())
       view()->WasHidden();
 
     // Loop through children and send WasHidden to them, too.
@@ -719,9 +719,6 @@ void WebContents::StopFinding(bool clear_selection) {
 }
 
 void WebContents::OpenFindInPageWindow(const Browser& browser) {
-  if (!CanFind())
-    return;
-
   if (!find_in_page_controller_.get()) {
     // Get the Chrome top-level (Frame) window.
     HWND hwnd = browser.GetTopLevelHWND();
@@ -739,9 +736,6 @@ void WebContents::ReparentFindWindow(HWND new_parent) {
 }
 
 bool WebContents::AdvanceFindSelection(bool forward_direction) {
-  if (!CanFind())
-    return false;
-
   // If no controller has been created or it doesn't know what to search for
   // then just return false so that caller knows that it should create and
   // show the window.
@@ -1078,6 +1072,12 @@ bool WebContents::IsActiveEntry(int32 page_id) {
 ///////////////////////////////////////////////////////////////////////////////
 // RenderViewHostDelegate implementation:
 
+RenderViewHostDelegate::FindInPage* WebContents::GetFindInPageDelegate() {
+  // The find in page controller implements this interface for us. Our return
+  // value can be NULL, so it's fine if the find in controller doesn't exist.
+  return find_in_page_controller_.get();
+}
+
 Profile* WebContents::GetProfile() const {
   return profile();
 }
@@ -1894,25 +1894,6 @@ void WebContents::DidFailProvisionalLoadWithError(
              Details<ProvisionalLoadDetails>(&details));
 }
 
-void WebContents::FindReply(int request_id,
-                            int number_of_matches,
-                            const gfx::Rect& selection_rect,
-                            int active_match_ordinal,
-                            bool final_update) {
-  // ViewMsgHost_FindResult message received. The find-in-page result
-  // is obtained. Fire the notification
-  FindNotificationDetails detail(request_id,
-                                 number_of_matches,
-                                 selection_rect,
-                                 active_match_ordinal,
-                                 final_update);
-  // Notify all observers of this notification.
-  // The current find box owns one such observer.
-  NotificationService::current()->
-      Notify(NOTIFY_FIND_RESULT_AVAILABLE, Source<TabContents>(this),
-             Details<FindNotificationDetails>(&detail));
-}
-
 void WebContents::UpdateFavIconURL(RenderViewHost* render_view_host,
                                    int32 page_id,
                                    const GURL& icon_url) {
diff --git a/chrome/browser/web_contents.h b/chrome/browser/web_contents.h
index 25c32a6..0ece47b 100644
--- a/chrome/browser/web_contents.h
+++ b/chrome/browser/web_contents.h
@@ -106,7 +106,6 @@ class WebContents : public TabContents,
   virtual std::wstring GetStatusText() const;
 
   // Find functions
-  virtual bool CanFind() const { return true; }
   virtual void StartFinding(int request_id,
                             const std::wstring& search_string,
                             bool forward,
@@ -369,6 +368,7 @@ class WebContents : public TabContents,
   virtual ~WebContents();
 
   // RenderViewHostDelegate
+  virtual RenderViewHostDelegate::FindInPage* GetFindInPageDelegate();
   virtual Profile* GetProfile() const;
 
   virtual void CreateView(int route_id, HANDLE modal_dialog_event);
@@ -415,11 +415,6 @@ class WebContents : public TabContents,
       int error_code,
       const GURL& url,
       bool showing_repost_interstitial);
-  virtual void FindReply(int request_id,
-                         int number_of_matches,
-                         const gfx::Rect& selection_rect,
-                         int active_match_ordinal,
-                         bool final_update);
   virtual void UpdateFavIconURL(RenderViewHost* render_view_host,
                                 int32 page_id, const GURL& icon_url);
   virtual void DidDownloadImage(RenderViewHost* render_view_host,