diff options
author | kmadhusu@chromium.org <kmadhusu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-28 22:58:06 +0000 |
---|---|---|
committer | kmadhusu@chromium.org <kmadhusu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-28 22:58:06 +0000 |
commit | 416d753a3646cef8ae2e3174e93e4ed266913432 (patch) | |
tree | 7ce065a402dd4633b4c1ef5ee11cf0410f66e716 /chrome/browser/ui | |
parent | c4edfeb7480fbe2ef45415fff655ca75688dac14 (diff) | |
download | chromium_src-416d753a3646cef8ae2e3174e93e4ed266913432.zip chromium_src-416d753a3646cef8ae2e3174e93e4ed266913432.tar.gz chromium_src-416d753a3646cef8ae2e3174e93e4ed266913432.tar.bz2 |
Send ChromeViewMsg_SearchBoxFocusChanged IPC msg and ChromeViewMsg_SearchBoxSetInputInProgress IPC msg from SearchIPCRouter.
This CL,
- Removes OmniboxFocusState details from InstantController.
- Deletes InstantIPCSender and updates the InstantPage class.
- Adds a new function in OmniboxCurrentPageDelegate to notify the client (such as SearchTabHelper) about the omnibox focus state change event.
- Adds a new function in SearchIPCRouter to send a message to the underlying page about the focus change event.
- Adds unit tests.
BUG=272583
TEST=none
Review URL: https://codereview.chromium.org/159713009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@254262 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/ui')
21 files changed, 162 insertions, 198 deletions
diff --git a/chrome/browser/ui/omnibox/omnibox_current_page_delegate.h b/chrome/browser/ui/omnibox/omnibox_current_page_delegate.h index 5674f78..bbd4ce5 100644 --- a/chrome/browser/ui/omnibox/omnibox_current_page_delegate.h +++ b/chrome/browser/ui/omnibox/omnibox_current_page_delegate.h @@ -6,6 +6,7 @@ #define CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_CURRENT_PAGE_DELEGATE_H_ #include "base/basictypes.h" +#include "chrome/common/omnibox_focus_state.h" #include "ui/base/window_open_disposition.h" class GURL; @@ -57,6 +58,10 @@ class OmniboxCurrentPageDelegate { // Called to notify clients that the omnibox input state has changed. virtual void OnInputStateChanged() = 0; + // Called to notify clients that the omnibox focus state has changed. + virtual void OnFocusChanged(OmniboxFocusState state, + OmniboxFocusChangeReason reason) = 0; + // Performs prerendering for |match|. virtual void DoPrerender(const AutocompleteMatch& match) = 0; }; diff --git a/chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.cc b/chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.cc index d3708a8..41ab3f7 100644 --- a/chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.cc +++ b/chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.cc @@ -89,6 +89,15 @@ void OmniboxCurrentPageDelegateImpl::OnInputStateChanged() { controller_->GetWebContents())->OmniboxInputStateChanged(); } +void OmniboxCurrentPageDelegateImpl::OnFocusChanged( + OmniboxFocusState state, + OmniboxFocusChangeReason reason) { + if (!controller_->GetWebContents()) + return; + SearchTabHelper::FromWebContents( + controller_->GetWebContents())->OmniboxFocusChanged(state, reason); +} + void OmniboxCurrentPageDelegateImpl::DoPrerender( const AutocompleteMatch& match) { content::WebContents* web_contents = controller_->GetWebContents(); diff --git a/chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.h b/chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.h index 78e4682..541f94b 100644 --- a/chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.h +++ b/chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.h @@ -31,6 +31,8 @@ class OmniboxCurrentPageDelegateImpl : public OmniboxCurrentPageDelegate { const AutocompleteMatch& match, WindowOpenDisposition disposition) OVERRIDE; virtual void OnInputStateChanged() OVERRIDE; + virtual void OnFocusChanged(OmniboxFocusState state, + OmniboxFocusChangeReason reason) OVERRIDE; virtual void DoPrerender(const AutocompleteMatch& match) OVERRIDE; private: diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.cc b/chrome/browser/ui/omnibox/omnibox_edit_model.cc index f724bc5..d5cc346 100644 --- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc +++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc @@ -50,7 +50,6 @@ #include "chrome/browser/ui/omnibox/omnibox_popup_model.h" #include "chrome/browser/ui/omnibox/omnibox_popup_view.h" #include "chrome/browser/ui/omnibox/omnibox_view.h" -#include "chrome/browser/ui/search/instant_controller.h" #include "chrome/browser/ui/search/instant_search_prerenderer.h" #include "chrome/browser/ui/search/search_tab_helper.h" #include "chrome/browser/ui/toolbar/toolbar_model.h" @@ -934,13 +933,6 @@ void OmniboxEditModel::SetCaretVisibility(bool visible) { } void OmniboxEditModel::OnWillKillFocus(gfx::NativeView view_gaining_focus) { - InstantController* instant = GetInstantController(); - if (instant) { - instant->OmniboxFocusChanged(OMNIBOX_FOCUS_NONE, - OMNIBOX_FOCUS_CHANGE_EXPLICIT, - view_gaining_focus); - } - // TODO(jered): Rip this out along with StartZeroSuggest. autocomplete_controller()->StopZeroSuggest(); @@ -949,10 +941,7 @@ void OmniboxEditModel::OnWillKillFocus(gfx::NativeView view_gaining_focus) { } void OmniboxEditModel::OnKillFocus() { - // TODO(samarth): determine if it is safe to move the call to - // OmniboxFocusChanged() from OnWillKillFocus() to here, which would let us - // just call SetFocusState() to handle the state change. - focus_state_ = OMNIBOX_FOCUS_NONE; + SetFocusState(OMNIBOX_FOCUS_NONE, OMNIBOX_FOCUS_CHANGE_EXPLICIT); focus_source_ = INVALID; control_key_state_ = UP; paste_state_ = NONE; @@ -1436,10 +1425,6 @@ void OmniboxEditModel::SetFocusState(OmniboxFocusState state, if (state == focus_state_) return; - InstantController* instant = GetInstantController(); - if (instant) - instant->OmniboxFocusChanged(state, reason, NULL); - // Update state and notify view if the omnibox has focus and the caret // visibility changed. const bool was_caret_visible = is_caret_visible(); @@ -1447,4 +1432,6 @@ void OmniboxEditModel::SetFocusState(OmniboxFocusState state, if (focus_state_ != OMNIBOX_FOCUS_NONE && is_caret_visible() != was_caret_visible) view_->ApplyCaretVisibility(); + + delegate_->OnFocusChanged(focus_state_, reason); } diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.h b/chrome/browser/ui/omnibox/omnibox_edit_model.h index e1fa60e..57c9930 100644 --- a/chrome/browser/ui/omnibox/omnibox_edit_model.h +++ b/chrome/browser/ui/omnibox/omnibox_edit_model.h @@ -332,6 +332,10 @@ class OmniboxEditModel { // InstantController via OmniboxEditController, because the only valid pointer // to InstantController is kept in Browser. We should try to get rid of this, // maybe by ensuring InstantController lives as long as Browser. + // + // TODO(kmadhusu): Remove this function and update the callers. + // + // This method is deprecated and should not be used in new code. InstantController* GetInstantController() const; // Name of the histogram tracking cut or copy omnibox commands. diff --git a/chrome/browser/ui/search/instant_controller.cc b/chrome/browser/ui/search/instant_controller.cc index 93597b9..02a6000 100644 --- a/chrome/browser/ui/search/instant_controller.cc +++ b/chrome/browser/ui/search/instant_controller.cc @@ -79,9 +79,7 @@ void EnsureSearchTermsAreSet(content::WebContents* contents, } // namespace InstantController::InstantController(BrowserInstantController* browser) - : browser_(browser), - omnibox_focus_state_(OMNIBOX_FOCUS_NONE), - omnibox_focus_change_reason_(OMNIBOX_FOCUS_CHANGE_EXPLICIT) { + : browser_(browser) { } InstantController::~InstantController() { @@ -121,31 +119,6 @@ bool InstantController::SubmitQuery(const base::string16& search_terms) { return false; } -void InstantController::OmniboxFocusChanged( - OmniboxFocusState state, - OmniboxFocusChangeReason reason, - gfx::NativeView view_gaining_focus) { - LOG_INSTANT_DEBUG_EVENT(this, base::StringPrintf( - "OmniboxFocusChanged: %d to %d for reason %d", omnibox_focus_state_, - state, reason)); - - omnibox_focus_state_ = state; - if (!instant_tab_) - return; - - content::NotificationService::current()->Notify( - chrome::NOTIFICATION_OMNIBOX_FOCUS_CHANGED, - content::Source<InstantController>(this), - content::NotificationService::NoDetails()); - - instant_tab_->sender()->FocusChanged(omnibox_focus_state_, reason); - // Don't send oninputstart/oninputend updates in response to focus changes - // if there's a navigation in progress. This prevents Chrome from sending - // a spurious oninputend when the user accepts a match in the omnibox. - if (instant_tab_->contents()->GetController().GetPendingEntry() == NULL) - instant_tab_->sender()->SetInputInProgress(IsInputInProgress()); -} - void InstantController::SearchModeChanged(const SearchMode& old_mode, const SearchMode& new_mode) { LOG_INSTANT_DEBUG_EVENT(this, base::StringPrintf( @@ -154,9 +127,6 @@ void InstantController::SearchModeChanged(const SearchMode& old_mode, search_mode_ = new_mode; ResetInstantTab(); - - if (instant_tab_ && old_mode.is_ntp() != new_mode.is_ntp()) - instant_tab_->sender()->SetInputInProgress(IsInputInProgress()); } void InstantController::ActiveTabChanged() { @@ -249,18 +219,9 @@ void InstantController::UpdateInfoForInstantTab() { instant_service->UpdateThemeInfo(); instant_service->UpdateMostVisitedItemsInfo(); } - - instant_tab_->sender()->FocusChanged(omnibox_focus_state_, - omnibox_focus_change_reason_); - instant_tab_->sender()->SetInputInProgress(IsInputInProgress()); } } -bool InstantController::IsInputInProgress() const { - return !search_mode_.is_ntp() && - omnibox_focus_state_ == OMNIBOX_FOCUS_VISIBLE; -} - InstantService* InstantController::GetInstantService() const { return InstantServiceFactory::GetForProfile(profile()); } diff --git a/chrome/browser/ui/search/instant_controller.h b/chrome/browser/ui/search/instant_controller.h index 2ffcdf0..683a74c 100644 --- a/chrome/browser/ui/search/instant_controller.h +++ b/chrome/browser/ui/search/instant_controller.h @@ -16,7 +16,6 @@ #include "base/strings/string16.h" #include "chrome/browser/ui/search/instant_page.h" #include "chrome/common/instant_types.h" -#include "chrome/common/omnibox_focus_state.h" #include "chrome/common/search_types.h" #include "ui/gfx/native_widget_types.h" @@ -56,13 +55,6 @@ class InstantController : public InstantPage::Delegate { // the search, this does so and returns true. Else, returns false. bool SubmitQuery(const base::string16& search_terms); - // 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. Bind |instant_tab_| if the // |new_mode| reflects an Instant search results page. void SearchModeChanged(const SearchMode& old_mode, @@ -147,10 +139,6 @@ class InstantController : public InstantPage::Delegate { // Sends theme info, omnibox bounds, etc. down to the Instant tab. void UpdateInfoForInstantTab(); - // Returns whether input is in progress, i.e. if the omnibox has focus and the - // active tab is in mode SEARCH_SUGGESTIONS. - bool IsInputInProgress() const; - // Returns the InstantService for the browser profile. InstantService* GetInstantService() const; @@ -159,12 +147,6 @@ class InstantController : public InstantPage::Delegate { // The instance of InstantPage maintained by InstantController. scoped_ptr<InstantTab> instant_tab_; - // Omnibox focus state. - OmniboxFocusState omnibox_focus_state_; - - // The reason for the most recent omnibox focus change. - OmniboxFocusChangeReason omnibox_focus_change_reason_; - // The search model mode for the active tab. SearchMode search_mode_; diff --git a/chrome/browser/ui/search/instant_ipc_sender.cc b/chrome/browser/ui/search/instant_ipc_sender.cc deleted file mode 100644 index 22c2120..0000000 --- a/chrome/browser/ui/search/instant_ipc_sender.cc +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright 2013 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/ui/search/instant_ipc_sender.h" - -#include "chrome/common/render_messages.h" - -namespace { - -// Implementation for regular profiles. -class InstantIPCSenderImpl : public InstantIPCSender { - public: - InstantIPCSenderImpl() {} - virtual ~InstantIPCSenderImpl() {} - - private: - virtual void FocusChanged(OmniboxFocusState state, - OmniboxFocusChangeReason reason) OVERRIDE { - Send(new ChromeViewMsg_SearchBoxFocusChanged(routing_id(), state, reason)); - } - - virtual void SetInputInProgress(bool input_in_progress) OVERRIDE { - Send(new ChromeViewMsg_SearchBoxSetInputInProgress( - routing_id(), input_in_progress)); - } - - DISALLOW_COPY_AND_ASSIGN(InstantIPCSenderImpl); -}; - -// Implementation for incognito profiles. -class IncognitoInstantIPCSenderImpl : public InstantIPCSender { - public: - IncognitoInstantIPCSenderImpl() {} - virtual ~IncognitoInstantIPCSenderImpl() {} - - private: - DISALLOW_COPY_AND_ASSIGN(IncognitoInstantIPCSenderImpl); -}; - -} // anonymous namespace - -// static -scoped_ptr<InstantIPCSender> InstantIPCSender::Create(bool is_incognito) { - scoped_ptr<InstantIPCSender> sender( - is_incognito ? - static_cast<InstantIPCSender*>(new IncognitoInstantIPCSenderImpl()) : - static_cast<InstantIPCSender*>(new InstantIPCSenderImpl())); - return sender.Pass(); -} - -void InstantIPCSender::SetContents(content::WebContents* web_contents) { - Observe(web_contents); -} diff --git a/chrome/browser/ui/search/instant_ipc_sender.h b/chrome/browser/ui/search/instant_ipc_sender.h deleted file mode 100644 index 564fbf3..0000000 --- a/chrome/browser/ui/search/instant_ipc_sender.h +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2013 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_UI_SEARCH_INSTANT_IPC_SENDER_H_ -#define CHROME_BROWSER_UI_SEARCH_INSTANT_IPC_SENDER_H_ - -#include "base/memory/scoped_ptr.h" -#include "chrome/common/omnibox_focus_state.h" -#include "content/public/browser/web_contents_observer.h" - -namespace IPC { -class Sender; -} - -class InstantIPCSender : public content::WebContentsObserver { - public: - // Creates a new instance of InstantIPCSender. If |is_incognito| is true, - // the instance will only send appropriate IPCs for incognito profiles. - static scoped_ptr<InstantIPCSender> Create(bool is_incognito); - - virtual ~InstantIPCSender() {} - - // Sets |web_contents| as the receiver of IPCs. - void SetContents(content::WebContents* web_contents); - - // Tells the page that the omnibox focus has changed. - virtual void FocusChanged(OmniboxFocusState state, - OmniboxFocusChangeReason reason) {} - - // Tells the page that user input started or stopped. - virtual void SetInputInProgress(bool input_in_progress) {} - - protected: - InstantIPCSender() {} - - private: - DISALLOW_COPY_AND_ASSIGN(InstantIPCSender); -}; - -#endif // CHROME_BROWSER_UI_SEARCH_INSTANT_IPC_SENDER_H_ diff --git a/chrome/browser/ui/search/instant_page.cc b/chrome/browser/ui/search/instant_page.cc index 0b4f692..9ffc0b7 100644 --- a/chrome/browser/ui/search/instant_page.cc +++ b/chrome/browser/ui/search/instant_page.cc @@ -44,7 +44,6 @@ InstantPage::InstantPage(Delegate* delegate, const std::string& instant_url, Profile* profile, bool is_incognito) : profile_(profile), delegate_(delegate), - ipc_sender_(InstantIPCSender::Create(is_incognito)), instant_url_(instant_url), is_incognito_(is_incognito) { } @@ -55,7 +54,6 @@ void InstantPage::SetContents(content::WebContents* web_contents) { if (!web_contents) return; - sender()->SetContents(web_contents); Observe(web_contents); SearchModel* model = SearchTabHelper::FromWebContents(contents())->model(); model->AddObserver(this); @@ -98,6 +96,5 @@ void InstantPage::ClearContents() { if (contents()) SearchTabHelper::FromWebContents(contents())->model()->RemoveObserver(this); - sender()->SetContents(NULL); Observe(NULL); } diff --git a/chrome/browser/ui/search/instant_page.h b/chrome/browser/ui/search/instant_page.h index a054cb4..d65dba1 100644 --- a/chrome/browser/ui/search/instant_page.h +++ b/chrome/browser/ui/search/instant_page.h @@ -10,9 +10,7 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" #include "base/gtest_prod_util.h" -#include "base/memory/scoped_ptr.h" #include "base/strings/string16.h" -#include "chrome/browser/ui/search/instant_ipc_sender.h" #include "chrome/browser/ui/search/search_model_observer.h" #include "content/public/browser/web_contents_observer.h" #include "content/public/common/page_transition_types.h" @@ -61,9 +59,6 @@ class InstantPage : public content::WebContentsObserver, // The WebContents corresponding to the page we're talking to. May be NULL. content::WebContents* contents() const { return web_contents(); } - // Used to send IPC messages to the page. - InstantIPCSender* sender() const { return ipc_sender_.get(); } - // Returns the Instant URL that was loaded for this page. Returns the empty // string if no URL was explicitly loaded as is the case for InstantTab. virtual const std::string& instant_url() const; @@ -105,8 +100,6 @@ class InstantPage : public content::WebContentsObserver, FRIEND_TEST_ALL_PREFIXES(InstantPageTest, PageURLDoesntBelongToInstantRenderer); FRIEND_TEST_ALL_PREFIXES(InstantPageTest, PageSupportsInstant); - FRIEND_TEST_ALL_PREFIXES(InstantPageTest, - AppropriateMessagesSentToIncognitoPages); // Overridden from content::WebContentsObserver: virtual void DidCommitProvisionalLoadForFrame( @@ -131,7 +124,6 @@ class InstantPage : public content::WebContentsObserver, Profile* profile_; Delegate* const delegate_; - scoped_ptr<InstantIPCSender> ipc_sender_; const std::string instant_url_; const bool is_incognito_; diff --git a/chrome/browser/ui/search/instant_page_unittest.cc b/chrome/browser/ui/search/instant_page_unittest.cc index 6d85cfb..1632bcc 100644 --- a/chrome/browser/ui/search/instant_page_unittest.cc +++ b/chrome/browser/ui/search/instant_page_unittest.cc @@ -152,18 +152,3 @@ TEST_F(InstantPageTest, PageSupportsInstant) { SearchTabHelper::FromWebContents(web_contents())->InstantSupportChanged(true); EXPECT_TRUE(page->supports_instant()); } - -TEST_F(InstantPageTest, AppropriateMessagesSentToIncognitoPages) { - page.reset(new InstantPage(&delegate, "", NULL, true)); - page->SetContents(web_contents()); - NavigateAndCommit(GURL(chrome::kChromeSearchLocalNtpUrl)); - process()->sink().ClearMessages(); - - // Incognito pages should not get any others. - page->sender()->FocusChanged( - OMNIBOX_FOCUS_NONE, OMNIBOX_FOCUS_CHANGE_EXPLICIT); - EXPECT_FALSE(MessageWasSent(ChromeViewMsg_SearchBoxFocusChanged::ID)); - - page->sender()->SetInputInProgress(false); - EXPECT_FALSE(MessageWasSent(ChromeViewMsg_SearchBoxSetInputInProgress::ID)); -} diff --git a/chrome/browser/ui/search/instant_test_utils.cc b/chrome/browser/ui/search/instant_test_utils.cc index cc3e96d..3115bf3 100644 --- a/chrome/browser/ui/search/instant_test_utils.cc +++ b/chrome/browser/ui/search/instant_test_utils.cc @@ -12,6 +12,8 @@ #include "chrome/browser/search_engines/template_url_service.h" #include "chrome/browser/search_engines/template_url_service_factory.h" #include "chrome/browser/ui/omnibox/omnibox_view.h" +#include "chrome/browser/ui/search/search_tab_helper.h" +#include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/pref_names.h" #include "chrome/test/base/interactive_test_utils.h" @@ -92,10 +94,12 @@ void InstantTestBase::Init(const GURL& instant_url, } void InstantTestBase::FocusOmnibox() { - // If the omnibox already has focus, just notify Instant. + // If the omnibox already has focus, just notify SearchTabHelper. if (omnibox()->model()->has_focus()) { - instant()->OmniboxFocusChanged(OMNIBOX_FOCUS_VISIBLE, - OMNIBOX_FOCUS_CHANGE_EXPLICIT, NULL); + content::WebContents* active_tab = + browser_->tab_strip_model()->GetActiveWebContents(); + SearchTabHelper::FromWebContents(active_tab)->OmniboxFocusChanged( + OMNIBOX_FOCUS_VISIBLE, OMNIBOX_FOCUS_CHANGE_EXPLICIT); } else { browser_->window()->GetLocationBar()->FocusLocation(false); } diff --git a/chrome/browser/ui/search/search_ipc_router.cc b/chrome/browser/ui/search/search_ipc_router.cc index 1d4a73e..013e351 100644 --- a/chrome/browser/ui/search/search_ipc_router.cc +++ b/chrome/browser/ui/search/search_ipc_router.cc @@ -70,6 +70,22 @@ void SearchIPCRouter::SetOmniboxStartMargin(int start_margin) { Send(new ChromeViewMsg_SearchBoxMarginChange(routing_id(), start_margin)); } +void SearchIPCRouter::SetInputInProgress(bool input_in_progress) { + if (!policy_->ShouldSendSetInputInProgress(is_active_tab_)) + return; + + Send(new ChromeViewMsg_SearchBoxSetInputInProgress(routing_id(), + input_in_progress)); +} + +void SearchIPCRouter::OmniboxFocusChanged(OmniboxFocusState state, + OmniboxFocusChangeReason reason) { + if (!policy_->ShouldSendOmniboxFocusChanged()) + return; + + Send(new ChromeViewMsg_SearchBoxFocusChanged(routing_id(), state, reason)); +} + void SearchIPCRouter::SendMostVisitedItems( const std::vector<InstantMostVisitedItem>& items) { if (!policy_->ShouldSendMostVisitedItems()) diff --git a/chrome/browser/ui/search/search_ipc_router.h b/chrome/browser/ui/search/search_ipc_router.h index ab90bd9..69dfb52 100644 --- a/chrome/browser/ui/search/search_ipc_router.h +++ b/chrome/browser/ui/search/search_ipc_router.h @@ -98,6 +98,8 @@ class SearchIPCRouter : public content::WebContentsObserver { virtual bool ShouldSendSetDisplayInstantResults() = 0; virtual bool ShouldSendSetSuggestionToPrefetch() = 0; virtual bool ShouldSendSetOmniboxStartMargin() = 0; + virtual bool ShouldSendSetInputInProgress(bool is_active_tab) = 0; + virtual bool ShouldSendOmniboxFocusChanged() = 0; virtual bool ShouldSendMostVisitedItems() = 0; virtual bool ShouldSendThemeBackgroundInfo() = 0; virtual bool ShouldSendToggleVoiceSearch() = 0; @@ -130,6 +132,13 @@ class SearchIPCRouter : public content::WebContentsObserver { // align text or assets properly with the omnibox. void SetOmniboxStartMargin(int start_margin); + // Tells the page that user input started or stopped. + void SetInputInProgress(bool input_in_progress); + + // Tells the page that the omnibox focus has changed. + void OmniboxFocusChanged(OmniboxFocusState state, + OmniboxFocusChangeReason reason); + // Tells the renderer about the most visited items. void SendMostVisitedItems(const std::vector<InstantMostVisitedItem>& items); diff --git a/chrome/browser/ui/search/search_ipc_router_policy_impl.cc b/chrome/browser/ui/search/search_ipc_router_policy_impl.cc index 954810b..69fec25 100644 --- a/chrome/browser/ui/search/search_ipc_router_policy_impl.cc +++ b/chrome/browser/ui/search/search_ipc_router_policy_impl.cc @@ -75,6 +75,15 @@ bool SearchIPCRouterPolicyImpl::ShouldSendSetOmniboxStartMargin() { return true; } +bool SearchIPCRouterPolicyImpl::ShouldSendSetInputInProgress( + bool is_active_tab) { + return is_active_tab && !is_incognito_; +} + +bool SearchIPCRouterPolicyImpl::ShouldSendOmniboxFocusChanged() { + return !is_incognito_; +} + bool SearchIPCRouterPolicyImpl::ShouldSendMostVisitedItems() { return !is_incognito_ && chrome::IsInstantNTP(web_contents_); } diff --git a/chrome/browser/ui/search/search_ipc_router_policy_impl.h b/chrome/browser/ui/search/search_ipc_router_policy_impl.h index 235b6f8..2812a64 100644 --- a/chrome/browser/ui/search/search_ipc_router_policy_impl.h +++ b/chrome/browser/ui/search/search_ipc_router_policy_impl.h @@ -34,6 +34,8 @@ class SearchIPCRouterPolicyImpl : public SearchIPCRouter::Policy { virtual bool ShouldSendSetDisplayInstantResults() OVERRIDE; virtual bool ShouldSendSetSuggestionToPrefetch() OVERRIDE; virtual bool ShouldSendSetOmniboxStartMargin() OVERRIDE; + virtual bool ShouldSendSetInputInProgress(bool is_active_tab) OVERRIDE; + virtual bool ShouldSendOmniboxFocusChanged() OVERRIDE; virtual bool ShouldSendMostVisitedItems() OVERRIDE; virtual bool ShouldSendThemeBackgroundInfo() OVERRIDE; virtual bool ShouldSendToggleVoiceSearch() OVERRIDE; diff --git a/chrome/browser/ui/search/search_ipc_router_policy_unittest.cc b/chrome/browser/ui/search/search_ipc_router_policy_unittest.cc index c135dcc..e353ee85 100644 --- a/chrome/browser/ui/search/search_ipc_router_policy_unittest.cc +++ b/chrome/browser/ui/search/search_ipc_router_policy_unittest.cc @@ -146,6 +146,7 @@ TEST_F(SearchIPCRouterPolicyTest, DoNotProcessMessagesForInactiveTab) { EXPECT_FALSE(router_policy->ShouldProcessFocusOmnibox(false)); EXPECT_FALSE(router_policy->ShouldProcessNavigateToURL(false)); EXPECT_FALSE(router_policy->ShouldProcessPasteIntoOmnibox(false)); + EXPECT_FALSE(router_policy->ShouldSendSetInputInProgress(false)); } TEST_F(SearchIPCRouterPolicyTest, SendSetDisplayInstantResults) { @@ -174,6 +175,8 @@ TEST_F(SearchIPCRouterPolicyTest, EXPECT_FALSE(router_policy->ShouldSendSetPromoInformation()); EXPECT_FALSE(router_policy->ShouldSendThemeBackgroundInfo()); EXPECT_FALSE(router_policy->ShouldSendMostVisitedItems()); + EXPECT_FALSE(router_policy->ShouldSendSetInputInProgress(true)); + EXPECT_FALSE(router_policy->ShouldSendOmniboxFocusChanged()); } TEST_F(SearchIPCRouterPolicyTest, diff --git a/chrome/browser/ui/search/search_ipc_router_unittest.cc b/chrome/browser/ui/search/search_ipc_router_unittest.cc index a95fd12..b0418d8 100644 --- a/chrome/browser/ui/search/search_ipc_router_unittest.cc +++ b/chrome/browser/ui/search/search_ipc_router_unittest.cc @@ -75,6 +75,8 @@ class MockSearchIPCRouterPolicy : public SearchIPCRouter::Policy { MOCK_METHOD0(ShouldSendSetDisplayInstantResults, bool()); MOCK_METHOD0(ShouldSendSetSuggestionToPrefetch, bool()); MOCK_METHOD0(ShouldSendSetOmniboxStartMargin, bool()); + MOCK_METHOD1(ShouldSendSetInputInProgress, bool(bool)); + MOCK_METHOD0(ShouldSendOmniboxFocusChanged, bool()); MOCK_METHOD0(ShouldSendMostVisitedItems, bool()); MOCK_METHOD0(ShouldSendThemeBackgroundInfo, bool()); MOCK_METHOD0(ShouldSendToggleVoiceSearch, bool()); @@ -754,6 +756,56 @@ TEST_F(SearchIPCRouterTest, DoNotSendSetOmniboxStartMargin) { EXPECT_FALSE(MessageWasSent(ChromeViewMsg_SearchBoxMarginChange::ID)); } +TEST_F(SearchIPCRouterTest, SendOmniboxFocusChange) { + NavigateAndCommitActiveTab(GURL(chrome::kChromeSearchLocalNtpUrl)); + SetupMockDelegateAndPolicy(); + MockSearchIPCRouterPolicy* policy = GetSearchIPCRouterPolicy(); + EXPECT_CALL(*policy, ShouldSendOmniboxFocusChanged()).Times(1) + .WillOnce(testing::Return(true)); + + process()->sink().ClearMessages(); + GetSearchIPCRouter().OmniboxFocusChanged(OMNIBOX_FOCUS_NONE, + OMNIBOX_FOCUS_CHANGE_EXPLICIT); + EXPECT_TRUE(MessageWasSent(ChromeViewMsg_SearchBoxFocusChanged::ID)); +} + +TEST_F(SearchIPCRouterTest, DoNotSendOmniboxFocusChange) { + NavigateAndCommitActiveTab(GURL(chrome::kChromeSearchLocalNtpUrl)); + SetupMockDelegateAndPolicy(); + MockSearchIPCRouterPolicy* policy = GetSearchIPCRouterPolicy(); + EXPECT_CALL(*policy, ShouldSendOmniboxFocusChanged()).Times(1) + .WillOnce(testing::Return(false)); + + process()->sink().ClearMessages(); + GetSearchIPCRouter().OmniboxFocusChanged(OMNIBOX_FOCUS_NONE, + OMNIBOX_FOCUS_CHANGE_EXPLICIT); + EXPECT_FALSE(MessageWasSent(ChromeViewMsg_SearchBoxFocusChanged::ID)); +} + +TEST_F(SearchIPCRouterTest, SendSetInputInProgress) { + NavigateAndCommitActiveTab(GURL(chrome::kChromeSearchLocalNtpUrl)); + SetupMockDelegateAndPolicy(); + MockSearchIPCRouterPolicy* policy = GetSearchIPCRouterPolicy(); + EXPECT_CALL(*policy, ShouldSendSetInputInProgress(true)).Times(1) + .WillOnce(testing::Return(true)); + + process()->sink().ClearMessages(); + GetSearchIPCRouter().SetInputInProgress(true); + EXPECT_TRUE(MessageWasSent(ChromeViewMsg_SearchBoxSetInputInProgress::ID)); +} + +TEST_F(SearchIPCRouterTest, DoNotSendSetInputInProgress) { + NavigateAndCommitActiveTab(GURL(chrome::kChromeSearchLocalNtpUrl)); + SetupMockDelegateAndPolicy(); + MockSearchIPCRouterPolicy* policy = GetSearchIPCRouterPolicy(); + EXPECT_CALL(*policy, ShouldSendSetInputInProgress(true)).Times(1) + .WillOnce(testing::Return(false)); + + process()->sink().ClearMessages(); + GetSearchIPCRouter().SetInputInProgress(true); + EXPECT_FALSE(MessageWasSent(ChromeViewMsg_SearchBoxSetInputInProgress::ID)); +} + TEST_F(SearchIPCRouterTest, SendMostVisitedItemsMsg) { NavigateAndCommitActiveTab(GURL(chrome::kChromeSearchLocalNtpUrl)); SetupMockDelegateAndPolicy(); diff --git a/chrome/browser/ui/search/search_tab_helper.cc b/chrome/browser/ui/search/search_tab_helper.cc index b6e6b30..ebfcf52 100644 --- a/chrome/browser/ui/search/search_tab_helper.cc +++ b/chrome/browser/ui/search/search_tab_helper.cc @@ -11,6 +11,7 @@ #include "base/strings/string16.h" #include "base/strings/string_util.h" #include "build/build_config.h" +#include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/history/most_visited_tiles_experiment.h" #include "chrome/browser/history/top_sites.h" #include "chrome/browser/profiles/profile.h" @@ -38,6 +39,8 @@ #include "content/public/browser/navigation_details.h" #include "content/public/browser/navigation_entry.h" #include "content/public/browser/navigation_type.h" +#include "content/public/browser/notification_service.h" +#include "content/public/browser/notification_source.h" #include "content/public/browser/render_process_host.h" #include "content/public/browser/user_metrics.h" #include "content/public/browser/web_contents.h" @@ -186,6 +189,23 @@ void SearchTabHelper::OmniboxInputStateChanged() { UpdateMode(false, false); } +void SearchTabHelper::OmniboxFocusChanged(OmniboxFocusState state, + OmniboxFocusChangeReason reason) { + content::NotificationService::current()->Notify( + chrome::NOTIFICATION_OMNIBOX_FOCUS_CHANGED, + content::Source<SearchTabHelper>(this), + content::NotificationService::NoDetails()); + + ipc_router_.OmniboxFocusChanged(state, reason); + + // Don't send oninputstart/oninputend updates in response to focus changes + // if there's a navigation in progress. This prevents Chrome from sending + // a spurious oninputend when the user accepts a match in the omnibox. + if (web_contents_->GetController().GetPendingEntry() == NULL) { + ipc_router_.SetInputInProgress(IsInputInProgress()); + } +} + void SearchTabHelper::NavigationEntryUpdated() { if (!is_search_enabled_) return; @@ -575,7 +595,11 @@ void SearchTabHelper::UpdateMode(bool update_origin, bool is_preloaded_ntp) { if (omnibox && omnibox->model()->user_input_in_progress()) type = SearchMode::MODE_SEARCH_SUGGESTIONS; + SearchMode old_mode(model_.mode()); model_.SetMode(SearchMode(type, origin)); + if (old_mode.is_ntp() != model_.mode().is_ntp()) { + ipc_router_.SetInputInProgress(IsInputInProgress()); + } } void SearchTabHelper::DetermineIfPageSupportsInstant() { @@ -607,3 +631,9 @@ void SearchTabHelper::RedirectToLocalNTP() { load_params.should_replace_current_entry = true; web_contents_->GetController().LoadURLWithParams(load_params); } + +bool SearchTabHelper::IsInputInProgress() const { + OmniboxView* omnibox = GetOmniboxView(web_contents()); + return !model_.mode().is_ntp() && omnibox && + omnibox->model()->focus_state() == OMNIBOX_FOCUS_VISIBLE; +} diff --git a/chrome/browser/ui/search/search_tab_helper.h b/chrome/browser/ui/search/search_tab_helper.h index 50f9a58..cc250d6 100644 --- a/chrome/browser/ui/search/search_tab_helper.h +++ b/chrome/browser/ui/search/search_tab_helper.h @@ -15,6 +15,7 @@ #include "chrome/browser/ui/search/search_model.h" #include "chrome/common/instant_types.h" #include "chrome/common/ntp_logging_events.h" +#include "chrome/common/omnibox_focus_state.h" #include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_user_data.h" #include "ui/base/window_open_disposition.h" @@ -55,6 +56,11 @@ class SearchTabHelper : public content::WebContentsObserver, // affect the search mode. void OmniboxInputStateChanged(); + // Called to indicate that the omnibox focus state changed with the given + // |reason|. + void OmniboxFocusChanged(OmniboxFocusState state, + OmniboxFocusChangeReason reason); + // Invoked when the active navigation entry is updated in some way that might // affect the search mode. This is used by Instant when it "fixes up" the // virtual URL of the active entry. Regular navigations are captured through @@ -199,6 +205,10 @@ class SearchTabHelper : public content::WebContentsObserver, // Instant URL and trim the history correctly. void RedirectToLocalNTP(); + // Returns whether input is in progress, i.e. if the omnibox has focus and the + // active tab is in mode SEARCH_SUGGESTIONS. + bool IsInputInProgress() const; + const bool is_search_enabled_; // Model object for UI that cares about search state. |