New issue
Advanced search Search tips

Issue 704406 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocked on:
issue 737581



Sign in to add a comment

Non-Contextual Zero Suggest Only Appears on HTTP[S] Pages

Project Member Reported by mpear...@chromium.org, Mar 23 2017

Issue description

See https://chromium.googlesource.com/chromium/src/+/lkgr/components/omnibox/browser/zero_suggest_provider.cc#503

bool ZeroSuggestProvider::ShouldShowNonContextualZeroSuggest(
    const GURL& suggest_url,
    const GURL& current_page_url) const {
  ...
  // 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().
  if (!current_page_url.is_valid() ||
      ((current_page_url.scheme() != url::kHttpScheme) &&
      (current_page_url.scheme() != url::kHttpsScheme)))
    return false;

Other non-contextual zero-suggest-like providers (physical web, clipboard), both don't implement this restriction.

We should figure out if there's a real issue here with suggestion the verbatim match in these conditions and make them all converge on the same behavior.

 
It appears there are a surprising number of URLs marked as UNKNOWN scheme (which wouldn't pass this test): 2% on Android for Navigation.MainFrameScheme and 5% for Navigation.SchemePerUniqueOrigin.
Components: -UI>Browser>Omnibox UI>Browser>Omnibox>ZeroSuggest
Labels: -OS-iOS
Owner: mpear...@chromium.org
Status: Assigned (was: Available)
Dropping iOS, as iOS doesn't have zero suggest.

Sent inquiry to owners of Navigation.MainFrameScheme histogram to see if they have any idea what kinds of things end up as UNKNOWN.

Assigning to myself to remember to follow-up on this bug as I recently touched this code.
Blockedon: 737581
NextAction: 2017-09-15
Let's re-evaluate this proposal after  bug 737581  is fixed, which may get rid of some of the unknown logging.
The NextAction date has arrived: 2017-09-15
NextAction: ----
M-61, which has better logging, has hit stable.

It appears that most of the UNKNOWN were Chrome native.  Navigation.MainFrameScheme reports 66% https, 32% http, 2% chrome-native.  Everything else is tiny (0.08% about, 0.05% file, 0.04% chrome, 0.01% chrome-distiller, 0.02% true unknown).

Now to evaluate whether putting URLs of those known type (chrome-native, file, etc.) in the omnibox all works fine Android.
Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3fb0e3163028bbd9a8cd3276dfc6bd8370ea3170

commit 3fb0e3163028bbd9a8cd3276dfc6bd8370ea3170
Author: Mark Pearson <mpearson@chromium.org>
Date: Mon Aug 27 21:53:57 2018

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: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586429}
[modify] https://crrev.com/3fb0e3163028bbd9a8cd3276dfc6bd8370ea3170/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/LocationBarLayoutTest.java
[modify] https://crrev.com/3fb0e3163028bbd9a8cd3276dfc6bd8370ea3170/chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc
[modify] https://crrev.com/3fb0e3163028bbd9a8cd3276dfc6bd8370ea3170/chrome/browser/autocomplete/chrome_autocomplete_provider_client.h
[modify] https://crrev.com/3fb0e3163028bbd9a8cd3276dfc6bd8370ea3170/components/omnibox/browser/autocomplete_provider_client.h
[modify] https://crrev.com/3fb0e3163028bbd9a8cd3276dfc6bd8370ea3170/components/omnibox/browser/builtin_provider_unittest.cc
[modify] https://crrev.com/3fb0e3163028bbd9a8cd3276dfc6bd8370ea3170/components/omnibox/browser/mock_autocomplete_provider_client.h
[modify] https://crrev.com/3fb0e3163028bbd9a8cd3276dfc6bd8370ea3170/components/omnibox/browser/zero_suggest_provider.cc
[modify] https://crrev.com/3fb0e3163028bbd9a8cd3276dfc6bd8370ea3170/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.cc
[modify] https://crrev.com/3fb0e3163028bbd9a8cd3276dfc6bd8370ea3170/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.h

Status: Fixed (was: Started)

Sign in to add a comment