Skip to content

Commit

Permalink
Quick Answers: Add Spell checker
Browse files Browse the repository at this point in the history
This change adds a spell checker (of application locale) for Quick
answers to filter out invalid words (so that we do not need to send
queries to the server which takes long time).

We download the dictionary if necessary (if ChromeOS spell check service
is on the dict file should be already available).

The logics related to third party Hunspell library are posted to an
utility process, see more details in the upstream CL.

DD: go/qa-spellcheck, go/qa-always-on

Bug: b/221967354
Test: Run existing tests
Change-Id: If2bd705da9496290cb9cbe8d1d58d9fd39a2baf1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3531562
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Josh Simmons <jds@google.com>
Commit-Queue: Yue Li <updowndota@chromium.org>
Cr-Commit-Position: refs/heads/main@{#985164}
  • Loading branch information
Yue Li authored and Chromium LUCI CQ committed Mar 25, 2022
1 parent 15cfab5 commit 1696ae4
Show file tree
Hide file tree
Showing 7 changed files with 370 additions and 3 deletions.
Expand Up @@ -24,11 +24,17 @@ class TestQuickAnswersStateObserver : public QuickAnswersStateObserver {
void OnSettingsEnabled(bool settings_enabled) override {
settings_enabled_ = settings_enabled;
}
void OnApplicationLocaleReady(
const std::string& application_locale) override {
application_locale_ = application_locale;
}

bool settings_enabled() const { return settings_enabled_; }
const std::string& application_locale() const { return application_locale_; }

private:
bool settings_enabled_ = false;
std::string application_locale_;
};

class QuickAnswersStateControllerTest : public ChromeQuickAnswersTestBase {
Expand Down Expand Up @@ -91,6 +97,22 @@ TEST_F(QuickAnswersStateControllerTest, NotifySettingsEnabled) {
QuickAnswersState::Get()->RemoveObserver(observer());
}

TEST_F(QuickAnswersStateControllerTest, NotifyApplicationLocaleReady) {
QuickAnswersState::Get()->AddObserver(observer());

EXPECT_TRUE(QuickAnswersState::Get()->application_locale().empty());
EXPECT_TRUE(observer()->application_locale().empty());

const std::string application_locale = "en-US";

// The observer class should get an notification when the pref value changes.
prefs()->SetString(language::prefs::kApplicationLocale, application_locale);
EXPECT_EQ(QuickAnswersState::Get()->application_locale(), application_locale);
EXPECT_EQ(observer()->application_locale(), application_locale);

QuickAnswersState::Get()->RemoveObserver(observer());
}

TEST_F(QuickAnswersStateControllerTest, LocaleEligible) {
UErrorCode error_code = U_ZERO_ERROR;
icu::Locale::setDefault(icu::Locale(ULOC_US), error_code);
Expand Down
6 changes: 6 additions & 0 deletions chromeos/components/quick_answers/BUILD.gn
Expand Up @@ -38,15 +38,19 @@ source_set("quick_answers") {
"utils/quick_answers_metrics.h",
"utils/quick_answers_utils.cc",
"utils/quick_answers_utils.h",
"utils/spell_checker.cc",
"utils/spell_checker.h",
"utils/unit_conversion_constants.cc",
"utils/unit_conversion_constants.h",
"utils/unit_converter.cc",
"utils/unit_converter.h",
]
deps = [
"//base",
"//chrome/common:constants",
"//chromeos/components/quick_answers/public/cpp:cpp",
"//chromeos/components/quick_answers/public/cpp:prefs",
"//chromeos/components/quick_answers/public/mojom",
"//chromeos/constants",
"//chromeos/services/assistant/public/shared",
"//chromeos/services/machine_learning/public/cpp",
Expand All @@ -56,6 +60,8 @@ source_set("quick_answers") {
"//components/prefs:prefs",
"//components/signin/public/base",
"//components/signin/public/identity_manager",
"//components/spellcheck/common",
"//content/public/browser",
"//net:net",
"//services/data_decoder/public/cpp",
"//services/network/public/cpp:cpp",
Expand Down
3 changes: 3 additions & 0 deletions chromeos/components/quick_answers/DEPS
@@ -1,6 +1,9 @@
include_rules = [
"+ash/public",
"+chrome/common",
"+components/language/core/browser",
"+components/spellcheck/common",
"+content/public/browser",
"+services/data_decoder/public",
"+third_party/hunspell",
"+ui/base/l10n",
Expand Down
Expand Up @@ -134,12 +134,17 @@ void QuickAnswersState::RegisterPrefChanges(PrefService* pref_service) {
kQuickAnswersUnitConversionEnabled,
base::BindRepeating(&QuickAnswersState::UpdateUnitConversionEnabled,
base::Unretained(this)));
pref_change_registrar_->Add(
language::prefs::kApplicationLocale,
base::BindRepeating(&QuickAnswersState::OnApplicationLocaleReady,
base::Unretained(this)));

UpdateSettingsEnabled();
UpdateConsentStatus();
UpdateDefinitionEnabled();
UpdateTranslationEnabled();
UpdateUnitConversionEnabled();
OnApplicationLocaleReady();

prefs_initialized_ = true;

Expand Down Expand Up @@ -267,14 +272,30 @@ void QuickAnswersState::UpdateUnitConversionEnabled() {
unit_conversion_enabled_ = unit_conversion_enabled;
}

void QuickAnswersState::OnApplicationLocaleReady() {
auto locale = pref_change_registrar_->prefs()->GetString(
language::prefs::kApplicationLocale);
if (application_locale_ == locale) {
return;
}
application_locale_ = locale;

for (auto& observer : observers_) {
observer.OnApplicationLocaleReady(locale);
}

UpdateEligibility();
}

void QuickAnswersState::UpdateEligibility() {
if (!pref_change_registrar_)
return;

std::string locale = pref_change_registrar_->prefs()->GetString(
language::prefs::kApplicationLocale);
if (application_locale_.empty())
return;

std::string resolved_locale;
l10n_util::CheckAndResolveLocale(locale, &resolved_locale,
l10n_util::CheckAndResolveLocale(application_locale_, &resolved_locale,
/*perform_io=*/false);
is_eligible_ = IsQuickAnswersAllowedForLocale(
resolved_locale, icu::Locale::getDefault().getName());
Expand Down
Expand Up @@ -35,6 +35,7 @@ enum class ConsentResultType {
class QuickAnswersStateObserver : public base::CheckedObserver {
public:
virtual void OnSettingsEnabled(bool enabled) {}
virtual void OnApplicationLocaleReady(const std::string& locale) {}
};

// A class that holds Quick Answers related prefs and states.
Expand Down Expand Up @@ -66,6 +67,7 @@ class QuickAnswersState {
bool definition_enabled() const { return definition_enabled_; }
bool translation_enabled() const { return translation_enabled_; }
bool unit_conversion_enabled() const { return unit_conversion_enabled_; }
const std::string& application_locale() const { return application_locale_; }
bool is_eligible() const { return is_eligible_; }

void set_eligibility_for_testing(bool is_eligible) {
Expand All @@ -84,6 +86,7 @@ class QuickAnswersState {
void UpdateDefinitionEnabled();
void UpdateTranslationEnabled();
void UpdateUnitConversionEnabled();
void OnApplicationLocaleReady();

// Called when the feature eligibility might change.
void UpdateEligibility();
Expand All @@ -104,6 +107,9 @@ class QuickAnswersState {
// Whether the Quick Answers unit conversion is enabled.
bool unit_conversion_enabled_ = true;

// The application locale.
std::string application_locale_;

// Whether the Quick Answers feature is eligible. The value is derived from a
// number of other states.
bool is_eligible_ = false;
Expand Down
229 changes: 229 additions & 0 deletions chromeos/components/quick_answers/utils/spell_checker.cc
@@ -0,0 +1,229 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chromeos/components/quick_answers/utils/spell_checker.h"

#include "base/files/file_util.h"
#include "base/logging.h"
#include "base/path_service.h"
#include "base/threading/scoped_blocking_call.h"
#include "chrome/common/chrome_paths.h"
#include "components/spellcheck/common/spellcheck_common.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/service_process_host.h"
#include "services/network/public/cpp/resource_request.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/cpp/simple_url_loader.h"
#include "services/network/public/mojom/url_response_head.mojom.h"

namespace quick_answers {
namespace {

constexpr char kDownloadServerUrl[] =
"https://redirector.gvt1.com/edgedl/chrome/dict/";

constexpr net::NetworkTrafficAnnotationTag kNetworkTrafficAnnotationTag =
net::DefineNetworkTrafficAnnotation("quick_answers_spellchecker", R"(
semantics {
sender: "Quick answers Spellchecker"
description:
"Download dictionary for Quick answers feature if necessary."
trigger: "Quick answers feature enabled."
data:
"The spell checking language identifier. No user identifier is "
"sent."
destination: GOOGLE_OWNED_SERVICE
}
policy {
cookies_allowed: NO
setting:
"Quick Answers can be enabled/disabled in ChromeOS Settings and"
"is subject to eligibility requirements."
})");

constexpr int kMaxRetries = 3;

base::FilePath GetDictionaryFilePath(const std::string& language) {
base::FilePath dict_dir;
base::PathService::Get(chrome::DIR_APP_DICTIONARIES, &dict_dir);
base::FilePath dict_path =
spellcheck::GetVersionedFileName(language, dict_dir);
return dict_path;
}

GURL GetDictionaryURL(const std::string& file_name) {
return GURL(std::string(kDownloadServerUrl) + base::ToLowerASCII(file_name));
}

bool SaveDictionaryData(std::unique_ptr<std::string> data,
const base::FilePath& file_path) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
base::BlockingType::MAY_BLOCK);

// Create a temporary file.
base::FilePath tmp_path;
if (!base::CreateTemporaryFileInDir(file_path.DirName(), &tmp_path)) {
LOG(ERROR) << "Failed to create a temporary file.";
return false;
}

// Write to the temporary file.
size_t bytes_written =
base::WriteFile(tmp_path, data->data(), data->length());
if (bytes_written != data->length()) {
base::DeleteFile(tmp_path);
LOG(ERROR) << "Failed to write dictionary data to the temporary file";
return false;
}

// Atomically rename the temporary file to become the real one.
return base::ReplaceFile(tmp_path, file_path, nullptr);
}

void RemoveDictionaryFle(const base::FilePath& file_path) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
base::BlockingType::MAY_BLOCK);

base::DeleteFile(file_path);
}

} // namespace

SpellChecker::SpellChecker(
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
: url_loader_factory_(url_loader_factory) {
quick_answers_state_observation_.Observe(QuickAnswersState::Get());
}

SpellChecker::~SpellChecker() = default;

void SpellChecker::CheckSpelling(const std::string& word,
CheckSpellingCallback callback) {
if (!dictionary_initialized_) {
std::move(callback).Run(false);
return;
}

dictionary_->CheckSpelling(word, std::move(callback));
}

void SpellChecker::OnSettingsEnabled(bool enabled) {
feature_enabled_ = enabled;

// Reset spell check service if the feature is disabled.
if (!enabled) {
dictionary_.reset();
service_.reset();
return;
}

if (!dictionary_file_path_.empty())
InitializeDictionary();
}

void SpellChecker::OnApplicationLocaleReady(const std::string& locale) {
dictionary_file_path_ = GetDictionaryFilePath(locale);

if (feature_enabled_)
InitializeDictionary();
}

void SpellChecker::InitializeDictionary() {
DCHECK(!dictionary_file_path_.empty());

// If the dictionary is not available, try to download it from the server.
if (!base::PathExists(dictionary_file_path_)) {
auto url =
GetDictionaryURL(dictionary_file_path_.BaseName().MaybeAsASCII());

auto resource_request = std::make_unique<network::ResourceRequest>();
resource_request->url = url;
resource_request->credentials_mode = network::mojom::CredentialsMode::kOmit;
loader_ = network::SimpleURLLoader::Create(std::move(resource_request),
kNetworkTrafficAnnotationTag);
// TODO(b/226221138): Probably use |DownloadToTempFile| instead.
loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
url_loader_factory_.get(),
base::BindOnce(&SpellChecker::OnSimpleURLLoaderComplete,
base::Unretained(this)));
return;
}

InitializeSpellCheckService();
}

void SpellChecker::InitializeSpellCheckService() {
DCHECK(base::PathExists(dictionary_file_path_));

if (!service_) {
service_ = content::ServiceProcessHost::Launch<mojom::SpellCheckService>(
content::ServiceProcessHost::Options()
.WithDisplayName("Quick answers spell check service")
.Pass());
}

base::File file(dictionary_file_path_,
base::File::FLAG_READ | base::File::FLAG_OPEN);

service_->CreateDictionary(file.Duplicate(),
base::BindOnce(&SpellChecker::OnDictionaryCreated,
base::Unretained(this)));
}

void SpellChecker::OnSimpleURLLoaderComplete(
std::unique_ptr<std::string> data) {
int response_code = -1;
if (loader_->ResponseInfo() && loader_->ResponseInfo()->headers)
response_code = loader_->ResponseInfo()->headers->response_code();

if (loader_->NetError() != net::OK || ((response_code / 100) != 2)) {
LOG(ERROR) << "Failed to download the dictionary.";
MaybeRetryInitialize();
return;
}

// Basic sanity check on the dictionary data.
if (!data || data->size() < 4 || data->compare(0, 4, "BDic") != 0) {
LOG(ERROR) << "Downloaded dictionary data is empty or broken.";
MaybeRetryInitialize();
return;
}

if (!SaveDictionaryData(std::move(data), dictionary_file_path_)) {
MaybeRetryInitialize();
return;
}

InitializeSpellCheckService();
}

void SpellChecker::OnDictionaryCreated(
mojo::PendingRemote<mojom::SpellCheckDictionary> dictionary) {
dictionary_.reset();

if (dictionary.is_valid()) {
dictionary_.Bind(std::move(dictionary));
dictionary_initialized_ = true;
return;
}

MaybeRetryInitialize();
}

void SpellChecker::MaybeRetryInitialize() {
RemoveDictionaryFle(dictionary_file_path_);

if (num_retries_ >= kMaxRetries) {
LOG(ERROR) << "Service initialize failed after max retries";
service_.reset();
return;
}

++num_retries_;
InitializeDictionary();
}

} // namespace quick_answers

0 comments on commit 1696ae4

Please sign in to comment.