Commit 3fb0e316 authored by Mark Pearson's avatar Mark Pearson Committed by Commit Bot

Allow Zero Suggest (on Android) on chrome: and about: Pages

In effect, this fixes this TODO in zero_suggest_provider.cc
  // Only show zero suggest for HTTP[S] pages.
  // TODO(mariakhomenko): We may be able to expand this set to include pages
  // with other schemes (e.g. chrome://). That may require improvements to
  // the formatting of the verbatim result returned by MatchForCurrentURL().

As such, the main part of this change is in
components/omnibox/browser/zero_suggest_provider.cc
Everything else in this changelist is simply to get that change working.

I audited all the schemes in the URL constants files.
* Some schemes such as ftp are never used on Android; they're not
  supported.
* Some schemes such as chrome-native are used on Android, but they're
  never displayed in a context that has an omnibox.
The only ones I found I could get displayed in the omnibox were
about: and chrome:.

For identifying what schemes to investigate, the histogram
Navigation.MainFrameScheme was useful.

This changelist allows zero suggest to appear on those contexts.
Zero suggest still not appear on the new tab page or incognito,
both by product decision.  This change does not alter that.

To make this work, I had to make the AutocompleteClient function
GetEmbedderRepresentationOfAboutScheme() const.  Most of this
changelist is fixes declarations due to that.

Tested interactively.

TBR=rohitrao
for mechanical changes to ios/c/b/autocomplete

Bug: 704406
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I65be6327d29bd7616e44999304c85317406b67cc
Reviewed-on: https://chromium-review.googlesource.com/1172080
Commit-Queue: Mark Pearson <mpearson@chromium.org>
Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586429}
parent 7e23a87c
......@@ -336,6 +336,8 @@ public class LocationBarLayoutTest {
final UrlBar urlBar = getUrlBar();
final LocationBarLayout locationBar = getLocationBar();
mTestToolbarModel.setCurrentUrl(VERBOSE_URL);
mTestToolbarModel.setSecurityLevel(ConnectionSecurityLevel.SECURE);
mTestToolbarModel.mDisplayText = TRIMMED_URL;
mTestToolbarModel.mEditingText = VERBOSE_URL;
setUrlToPageUrl(locationBar);
......@@ -351,4 +353,4 @@ public class LocationBarLayoutTest {
Assert.assertEquals(VERBOSE_URL.length(), urlBar.getSelectionEnd());
});
}
}
\ No newline at end of file
}
......@@ -201,7 +201,8 @@ std::string ChromeAutocompleteProviderClient::GetAcceptLanguages() const {
}
std::string
ChromeAutocompleteProviderClient::GetEmbedderRepresentationOfAboutScheme() {
ChromeAutocompleteProviderClient::GetEmbedderRepresentationOfAboutScheme()
const {
return content::kChromeUIScheme;
}
......
......@@ -47,7 +47,7 @@ class ChromeAutocompleteProviderClient : public AutocompleteProviderClient {
std::unique_ptr<KeywordExtensionsDelegate> GetKeywordExtensionsDelegate(
KeywordProvider* keyword_provider) override;
std::string GetAcceptLanguages() const override;
std::string GetEmbedderRepresentationOfAboutScheme() override;
std::string GetEmbedderRepresentationOfAboutScheme() const override;
std::vector<base::string16> GetBuiltinURLs() override;
std::vector<base::string16> GetBuiltinsToProvideAsUserTypes() override;
// GetCurrentVisitTimestamp is only implemented for desktop users. For mobile
......
......@@ -77,7 +77,7 @@ class AutocompleteProviderClient {
// The embedder's representation of the |about| URL scheme for builtin URLs
// (e.g., |chrome| for Chrome).
virtual std::string GetEmbedderRepresentationOfAboutScheme() = 0;
virtual std::string GetEmbedderRepresentationOfAboutScheme() const = 0;
// The set of built-in URLs considered worth suggesting as autocomplete
// suggestions to the user. Some built-in URLs, e.g. hidden URLs that
......
......@@ -50,7 +50,7 @@ class FakeAutocompleteProviderClient : public MockAutocompleteProviderClient {
public:
FakeAutocompleteProviderClient() {}
std::string GetEmbedderRepresentationOfAboutScheme() override {
std::string GetEmbedderRepresentationOfAboutScheme() const override {
return kEmbedderAboutScheme;
}
......
......@@ -77,7 +77,7 @@ class MockAutocompleteProviderClient
}
MOCK_CONST_METHOD0(GetAcceptLanguages, std::string());
MOCK_METHOD0(GetEmbedderRepresentationOfAboutScheme, std::string());
MOCK_CONST_METHOD0(GetEmbedderRepresentationOfAboutScheme, std::string());
MOCK_METHOD0(GetBuiltinURLs, std::vector<base::string16>());
MOCK_METHOD0(GetBuiltinsToProvideAsUserTypes, std::vector<base::string16>());
MOCK_CONST_METHOD0(GetCurrentVisitTimestamp, base::Time());
......
......@@ -473,10 +473,7 @@ void ZeroSuggestProvider::ConvertResultsToAutocompleteMatches() {
if (num_results == 0)
return;
// TODO(jered): Rip this out once the first match is decoupled from the
// current typing in the omnibox.
matches_.push_back(current_url_match_);
for (MatchMap::const_iterator it(map.begin()); it != map.end(); ++it)
matches_.push_back(it->second);
......@@ -516,13 +513,19 @@ bool ZeroSuggestProvider::AllowZeroSuggestSuggestions(
if (client()->IsOffTheRecord())
return false;
// Only show zero suggest for HTTP[S] pages.
// TODO(mariakhomenko): We may be able to expand this set to include pages
// with other schemes (e.g. chrome://). That may require improvements to
// the formatting of the verbatim result returned by MatchForCurrentURL().
// Only show zero suggest for pages with URLs the user will recognize.
// This list intentionally does not include items such as ftp: and file:
// because (a) these do not work on Android and iOS, where non-contextual
// zero suggest is launched and (b) on desktop, where contextual zero suggest
// is running, these types of schemes aren't eligible to be sent to the
// server to ask for suggestions (and thus in practice we won't display zero
// suggest for them).
if (!current_page_url.is_valid() ||
((current_page_url.scheme() != url::kHttpScheme) &&
(current_page_url.scheme() != url::kHttpsScheme)))
(current_page_url.scheme() != url::kHttpsScheme) &&
(current_page_url.scheme() != url::kAboutScheme) &&
(current_page_url.scheme() !=
client()->GetEmbedderRepresentationOfAboutScheme())))
return false;
return true;
......
......@@ -132,7 +132,7 @@ std::string AutocompleteProviderClientImpl::GetAcceptLanguages() const {
}
std::string
AutocompleteProviderClientImpl::GetEmbedderRepresentationOfAboutScheme() {
AutocompleteProviderClientImpl::GetEmbedderRepresentationOfAboutScheme() const {
return kChromeUIScheme;
}
......
......@@ -48,7 +48,7 @@ class AutocompleteProviderClientImpl : public AutocompleteProviderClient {
std::unique_ptr<KeywordExtensionsDelegate> GetKeywordExtensionsDelegate(
KeywordProvider* keyword_provider) override;
std::string GetAcceptLanguages() const override;
std::string GetEmbedderRepresentationOfAboutScheme() override;
std::string GetEmbedderRepresentationOfAboutScheme() const override;
std::vector<base::string16> GetBuiltinURLs() override;
std::vector<base::string16> GetBuiltinsToProvideAsUserTypes() override;
// GetCurrentVisitTimestamp is only used by the contextual zero suggest
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment