[omnibox] Move page classification logic to LocationBarModel
This moves OmniboxEditModel::ClassifyPage to LocationBarModel to make
it accessible to the Android codebase.
This CL also fixes the Query in Omnibox behavior.
This logic is duplicated in AutocompleteControllerAndroid, which does
not handle the Query in Omnibox case. In a followup CL, we will
redirect the Android code to use the shared implementation within
LocationBarModel.
Mechanically speaking, this moves ClassifyPage from OmniboxEditModel to
LocationBarModel. It also moves three supporting methods from
OmniboxClient to LocationBarModelDelegate.
This will enable Android to use that code, since Android has no
concept of either OmniboxEditModel or OmniboxClient, but does use
LocationBarModel extensively.
Bug: 963680, 912722
Change-Id: I1f607ff720282a8e405caaf86bf3e11e5ec43e06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1622637
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662673}
diff --git a/chrome/browser/ui/omnibox/chrome_omnibox_client.cc b/chrome/browser/ui/omnibox/chrome_omnibox_client.cc
index 1d84a0df..1dfa25e 100644
--- a/chrome/browser/ui/omnibox/chrome_omnibox_client.cc
+++ b/chrome/browser/ui/omnibox/chrome_omnibox_client.cc
@@ -176,17 +176,6 @@
->GetFavicon();
}
-bool ChromeOmniboxClient::IsInstantNTP() const {
- return search::IsInstantNTP(controller_->GetWebContents());
-}
-
-bool ChromeOmniboxClient::IsSearchResultsPage() const {
- Profile* profile = Profile::FromBrowserContext(
- controller_->GetWebContents()->GetBrowserContext());
- return TemplateURLServiceFactory::GetForProfile(profile)->
- IsSearchResultsPageFromDefaultSearchProvider(GetURL());
-}
-
bool ChromeOmniboxClient::IsLoading() const {
return controller_->GetWebContents()->IsLoading();
}
@@ -195,14 +184,6 @@
return controller_->command_updater()->IsCommandEnabled(IDC_OPEN_CURRENT_URL);
}
-bool ChromeOmniboxClient::IsNewTabPage(const GURL& url) const {
- return url.spec() == chrome::kChromeUINewTabURL;
-}
-
-bool ChromeOmniboxClient::IsHomePage(const GURL& url) const {
- return url.spec() == profile_->GetPrefs()->GetString(prefs::kHomePage);
-}
-
bool ChromeOmniboxClient::IsDefaultSearchProviderEnabled() const {
const base::DictionaryValue* url_dict = profile_->GetPrefs()->GetDictionary(
DefaultSearchManager::kDefaultSearchProviderDataPrefName);
diff --git a/chrome/browser/ui/omnibox/chrome_omnibox_client.h b/chrome/browser/ui/omnibox/chrome_omnibox_client.h
index 2cf242c..20ea2b3 100644
--- a/chrome/browser/ui/omnibox/chrome_omnibox_client.h
+++ b/chrome/browser/ui/omnibox/chrome_omnibox_client.h
@@ -37,12 +37,8 @@
const GURL& GetURL() const override;
const base::string16& GetTitle() const override;
gfx::Image GetFavicon() const override;
- bool IsInstantNTP() const override;
- bool IsSearchResultsPage() const override;
bool IsLoading() const override;
bool IsPasteAndGoEnabled() const override;
- bool IsNewTabPage(const GURL& url) const override;
- bool IsHomePage(const GURL& url) const override;
bool IsDefaultSearchProviderEnabled() const override;
const SessionID& GetSessionID() const override;
bookmarks::BookmarkModel* GetBookmarkModel() override;
diff --git a/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.cc b/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.cc
index da2df27..f8b3ad7 100644
--- a/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.cc
+++ b/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.cc
@@ -177,6 +177,22 @@
#endif
}
+bool ChromeLocationBarModelDelegate::IsInstantNTP() const {
+ return search::IsInstantNTP(GetActiveWebContents());
+}
+
+bool ChromeLocationBarModelDelegate::IsNewTabPage(const GURL& url) const {
+ return url.spec() == chrome::kChromeUINewTabURL;
+}
+
+bool ChromeLocationBarModelDelegate::IsHomePage(const GURL& url) const {
+ Profile* const profile = GetProfile();
+ if (!profile)
+ return false;
+
+ return url.spec() == profile->GetPrefs()->GetString(prefs::kHomePage);
+}
+
content::NavigationController*
ChromeLocationBarModelDelegate::GetNavigationController() const {
// This |current_tab| can be null during the initialization of the toolbar
diff --git a/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.h b/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.h
index cbe5562..3766482 100644
--- a/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.h
+++ b/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.h
@@ -47,6 +47,9 @@
scoped_refptr<net::X509Certificate> GetCertificate() const override;
const gfx::VectorIcon* GetVectorIconOverride() const override;
bool IsOfflinePage() const override;
+ bool IsInstantNTP() const override;
+ bool IsNewTabPage(const GURL& url) const override;
+ bool IsHomePage(const GURL& url) const override;
AutocompleteClassifier* GetAutocompleteClassifier() override;
TemplateURLService* GetTemplateURLService() override;
diff --git a/components/omnibox/browser/location_bar_model.h b/components/omnibox/browser/location_bar_model.h
index 544026f..63f4cb8 100644
--- a/components/omnibox/browser/location_bar_model.h
+++ b/components/omnibox/browser/location_bar_model.h
@@ -11,7 +11,9 @@
#include "base/macros.h"
#include "base/strings/string16.h"
+#include "components/omnibox/common/omnibox_focus_state.h"
#include "components/security_state/core/security_state.h"
+#include "third_party/metrics_proto/omnibox_event.pb.h"
#include "url/gurl.h"
namespace gfx {
@@ -52,6 +54,13 @@
// to check the display status only. Virtual for testing purposes.
virtual bool GetDisplaySearchTerms(base::string16* search_terms) = 0;
+ // Classify the current page being viewed as, for example, the new tab
+ // page or a normal web page. Used for logging omnibox events for
+ // UMA opted-in users. Examines the user's profile to determine if the
+ // current page is the user's home page.
+ virtual metrics::OmniboxEventProto::PageClassification GetPageClassification(
+ OmniboxFocusSource focus_source) = 0;
+
// Returns the id of the icon to show to the left of the address, based on the
// current URL. When search term replacement is active, this returns a search
// icon.
diff --git a/components/omnibox/browser/location_bar_model_delegate.cc b/components/omnibox/browser/location_bar_model_delegate.cc
index 1ed20e6..3c00294 100644
--- a/components/omnibox/browser/location_bar_model_delegate.cc
+++ b/components/omnibox/browser/location_bar_model_delegate.cc
@@ -35,6 +35,18 @@
return false;
}
+bool LocationBarModelDelegate::IsInstantNTP() const {
+ return false;
+}
+
+bool LocationBarModelDelegate::IsNewTabPage(const GURL& url) const {
+ return false;
+}
+
+bool LocationBarModelDelegate::IsHomePage(const GURL& url) const {
+ return false;
+}
+
AutocompleteClassifier* LocationBarModelDelegate::GetAutocompleteClassifier() {
return nullptr;
}
diff --git a/components/omnibox/browser/location_bar_model_delegate.h b/components/omnibox/browser/location_bar_model_delegate.h
index fc5f6085..ccf5f0e0 100644
--- a/components/omnibox/browser/location_bar_model_delegate.h
+++ b/components/omnibox/browser/location_bar_model_delegate.h
@@ -67,6 +67,15 @@
// previously-downloaded content.
virtual bool IsOfflinePage() const;
+ // Returns true if the current page is a New Tab Page rendered by Instant.
+ virtual bool IsInstantNTP() const;
+
+ // Returns whether |url| corresponds to the new tab page.
+ virtual bool IsNewTabPage(const GURL& url) const;
+
+ // Returns whether |url| corresponds to the user's home page.
+ virtual bool IsHomePage(const GURL& url) const;
+
// Returns the AutocompleteClassifier instance for the current page.
virtual AutocompleteClassifier* GetAutocompleteClassifier();
diff --git a/components/omnibox/browser/location_bar_model_impl.cc b/components/omnibox/browser/location_bar_model_impl.cc
index 3266011..f4dd617 100644
--- a/components/omnibox/browser/location_bar_model_impl.cc
+++ b/components/omnibox/browser/location_bar_model_impl.cc
@@ -32,6 +32,8 @@
#include "components/vector_icons/vector_icons.h" // nogncheck
#endif
+using metrics::OmniboxEventProto;
+
LocationBarModelImpl::LocationBarModelImpl(LocationBarModelDelegate* delegate,
size_t max_url_display_chars)
: delegate_(delegate), max_url_display_chars_(max_url_display_chars) {
@@ -130,6 +132,46 @@
return true;
}
+OmniboxEventProto::PageClassification
+LocationBarModelImpl::GetPageClassification(OmniboxFocusSource focus_source) {
+ // We may be unable to fetch the current URL during startup or shutdown when
+ // the omnibox exists but there is no attached page.
+ GURL gurl;
+ if (!delegate_->GetURL(&gurl))
+ return OmniboxEventProto::OTHER;
+
+ if (focus_source == OmniboxFocusSource::SEARCH_BUTTON)
+ return OmniboxEventProto::SEARCH_BUTTON_AS_STARTING_FOCUS;
+ if (delegate_->IsInstantNTP()) {
+ // Note that we treat OMNIBOX as the source if focus_source_ is INVALID,
+ // i.e., if input isn't actually in progress.
+ return (focus_source == OmniboxFocusSource::FAKEBOX)
+ ? OmniboxEventProto::INSTANT_NTP_WITH_FAKEBOX_AS_STARTING_FOCUS
+ : OmniboxEventProto::INSTANT_NTP_WITH_OMNIBOX_AS_STARTING_FOCUS;
+ }
+ if (!gurl.is_valid())
+ return OmniboxEventProto::INVALID_SPEC;
+ if (delegate_->IsNewTabPage(gurl))
+ return OmniboxEventProto::NTP;
+ if (gurl.spec() == url::kAboutBlankURL)
+ return OmniboxEventProto::BLANK;
+ if (delegate_->IsHomePage(gurl))
+ return OmniboxEventProto::HOME_PAGE;
+
+ TemplateURLService* template_url_service = delegate_->GetTemplateURLService();
+ if (template_url_service &&
+ template_url_service->IsSearchResultsPageFromDefaultSearchProvider(
+ gurl)) {
+ return GetDisplaySearchTerms(nullptr)
+ ? OmniboxEventProto::
+ SEARCH_RESULT_PAGE_DOING_SEARCH_TERM_REPLACEMENT
+ : OmniboxEventProto::
+ SEARCH_RESULT_PAGE_NO_SEARCH_TERM_REPLACEMENT;
+ }
+
+ return OmniboxEventProto::OTHER;
+}
+
const gfx::VectorIcon& LocationBarModelImpl::GetVectorIcon() const {
#if (!defined(OS_ANDROID) || BUILDFLAG(ENABLE_VR)) && !defined(OS_IOS)
auto* const icon_override = delegate_->GetVectorIconOverride();
diff --git a/components/omnibox/browser/location_bar_model_impl.h b/components/omnibox/browser/location_bar_model_impl.h
index a5c6d24..becbe1b 100644
--- a/components/omnibox/browser/location_bar_model_impl.h
+++ b/components/omnibox/browser/location_bar_model_impl.h
@@ -33,6 +33,8 @@
GURL GetURL() const override;
security_state::SecurityLevel GetSecurityLevel() const override;
bool GetDisplaySearchTerms(base::string16* search_terms) override;
+ metrics::OmniboxEventProto::PageClassification GetPageClassification(
+ OmniboxFocusSource focus_source) override;
const gfx::VectorIcon& GetVectorIcon() const override;
base::string16 GetSecureDisplayText() const override;
base::string16 GetSecureAccessibilityText() const override;
diff --git a/components/omnibox/browser/location_bar_model_impl_unittest.cc b/components/omnibox/browser/location_bar_model_impl_unittest.cc
index 3c0ab686..983cf5e 100644
--- a/components/omnibox/browser/location_bar_model_impl_unittest.cc
+++ b/components/omnibox/browser/location_bar_model_impl_unittest.cc
@@ -54,6 +54,12 @@
return state;
}
+ bool IsInstantNTP() const override { return false; }
+
+ bool IsNewTabPage(const GURL& url) const override { return false; }
+
+ bool IsHomePage(const GURL& url) const override { return false; }
+
AutocompleteClassifier* GetAutocompleteClassifier() override {
return omnibox_client_.GetAutocompleteClassifier();
}
diff --git a/components/omnibox/browser/omnibox_client.cc b/components/omnibox/browser/omnibox_client.cc
index b46cf761..a98f043 100644
--- a/components/omnibox/browser/omnibox_client.cc
+++ b/components/omnibox/browser/omnibox_client.cc
@@ -33,14 +33,6 @@
return gfx::Image();
}
-bool OmniboxClient::IsInstantNTP() const {
- return false;
-}
-
-bool OmniboxClient::IsSearchResultsPage() const {
- return false;
-}
-
bool OmniboxClient::IsLoading() const {
return false;
}
@@ -49,14 +41,6 @@
return false;
}
-bool OmniboxClient::IsNewTabPage(const GURL& url) const {
- return false;
-}
-
-bool OmniboxClient::IsHomePage(const GURL& url) const {
- return false;
-}
-
bool OmniboxClient::IsDefaultSearchProviderEnabled() const {
return true;
}
diff --git a/components/omnibox/browser/omnibox_client.h b/components/omnibox/browser/omnibox_client.h
index 703f3d2..19029a0 100644
--- a/components/omnibox/browser/omnibox_client.h
+++ b/components/omnibox/browser/omnibox_client.h
@@ -67,24 +67,12 @@
// Returns the favicon of the current page.
virtual gfx::Image GetFavicon() const;
- // Returns true if the current page is a New Tab Page rendered by Instant.
- virtual bool IsInstantNTP() const;
-
- // Returns true if the committed entry is a search results page.
- virtual bool IsSearchResultsPage() const;
-
// Returns whether the current page is loading.
virtual bool IsLoading() const;
// Returns whether paste-and-go functionality is enabled.
virtual bool IsPasteAndGoEnabled() const;
- // Returns whether |url| corresponds to the new tab page.
- virtual bool IsNewTabPage(const GURL& url) const;
-
- // Returns whether |url| corresponds to the user's home page.
- virtual bool IsHomePage(const GURL& url) const;
-
// Returns false if Default Search is disabled by a policy.
virtual bool IsDefaultSearchProviderEnabled() const;
diff --git a/components/omnibox/browser/omnibox_edit_model.cc b/components/omnibox/browser/omnibox_edit_model.cc
index aee88ac..c6fc55f 100644
--- a/components/omnibox/browser/omnibox_edit_model.cc
+++ b/components/omnibox/browser/omnibox_edit_model.cc
@@ -108,7 +108,7 @@
bool is_keyword_hint,
OmniboxEventProto::KeywordModeEntryMethod keyword_mode_entry_method,
OmniboxFocusState focus_state,
- FocusSource focus_source,
+ OmniboxFocusSource focus_source,
const AutocompleteInput& autocomplete_input)
: user_input_in_progress(user_input_in_progress),
user_text(user_text),
@@ -134,7 +134,6 @@
view_(view),
controller_(controller),
focus_state_(OMNIBOX_FOCUS_NONE),
- focus_source_(INVALID),
user_input_in_progress_(false),
user_input_since_focus_(true),
just_deleted_text_(false),
@@ -151,6 +150,12 @@
OmniboxEditModel::~OmniboxEditModel() {
}
+metrics::OmniboxEventProto::PageClassification
+OmniboxEditModel::GetPageClassification() const {
+ return controller()->GetLocationBarModel()->GetPageClassification(
+ focus_source_);
+}
+
const OmniboxEditModel::State OmniboxEditModel::GetStateForTabSwitch() {
// Like typing, switching tabs "accepts" the temporary text as the user
// text, because it makes little sense to have temporary text when the
@@ -387,7 +392,7 @@
// screw up our calculation of the desired_tld.
AutocompleteMatch match_from_text;
client_->GetAutocompleteClassifier()->Classify(*text, is_keyword_selected(),
- true, ClassifyPage(),
+ true, GetPageClassification(),
&match_from_text, nullptr);
if (AutocompleteMatch::IsSearchType(match_from_text.type))
return;
@@ -519,8 +524,9 @@
cursor_position = input_text.length();
}
}
- input_ = AutocompleteInput(input_text, cursor_position, ClassifyPage(),
- client_->GetSchemeClassifier());
+ input_ =
+ AutocompleteInput(input_text, cursor_position, GetPageClassification(),
+ client_->GetSchemeClassifier());
input_.set_current_url(client_->GetURL());
input_.set_current_title(client_->GetTitle());
input_.set_prevent_inline_autocomplete(
@@ -703,7 +709,7 @@
input_text = user_input_in_progress_ ? user_text_ : url_for_editing_;
// Create a dummy AutocompleteInput for use in calling SuggestExactInput()
// to create an alternate navigational match.
- AutocompleteInput alternate_input(input_text, ClassifyPage(),
+ AutocompleteInput alternate_input(input_text, GetPageClassification(),
client_->GetSchemeClassifier());
// Somehow we can occasionally get here with no active tab. It's not
// clear why this happens.
@@ -746,7 +752,7 @@
keyword_mode_entry_method_, popup_open, dropdown_ignored ? 0 : index,
disposition, !pasted_text.empty(),
SessionID::InvalidValue(), // don't know tab ID; set later if appropriate
- ClassifyPage(), elapsed_time_since_user_first_modified_omnibox,
+ GetPageClassification(), elapsed_time_since_user_first_modified_omnibox,
match.allowed_to_be_default_match ? match.inline_autocompletion.length()
: base::string16::npos,
elapsed_time_since_last_change_to_default_match,
@@ -1037,7 +1043,7 @@
// off).
// TODO(hfung): Remove this when crbug/271590 is fixed.
if (client_->CurrentPageExists() && !user_input_in_progress_) {
- input_ = AutocompleteInput(url_for_editing_, ClassifyPage(),
+ input_ = AutocompleteInput(url_for_editing_, GetPageClassification(),
client_->GetSchemeClassifier());
input_.set_current_url(client_->GetURL());
input_.set_current_title(client_->GetTitle());
@@ -1069,7 +1075,7 @@
void OmniboxEditModel::OnKillFocus() {
SetFocusState(OMNIBOX_FOCUS_NONE, OMNIBOX_FOCUS_CHANGE_EXPLICIT);
- focus_source_ = INVALID;
+ focus_source_ = OmniboxFocusSource::INVALID;
last_omnibox_focus_ = base::TimeTicks();
paste_state_ = NONE;
control_key_state_ = UP;
@@ -1280,15 +1286,16 @@
if (state_changes.text_differs || state_changes.selection_differs) {
// Record current focus state for this input if we haven't already.
- if (focus_source_ == INVALID) {
+ if (focus_source_ == OmniboxFocusSource::INVALID) {
// We should generally expect the omnibox to have focus at this point, but
// it doesn't always on Linux. This is because, unlike other platforms,
// right clicking in the omnibox on Linux doesn't focus it. So pasting via
// right-click can change the contents without focusing the omnibox.
// TODO(samarth): fix Linux focus behavior and add a DCHECK here to
// check that the omnibox does have focus.
- focus_source_ =
- (focus_state_ == OMNIBOX_FOCUS_INVISIBLE) ? FAKEBOX : OMNIBOX;
+ focus_source_ = (focus_state_ == OMNIBOX_FOCUS_INVISIBLE)
+ ? OmniboxFocusSource::FAKEBOX
+ : OmniboxFocusSource::OMNIBOX;
}
// Restore caret visibility whenever the user changes text or selection in
@@ -1479,7 +1486,7 @@
client_->GetAutocompleteClassifier()->Classify(
MaybePrependKeyword(text_for_match_generation), is_keyword_selected(),
- true, ClassifyPage(), match, alternate_nav_url);
+ true, GetPageClassification(), match, alternate_nav_url);
}
}
@@ -1546,38 +1553,12 @@
}
}
-OmniboxEventProto::PageClassification OmniboxEditModel::ClassifyPage() const {
- if (!client_->CurrentPageExists())
- return OmniboxEventProto::OTHER;
- if (focus_source_ == SEARCH_BUTTON)
- return OmniboxEventProto::SEARCH_BUTTON_AS_STARTING_FOCUS;
- if (client_->IsInstantNTP()) {
- // Note that we treat OMNIBOX as the source if focus_source_ is INVALID,
- // i.e., if input isn't actually in progress.
- return (focus_source_ == FAKEBOX) ?
- OmniboxEventProto::INSTANT_NTP_WITH_FAKEBOX_AS_STARTING_FOCUS :
- OmniboxEventProto::INSTANT_NTP_WITH_OMNIBOX_AS_STARTING_FOCUS;
- }
- const GURL& gurl = client_->GetURL();
- if (!gurl.is_valid())
- return OmniboxEventProto::INVALID_SPEC;
- if (client_->IsNewTabPage(gurl))
- return OmniboxEventProto::NTP;
- if (gurl.spec() == url::kAboutBlankURL)
- return OmniboxEventProto::BLANK;
- if (client_->IsHomePage(gurl))
- return OmniboxEventProto::HOME_PAGE;
- if (client_->IsSearchResultsPage())
- return OmniboxEventProto::SEARCH_RESULT_PAGE_NO_SEARCH_TERM_REPLACEMENT;
- return OmniboxEventProto::OTHER;
-}
-
void OmniboxEditModel::ClassifyString(const base::string16& text,
AutocompleteMatch* match,
GURL* alternate_nav_url) const {
DCHECK(match);
client_->GetAutocompleteClassifier()->Classify(
- text, false, false, ClassifyPage(), match, alternate_nav_url);
+ text, false, false, GetPageClassification(), match, alternate_nav_url);
}
bool OmniboxEditModel::SetInputInProgressNoNotify(bool in_progress) {
diff --git a/components/omnibox/browser/omnibox_edit_model.h b/components/omnibox/browser/omnibox_edit_model.h
index a3928ab..8bff793b 100644
--- a/components/omnibox/browser/omnibox_edit_model.h
+++ b/components/omnibox/browser/omnibox_edit_model.h
@@ -37,15 +37,6 @@
class OmniboxEditModel {
public:
- // Did the Omnibox focus originate via the user clicking on the Omnibox, on
- // the Fakebox or the Search button?
- enum FocusSource {
- INVALID = 0,
- OMNIBOX = 1,
- FAKEBOX = 2,
- SEARCH_BUTTON = 3
- };
-
struct State {
State(bool user_input_in_progress,
const base::string16& user_text,
@@ -54,7 +45,7 @@
metrics::OmniboxEventProto::KeywordModeEntryMethod
keyword_mode_entry_method,
OmniboxFocusState focus_state,
- FocusSource focus_source,
+ OmniboxFocusSource focus_source,
const AutocompleteInput& autocomplete_input);
State(const State& other);
~State();
@@ -66,7 +57,7 @@
metrics::OmniboxEventProto::KeywordModeEntryMethod
keyword_mode_entry_method;
OmniboxFocusState focus_state;
- FocusSource focus_source;
+ OmniboxFocusSource focus_source;
const AutocompleteInput autocomplete_input;
private:
DISALLOW_ASSIGN(State);
@@ -98,6 +89,8 @@
OmniboxClient* client() const { return client_.get(); }
+ metrics::OmniboxEventProto::PageClassification GetPageClassification() const;
+
// Returns the current state. This assumes we are switching tabs, and changes
// the internal state appropriately.
const State GetStateForTabSwitch();
@@ -240,8 +233,8 @@
return focus_state_ == OMNIBOX_FOCUS_VISIBLE;
}
- FocusSource focus_source() const { return focus_source_; }
- void set_focus_source(FocusSource focus_source) {
+ OmniboxFocusSource focus_source() const { return focus_source_; }
+ void set_focus_source(OmniboxFocusSource focus_source) {
focus_source_ = focus_source;
}
@@ -451,12 +444,6 @@
// keyword.
static bool IsSpaceCharForAcceptingKeyword(wchar_t c);
- // Classify the current page being viewed as, for example, the new tab
- // page or a normal web page. Used for logging omnibox events for
- // UMA opted-in users. Examines the user's profile to determine if the
- // current page is the user's home page.
- metrics::OmniboxEventProto::PageClassification ClassifyPage() const;
-
// Sets |match| and |alternate_nav_url| based on classifying |text|.
// |alternate_nav_url| may be NULL.
void ClassifyString(const base::string16& text,
@@ -491,7 +478,7 @@
// Used to keep track whether the input currently in progress originated by
// focusing in the Omnibox, Fakebox or Search button. This will be INVALID if
// no input is in progress or the Omnibox is not focused.
- FocusSource focus_source_;
+ OmniboxFocusSource focus_source_ = OmniboxFocusSource::INVALID;
// Display-only text representing the current page. This could be any of:
// - The same as |url_for_editing_| if Steady State Elisions is OFF.
diff --git a/components/omnibox/browser/test_location_bar_model.cc b/components/omnibox/browser/test_location_bar_model.cc
index 34d09c60..26cd3f3d 100644
--- a/components/omnibox/browser/test_location_bar_model.cc
+++ b/components/omnibox/browser/test_location_bar_model.cc
@@ -52,6 +52,11 @@
return true;
}
+metrics::OmniboxEventProto::PageClassification
+TestLocationBarModel::GetPageClassification(OmniboxFocusSource focus_source) {
+ return metrics::OmniboxEventProto::OTHER;
+}
+
const gfx::VectorIcon& TestLocationBarModel::GetVectorIcon() const {
return *icon_;
}
diff --git a/components/omnibox/browser/test_location_bar_model.h b/components/omnibox/browser/test_location_bar_model.h
index bb3369b1..1a75a22 100644
--- a/components/omnibox/browser/test_location_bar_model.h
+++ b/components/omnibox/browser/test_location_bar_model.h
@@ -29,6 +29,8 @@
GURL GetURL() const override;
security_state::SecurityLevel GetSecurityLevel() const override;
bool GetDisplaySearchTerms(base::string16* search_terms) override;
+ metrics::OmniboxEventProto::PageClassification GetPageClassification(
+ OmniboxFocusSource focus_source) override;
const gfx::VectorIcon& GetVectorIcon() const override;
base::string16 GetSecureDisplayText() const override;
base::string16 GetSecureAccessibilityText() const override;
diff --git a/components/omnibox/common/omnibox_focus_state.h b/components/omnibox/common/omnibox_focus_state.h
index 8839d64..7f50f2d 100644
--- a/components/omnibox/common/omnibox_focus_state.h
+++ b/components/omnibox/common/omnibox_focus_state.h
@@ -39,4 +39,20 @@
OMNIBOX_FOCUS_CHANGE_REASON_LAST = OMNIBOX_FOCUS_CHANGE_TYPING
};
+// Did the Omnibox focus originate via the user clicking on the Omnibox, on
+// the Fakebox or the Search button?
+enum class OmniboxFocusSource {
+ // Invalid sentinel value.
+ INVALID = 0,
+
+ // Focus originated from user clicking on the omnibox.
+ OMNIBOX = 1,
+
+ // Focus originated from user clicking on the fakebox.
+ FAKEBOX = 2,
+
+ // Focus originated from user clicking on the search button.
+ SEARCH_BUTTON = 3,
+};
+
#endif // COMPONENTS_OMNIBOX_COMMON_OMNIBOX_FOCUS_STATE_H_
diff --git a/ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.h b/ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.h
index 63d484a..b15dbd8 100644
--- a/ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.h
+++ b/ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.h
@@ -35,6 +35,9 @@
scoped_refptr<net::X509Certificate> GetCertificate() const override;
const gfx::VectorIcon* GetVectorIconOverride() const override;
bool IsOfflinePage() const override;
+ bool IsInstantNTP() const override;
+ bool IsNewTabPage(const GURL& url) const override;
+ bool IsHomePage(const GURL& url) const override;
private:
// Helper method to extract the NavigationItem from which the states are
diff --git a/ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.mm b/ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.mm
index f481a23..3041c40 100644
--- a/ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.mm
+++ b/ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.mm
@@ -128,3 +128,25 @@
const GURL& url = visibleItem->GetURL();
return url.SchemeIs(kChromeUIScheme) && url.host() == kChromeUIOfflineHost;
}
+
+bool LocationBarModelDelegateIOS::IsInstantNTP() const {
+ // This is currently only called by the OmniboxEditModel to determine if the
+ // Google landing page is showing.
+ //
+ // TODO(crbug.com/315563)(lliabraa): This should also check the user's default
+ // search engine because if they're not using Google the Google landing page
+ // is not shown.
+ GURL currentURL;
+ if (!GetURL(¤tURL))
+ return false;
+ return currentURL == kChromeUINewTabURL;
+}
+
+bool LocationBarModelDelegateIOS::IsNewTabPage(const GURL& url) const {
+ return url.spec() == kChromeUINewTabURL;
+}
+
+bool LocationBarModelDelegateIOS::IsHomePage(const GURL& url) const {
+ // iOS does not have a notion of home page.
+ return false;
+}
diff --git a/ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.h b/ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.h
index 53c695f..d6295b5 100644
--- a/ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.h
+++ b/ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.h
@@ -34,10 +34,6 @@
const GURL& GetURL() const override;
bool IsLoading() const override;
bool IsPasteAndGoEnabled() const override;
- bool IsInstantNTP() const override;
- bool IsSearchResultsPage() const override;
- bool IsNewTabPage(const GURL& url) const override;
- bool IsHomePage(const GURL& url) const override;
bool IsDefaultSearchProviderEnabled() const override;
const SessionID& GetSessionID() const override;
bookmarks::BookmarkModel* GetBookmarkModel() override;
diff --git a/ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.mm b/ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.mm
index b9947ff..7eeafc5 100644
--- a/ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.mm
+++ b/ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.mm
@@ -71,33 +71,6 @@
return false;
}
-bool ChromeOmniboxClientIOS::IsInstantNTP() const {
- // This is currently only called by the OmniboxEditModel to determine if the
- // Google landing page is showing.
- // TODO(lliabraa): This should also check the user's default search engine
- // because if they're not using Google the Google landing page is not shown
- // (crbug/315563).
- GURL currentURL = controller_->GetWebState()->GetVisibleURL();
- return currentURL == kChromeUINewTabURL;
-}
-
-bool ChromeOmniboxClientIOS::IsSearchResultsPage() const {
- ios::ChromeBrowserState* browser_state =
- ios::ChromeBrowserState::FromBrowserState(
- controller_->GetWebState()->GetBrowserState());
- return ios::TemplateURLServiceFactory::GetForBrowserState(browser_state)
- ->IsSearchResultsPageFromDefaultSearchProvider(GetURL());
-}
-
-bool ChromeOmniboxClientIOS::IsNewTabPage(const GURL& url) const {
- return url.spec() == kChromeUINewTabURL;
-}
-
-bool ChromeOmniboxClientIOS::IsHomePage(const GURL& url) const {
- // iOS does not have a notion of home page.
- return false;
-}
-
bool ChromeOmniboxClientIOS::IsDefaultSearchProviderEnabled() const {
// iOS does not have Enterprise policies
return true;
diff --git a/ios/chrome/browser/ui/omnibox/omnibox_coordinator.mm b/ios/chrome/browser/ui/omnibox/omnibox_coordinator.mm
index ce4893b..1dfcdbf 100644
--- a/ios/chrome/browser/ui/omnibox/omnibox_coordinator.mm
+++ b/ios/chrome/browser/ui/omnibox/omnibox_coordinator.mm
@@ -9,6 +9,7 @@
#include "base/metrics/user_metrics_action.h"
#include "base/strings/sys_string_conversions.h"
#include "components/omnibox/browser/omnibox_edit_model.h"
+#include "components/omnibox/common/omnibox_focus_state.h"
#include "components/strings/grit/components_strings.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/favicon/ios_chrome_favicon_loader_factory.h"
@@ -110,7 +111,7 @@
- (void)setNextFocusSourceAsSearchButton {
OmniboxEditModel* model = _editView->model();
- model->set_focus_source(OmniboxEditModel::FocusSource::SEARCH_BUTTON);
+ model->set_focus_source(OmniboxFocusSource::SEARCH_BUTTON);
}
- (BOOL)isOmniboxFirstResponder {
diff --git a/ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm b/ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm
index 1123d83..f3399ad 100644
--- a/ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm
+++ b/ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm
@@ -20,6 +20,7 @@
#include "components/omnibox/browser/location_bar_model.h"
#include "components/omnibox/browser/omnibox_edit_model.h"
#include "components/omnibox/browser/omnibox_popup_model.h"
+#include "components/omnibox/common/omnibox_focus_state.h"
#include "ios/chrome/browser/autocomplete/autocomplete_scheme_classifier_impl.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.h"
@@ -388,10 +389,9 @@
// or from the secondary toolbar search button, the focus source is already
// set to FAKEBOX or SEARCH_BUTTON respectively. Otherwise, set it to
// OMNIBOX.
- if (model()->focus_source() != OmniboxEditModel::FocusSource::FAKEBOX &&
- model()->focus_source() !=
- OmniboxEditModel::FocusSource::SEARCH_BUTTON) {
- model()->set_focus_source(OmniboxEditModel::FocusSource::OMNIBOX);
+ if (model()->focus_source() != OmniboxFocusSource::FAKEBOX &&
+ model()->focus_source() != OmniboxFocusSource::SEARCH_BUTTON) {
+ model()->set_focus_source(OmniboxFocusSource::OMNIBOX);
}
model()->OnSetFocus(false);
