Commit 6111a169 authored by Ce Chen's avatar Ce Chen Committed by Commit Bot

[omnibox] Make on device model serving instance creation asynchronous.

The creation will happen on |task_runner_| when |model_update_listener|
fires the new model available notification, instead of synchronously
on receiving the user autocomplete input.

This is to fix the omnibox.QueryTime2.1 regression (other QueryTime2.x
is not affected by the existing design), e.g.
http://screenshot/1U2Oq5vfnRm

Tested on Pixel 2 XL for pre-existing model loading and new model
download.

Bug: 925072
Change-Id: Ieb26aa78c8bbd848ed1f0c9b722bab266b21aa59
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902513
Commit-Queue: Ce Chen <cch@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713978}
parent 8fd97193
......@@ -148,9 +148,6 @@ void OnDeviceHeadProvider::Start(const AutocompleteInput& input,
if (minimal_changes)
return;
// Load the new model first before fulfilling the request if it's available.
MaybeResetServingInstanceFromNewModel();
matches_.clear();
if (!input.text().empty() && serving_) {
done_ = false;
......@@ -199,18 +196,21 @@ void OnDeviceHeadProvider::Stop(bool clear_cached_results,
void OnDeviceHeadProvider::OnModelUpdate(
const std::string& new_model_filename) {
if (new_model_filename != current_model_filename_ &&
new_model_filename_.empty())
new_model_filename_ = new_model_filename;
!new_model_filename.empty()) {
task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&OnDeviceHeadProvider::ResetServingInstanceFromNewModel,
weak_ptr_factory_.GetWeakPtr(), new_model_filename));
}
}
void OnDeviceHeadProvider::MaybeResetServingInstanceFromNewModel() {
if (new_model_filename_.empty())
void OnDeviceHeadProvider::ResetServingInstanceFromNewModel(
const std::string& new_model_filename) {
if (new_model_filename.empty())
return;
serving_ =
OnDeviceHeadServing::Create(new_model_filename_, provider_max_matches_);
current_model_filename_ = new_model_filename_;
new_model_filename_.clear();
current_model_filename_ = new_model_filename;
serving_ = OnDeviceHeadServing::Create(current_model_filename_,
provider_max_matches_);
}
void OnDeviceHeadProvider::AddProviderInfo(ProvidersInfo* provider_info) const {
......
......@@ -64,7 +64,7 @@ class OnDeviceHeadProvider : public AutocompleteProvider {
// Resets |serving_| if new model is available and cleans up the old model if
// it exists.
void MaybeResetServingInstanceFromNewModel();
void ResetServingInstanceFromNewModel(const std::string& new_model_filename);
AutocompleteProviderClient* client_;
AutocompleteProviderListener* listener_;
......@@ -73,8 +73,9 @@ class OnDeviceHeadProvider : public AutocompleteProvider {
// suggestions matching the Autocomplete input.
std::unique_ptr<OnDeviceHeadServing> serving_;
// The task runner instance where asynchronous searches to the head model will
// be run.
// The task runner instance where asynchronous operations using |serving_|
// will be run. Note that SequencedTaskRunner guarantees that operations will
// be executed in sequence so we don't need to apply a lock to |serving_|.
scoped_refptr<base::SequencedTaskRunner> task_runner_;
// The request id used to trace current request to the on device head model.
......@@ -85,9 +86,6 @@ class OnDeviceHeadProvider : public AutocompleteProvider {
// The filename for the on device model currently being used.
std::string current_model_filename_;
// The filename for the new model updated by Component Updater.
std::string new_model_filename_;
// Owns the subscription after adding the model update callback to the
// listener such that the callback can be removed automatically from the
// listener on provider's deconstruction.
......
......@@ -59,7 +59,6 @@ class OnDeviceHeadProviderTest : public testing::Test,
if (provider_) {
provider_->serving_.reset();
provider_->current_model_filename_.clear();
provider_->new_model_filename_.clear();
}
}
......
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