New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 854242 link

Starred by 9 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 854233


Show other hotlists

Hotlists containing this issue:
Launcher-Feature


Sign in to add a comment

Eng Tracking: Recent Queries for Launcher Zero State

Project Member Reported by mmalpani@chromium.org, Jun 19 2018

Issue description

This is a tracking bug for the bottom panel on the Chrome OS Zero State, more specifically, the panel containing the user's recent queries across all devices.

 
Cc: hfung@chromium.org abodenha@chromium.org mpear...@chromium.org
The recent queries can be served by launcher’s OmniboxProvider which calls Chrome’s omnibox team’s code at:
https://cs.chromium.org/chromium/src/components/omnibox/browser/autocomplete_controller.h?type=cs&q=AutocompleteController&g=0&l=61

The AutoCompleteController is a wrapper of a set of data providers, and one of them is ZeroSuggestProvider which talks to the Google suggest server to serve the zero suggest results back depending on the type of results requested (recent queries, most visited sites, etc).

In order to get the recent queries Zero State feature wants, we will work with omnibox team to make some changes on both sides. Omnibox code needs to make some change to differentiate the queries from chrome omnibox vs launcher zeror state, and zero suggest server have to serve the right results for launcher without affecting omnibox. We have talked about adding a new page context parameter for launcher to configure the AutoCompleteInput, or too add a new client id with suggest server. Omnibox team has been busy working with other feature in R69. They will help us to work on this after the mid July.
Here's a draft of what I think needs to be done:

* Add code to the server that, in response to a certain type of request (client= param or a new param to be decided), issues the same response that that regular desktop Chrome requests would get *except* for zero suggest requests, for which it should return recent queries issues by the user.

* Add an new page classification type for launcher.  Make the AutocompleteInput the launcher constructs set this page classification type.
  - This new enum value needs to happen on the omnibox_event.proto file internally first.
  - Then in a Chromium changelist we can update the third_party import of that proto in Chromium.  During that import to Chromium step, also remember to update enums.xml for this new value.

* Add a feature flag (LauncherUsesOwnSuggestClient or something similar) somewhere to make the suggest requests that search_provider.cc and zero_suggest.cc provider send slightly different requests when in launcher page contexts.  Exactly how / where the feature flag resides depends on which this is a change in the client= type or an extra parameter or something.

* Revise omnibox_field_trial.cc regarding how zero suggest modes are determined.  Right now the mode to run is determined based on looking within a particular field trial (OmniboxBundledExperiment) for a particular parameter.  We probably want to decouple this from the regular omnibox experiments so it can be experimented with separately.  The current setup only requires that the mode be stable, not depending on page context.  Obviously, we need to get rid of this too.
  - I suggest changing the zero suggest code to use a feature flag, HasZeroSuggestContextParam.
  - If the feature flag is set, lookup a parameter associated with that feature.  Make this lookup context dependent.  For example, for page context == 10, we should lookup the value of the parameters "ZeroSuggestVariant:10".  The values can still be the previous ones, things like MostVisited, Personalized, etc.
  - These steps should be done carefully to not alter any current behavior unless the feature flag is set.

* Audit and modify zero_suggest.cc code as appropriate with regard to caching.  Some modes cache suggestions.  If we change modes depending on page context, then we need to make sure we're not reusing other-mode suggestions at the wrong time.

Status: Assigned (was: Unconfirmed)
Per discussion with the suggest folks, it sounds like the ChromeOS launcher should be using a different client= parameter (client=cros-launcher?).  And it would probably make sense to pass the client= to search requests too.  In other words, the SearchTermsData object passes to the autocomplete system when run from the launcher needs to have different implementation of these two functions.
https://cs.chromium.org/chromium/src/components/search_engines/search_terms_data.h?l=37-44
Per discussions with suggest folks, we might be able to use sclient= parameter, if it's easier.

Also, I'm not so sure we should use a different client= parameter for the search request, not just the suggest (/complete) request too.  We can discuss that later.
https://cs.chromium.org/chromium/src/components/search_engines/search_terms_data.h?l=37-44
We will need to override both SeartTermsData::GetSearchClient() and GetSuggestClient(). Currently ChromeAutocompleteProviderClient is using UIThreadSearchTermsData to construct its SearchTermsData object, which returns an empty string and "chrome-omni" for GetSuggestClient(). For the launcher suggest, what do you suggest us to return for these two? 

So it looks like for launcher side integration, we should pass back the new launcher client strings to omnibox code, and omni box code should give us the regular suggestion results for non-empty query and zero suggestion results for empty query. Is my understanding correct?


If the search client is currently blank, let's leave it blank.

If the suggest client is currently chrome-omni, let's do something like "cros-launcher" or "chrome-os-launcher".  Ask hfung@ (on the thread we already have open with him) if those are okay, he has a preference, and whether we need additional approvals or anything for it.

>>>
So it looks like for launcher side integration, we should pass back the new launcher client strings to omnibox code, and omni box code should give us the regular suggestion results for non-empty query and zero suggestion results for empty query. Is my understanding correct?
>>>
I think for ease of understanding we should make the chrome os launcher client be sent all requests from the launcher box (both zero suggestion requests and as-you-typed requests).  Does that answer your question?
Re#7, Mark, your understanding is correct, that's our plan :-)
Labels: -Pri-3 M-70 Pri-2
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 10

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

commit 5bc2e3de5bc4ff19baa646ed1a820883f2effb1e
Author: Jenny Zhang <jennyz@chromium.org>
Date: Mon Sep 10 18:50:55 2018

Integrate app_list zero state suggestion with omnibox.

1. Add query params pgcl=14 and sclient=cros-launcher for launcher queries to completion server.
2. Enable ZeroSuggestProvider to process empty queries from cros launcher.
3. The above changes are only enabled when zero state is enabled by flag, which by default is off.

Bug:  854242 
Change-Id: I9ca99a668b74ab642ff1d020eb14565fb4af12ff
Reviewed-on: https://chromium-review.googlesource.com/1178971
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Commit-Queue: Jenny Zhang <jennyz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590003}
[modify] https://crrev.com/5bc2e3de5bc4ff19baa646ed1a820883f2effb1e/chrome/browser/ui/app_list/search/omnibox_provider.cc
[modify] https://crrev.com/5bc2e3de5bc4ff19baa646ed1a820883f2effb1e/chrome/browser/ui/app_list/search/omnibox_provider.h
[modify] https://crrev.com/5bc2e3de5bc4ff19baa646ed1a820883f2effb1e/components/omnibox/browser/base_search_provider.cc
[modify] https://crrev.com/5bc2e3de5bc4ff19baa646ed1a820883f2effb1e/components/omnibox/browser/base_search_provider.h
[modify] https://crrev.com/5bc2e3de5bc4ff19baa646ed1a820883f2effb1e/components/omnibox/browser/contextual_suggestions_service.cc
[modify] https://crrev.com/5bc2e3de5bc4ff19baa646ed1a820883f2effb1e/components/omnibox/browser/contextual_suggestions_service.h
[modify] https://crrev.com/5bc2e3de5bc4ff19baa646ed1a820883f2effb1e/components/omnibox/browser/search_provider.cc
[modify] https://crrev.com/5bc2e3de5bc4ff19baa646ed1a820883f2effb1e/components/omnibox/browser/zero_suggest_provider.cc

Status: Started (was: Assigned)
Labels: -M-70 M-71
Labels: -M-71 M-72
Labels: -M-72 M-73
Issue 224608 has been merged into this issue.
Status: Fixed (was: Started)

Sign in to add a comment