From 1696ae4b4d7ce75bf65f9fc1a48a364c3304de99 Mon Sep 17 00:00:00 2001 From: Yue Li Date: Fri, 25 Mar 2022 03:29:38 +0000 Subject: [PATCH] Quick Answers: Add Spell checker 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 Reviewed-by: Avi Drissman Reviewed-by: Josh Simmons Commit-Queue: Yue Li Cr-Commit-Position: refs/heads/main@{#985164} --- ...quick_answers_state_controller_unittest.cc | 22 ++ chromeos/components/quick_answers/BUILD.gn | 6 + chromeos/components/quick_answers/DEPS | 3 + .../public/cpp/quick_answers_state.cc | 27 ++- .../public/cpp/quick_answers_state.h | 6 + .../quick_answers/utils/spell_checker.cc | 229 ++++++++++++++++++ .../quick_answers/utils/spell_checker.h | 80 ++++++ 7 files changed, 370 insertions(+), 3 deletions(-) create mode 100644 chromeos/components/quick_answers/utils/spell_checker.cc create mode 100644 chromeos/components/quick_answers/utils/spell_checker.h diff --git a/chrome/browser/ui/quick_answers/quick_answers_state_controller_unittest.cc b/chrome/browser/ui/quick_answers/quick_answers_state_controller_unittest.cc index f733b1482070c5..87cde4bff5d55e 100644 --- a/chrome/browser/ui/quick_answers/quick_answers_state_controller_unittest.cc +++ b/chrome/browser/ui/quick_answers/quick_answers_state_controller_unittest.cc @@ -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 { @@ -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); diff --git a/chromeos/components/quick_answers/BUILD.gn b/chromeos/components/quick_answers/BUILD.gn index 3259b9be9b1b9c..9d987c342dc84c 100644 --- a/chromeos/components/quick_answers/BUILD.gn +++ b/chromeos/components/quick_answers/BUILD.gn @@ -38,6 +38,8 @@ 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", @@ -45,8 +47,10 @@ source_set("quick_answers") { ] 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", @@ -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", diff --git a/chromeos/components/quick_answers/DEPS b/chromeos/components/quick_answers/DEPS index 34c2af07b41336..4e5168e07e3e7a 100644 --- a/chromeos/components/quick_answers/DEPS +++ b/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", diff --git a/chromeos/components/quick_answers/public/cpp/quick_answers_state.cc b/chromeos/components/quick_answers/public/cpp/quick_answers_state.cc index fc3f5ee7634a51..e3eef811b0e67b 100644 --- a/chromeos/components/quick_answers/public/cpp/quick_answers_state.cc +++ b/chromeos/components/quick_answers/public/cpp/quick_answers_state.cc @@ -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; @@ -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()); diff --git a/chromeos/components/quick_answers/public/cpp/quick_answers_state.h b/chromeos/components/quick_answers/public/cpp/quick_answers_state.h index 5700e1e453897b..1b746341bf9358 100644 --- a/chromeos/components/quick_answers/public/cpp/quick_answers_state.h +++ b/chromeos/components/quick_answers/public/cpp/quick_answers_state.h @@ -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. @@ -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) { @@ -84,6 +86,7 @@ class QuickAnswersState { void UpdateDefinitionEnabled(); void UpdateTranslationEnabled(); void UpdateUnitConversionEnabled(); + void OnApplicationLocaleReady(); // Called when the feature eligibility might change. void UpdateEligibility(); @@ -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; diff --git a/chromeos/components/quick_answers/utils/spell_checker.cc b/chromeos/components/quick_answers/utils/spell_checker.cc new file mode 100644 index 00000000000000..b2ae686debe21f --- /dev/null +++ b/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 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 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(); + 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( + 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 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 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 diff --git a/chromeos/components/quick_answers/utils/spell_checker.h b/chromeos/components/quick_answers/utils/spell_checker.h new file mode 100644 index 00000000000000..5ee1d5aecb8058 --- /dev/null +++ b/chromeos/components/quick_answers/utils/spell_checker.h @@ -0,0 +1,80 @@ +// 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. + +#ifndef CHROMEOS_COMPONENTS_QUICK_ANSWERS_UTILS_SPELL_CHECKER_H_ +#define CHROMEOS_COMPONENTS_QUICK_ANSWERS_UTILS_SPELL_CHECKER_H_ + +#include +#include + +#include "base/files/file_path.h" +#include "base/memory/weak_ptr.h" +#include "base/scoped_observation.h" +#include "chromeos/components/quick_answers/public/cpp/quick_answers_state.h" +#include "chromeos/components/quick_answers/public/mojom/spell_check.mojom.h" +#include "mojo/public/cpp/bindings/remote.h" + +namespace network { +class SharedURLLoaderFactory; +class SimpleURLLoader; +} // namespace network + +namespace quick_answers { + +// Utility class for spell check. +class SpellChecker : public QuickAnswersStateObserver { + public: + using CheckSpellingCallback = base::OnceCallback; + + explicit SpellChecker( + scoped_refptr url_loader_factory); + + SpellChecker(const SpellChecker&) = delete; + SpellChecker& operator=(const SpellChecker&) = delete; + + ~SpellChecker() override; + + // Check spelling of the given word, run |callback| with true if the word is + // spelled correctly. + void CheckSpelling(const std::string& word, CheckSpellingCallback callback); + + // QuickAnswersStateObserver: + void OnSettingsEnabled(bool enabled) override; + void OnApplicationLocaleReady(const std::string& locale) override; + + private: + void InitializeDictionary(); + void InitializeSpellCheckService(); + + void OnSimpleURLLoaderComplete(std::unique_ptr response_body); + + void OnDictionaryCreated( + mojo::PendingRemote dictionary); + + void MaybeRetryInitialize(); + + // Whether the Quick answers feature is enabled in settings. + bool feature_enabled_ = false; + + // Whether the spell check dictionary has been successfully initialized. + bool dictionary_initialized_ = false; + + // Number of retries used for initializing the spell check service. + int num_retries_ = 0; + + base::FilePath dictionary_file_path_; + scoped_refptr url_loader_factory_; + std::unique_ptr loader_; + mojo::Remote service_; + mojo::Remote dictionary_; + + base::ScopedObservation + quick_answers_state_observation_{this}; + + base::WeakPtrFactory weak_factory_{this}; +}; + +} // namespace quick_answers + +#endif // CHROMEOS_COMPONENTS_QUICK_ANSWERS_UTILS_SPELL_CHECKER_H_