From e378cf53f12d04a7d22084556579c1c0ca26c0a0 Mon Sep 17 00:00:00 2001 From: Yue Li Date: Thu, 24 Mar 2022 18:42:29 +0000 Subject: [PATCH] Quick Answers: Add spell checker in utility process This change adds a spell checker for Quick answers feature in utility process. A follow up change will add browser side implementation. The spell checker is added because QA needs different set of languages from spellcheck, and the two features are controlled by different set of settings toggles. Utility process is used because the third party Hunspell library is unstable and we want to protect browser process. DD: go/qa-spellcheck, go/qa-always-trigger Bug: b/221967354 Test: Run existing tests Change-Id: Ic92d1b5074a9456521ab8ccebbd0400a3f8fa50a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3528261 Reviewed-by: Avi Drissman Reviewed-by: Josh Simmons Reviewed-by: Daniel Cheng Commit-Queue: Yue Li Cr-Commit-Position: refs/heads/main@{#984946} --- chrome/utility/BUILD.gn | 2 + chrome/utility/DEPS | 1 + chrome/utility/services.cc | 9 ++++ chromeos/components/quick_answers/DEPS | 1 + .../quick_answers/public/cpp/BUILD.gn | 6 +++ .../cpp/service/spell_check_dictionary.cc | 45 +++++++++++++++++++ .../cpp/service/spell_check_dictionary.h | 44 ++++++++++++++++++ .../public/cpp/service/spell_check_service.cc | 33 ++++++++++++++ .../public/cpp/service/spell_check_service.h | 41 +++++++++++++++++ .../quick_answers/public/mojom/BUILD.gn | 14 ++++++ .../quick_answers/public/mojom/OWNERS | 2 + .../public/mojom/spell_check.mojom | 29 ++++++++++++ 12 files changed, 227 insertions(+) create mode 100644 chromeos/components/quick_answers/public/cpp/service/spell_check_dictionary.cc create mode 100644 chromeos/components/quick_answers/public/cpp/service/spell_check_dictionary.h create mode 100644 chromeos/components/quick_answers/public/cpp/service/spell_check_service.cc create mode 100644 chromeos/components/quick_answers/public/cpp/service/spell_check_service.h create mode 100644 chromeos/components/quick_answers/public/mojom/BUILD.gn create mode 100644 chromeos/components/quick_answers/public/mojom/OWNERS create mode 100644 chromeos/components/quick_answers/public/mojom/spell_check.mojom diff --git a/chrome/utility/BUILD.gn b/chrome/utility/BUILD.gn index 4f11cdb016fd53..805bae464aea9c 100644 --- a/chrome/utility/BUILD.gn +++ b/chrome/utility/BUILD.gn @@ -175,6 +175,8 @@ static_library("utility") { "//chromeos/assistant:buildflags", "//chromeos/components/local_search_service:local_search_service", "//chromeos/components/local_search_service/public/mojom", + "//chromeos/components/quick_answers/public/cpp", + "//chromeos/components/quick_answers/public/mojom", "//chromeos/services/tts", "//chromeos/services/tts/public/mojom", ] diff --git a/chrome/utility/DEPS b/chrome/utility/DEPS index 0ba4d0519aee1d..a1ff8fab6a661b 100644 --- a/chrome/utility/DEPS +++ b/chrome/utility/DEPS @@ -30,6 +30,7 @@ include_rules = [ "+chromeos/assistant/buildflags.h", "+chromeos/components/local_search_service/local_search_service.h", "+chromeos/components/local_search_service/public/mojom", + "+chromeos/components/quick_answers/public", "+chromeos/services/assistant", "+chromeos/services/libassistant/libassistant_service.h", "+chromeos/services/tts", diff --git a/chrome/utility/services.cc b/chrome/utility/services.cc index da71bb4a3515ba..97aaeac0073748 100644 --- a/chrome/utility/services.cc +++ b/chrome/utility/services.cc @@ -110,6 +110,8 @@ #include "chromeos/assistant/buildflags.h" // nogncheck #include "chromeos/components/local_search_service/local_search_service.h" #include "chromeos/components/local_search_service/public/mojom/local_search_service.mojom.h" +#include "chromeos/components/quick_answers/public/cpp/service/spell_check_service.h" +#include "chromeos/components/quick_answers/public/mojom/spell_check.mojom.h" #include "chromeos/services/tts/public/mojom/tts_service.mojom.h" #include "chromeos/services/tts/tts_service.h" @@ -333,6 +335,12 @@ auto RunQuickPairService( std::move(receiver)); } +auto RunQuickAnswersSpellCheckService( + mojo::PendingReceiver receiver) { + return std::make_unique( + std::move(receiver)); +} + #if BUILDFLAG(ENABLE_CROS_LIBASSISTANT) auto RunAssistantAudioDecoder( mojo::PendingReceiver< @@ -436,6 +444,7 @@ void RegisterMainThreadServices(mojo::ServiceFactory& services) { services.Add(RunTtsService); services.Add(RunLocalSearchService); services.Add(RunQuickPairService); + services.Add(RunQuickAnswersSpellCheckService); #if BUILDFLAG(ENABLE_CROS_LIBASSISTANT) services.Add(RunAssistantAudioDecoder); services.Add(RunLibassistantService); diff --git a/chromeos/components/quick_answers/DEPS b/chromeos/components/quick_answers/DEPS index 6a96639d137661..34c2af07b41336 100644 --- a/chromeos/components/quick_answers/DEPS +++ b/chromeos/components/quick_answers/DEPS @@ -2,6 +2,7 @@ include_rules = [ "+ash/public", "+components/language/core/browser", "+services/data_decoder/public", + "+third_party/hunspell", "+ui/base/l10n", "+ui/base/resource/resource_bundle.h", "+ui/color", diff --git a/chromeos/components/quick_answers/public/cpp/BUILD.gn b/chromeos/components/quick_answers/public/cpp/BUILD.gn index ac7970b9f1bcfd..a791fc2ea3a343 100644 --- a/chromeos/components/quick_answers/public/cpp/BUILD.gn +++ b/chromeos/components/quick_answers/public/cpp/BUILD.gn @@ -8,14 +8,20 @@ source_set("cpp") { "controller/quick_answers_controller.h", "quick_answers_state.cc", "quick_answers_state.h", + "service/spell_check_dictionary.cc", + "service/spell_check_dictionary.h", + "service/spell_check_service.cc", + "service/spell_check_service.h", ] deps = [ "//base", "//chromeos/components/quick_answers/public/cpp:prefs", + "//chromeos/components/quick_answers/public/mojom", "//components/language/core/browser:browser", "//components/pref_registry", "//components/prefs", + "//third_party/hunspell", "//ui/base", "//ui/gfx", ] diff --git a/chromeos/components/quick_answers/public/cpp/service/spell_check_dictionary.cc b/chromeos/components/quick_answers/public/cpp/service/spell_check_dictionary.cc new file mode 100644 index 00000000000000..686de1146bcf9c --- /dev/null +++ b/chromeos/components/quick_answers/public/cpp/service/spell_check_dictionary.cc @@ -0,0 +1,45 @@ +// 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/public/cpp/service/spell_check_dictionary.h" + +#include "base/files/memory_mapped_file.h" +#include "base/logging.h" +#include "third_party/hunspell/src/hunspell/hunspell.hxx" + +namespace quick_answers { + +SpellCheckDictionary::SpellCheckDictionary() = default; + +SpellCheckDictionary::~SpellCheckDictionary() = default; + +bool SpellCheckDictionary::Initialize(base::File file) { + mapped_dict_file_ = std::make_unique(); + + if (!mapped_dict_file_->Initialize(std::move(file))) { + LOG(ERROR) << "Failed to mmap dictionary file."; + return false; + } + + if (!hunspell::BDict::Verify( + reinterpret_cast(mapped_dict_file_->data()), + mapped_dict_file_->length())) { + LOG(ERROR) << "Failed to verify dictionary file."; + return false; + } + + hunspell_ = std::make_unique(mapped_dict_file_->data(), + mapped_dict_file_->length()); + + return true; +} + +void SpellCheckDictionary::CheckSpelling(const std::string& word, + CheckSpellingCallback callback) { + DCHECK(hunspell_); + + std::move(callback).Run(hunspell_->spell(word) != 0); +} + +} // namespace quick_answers diff --git a/chromeos/components/quick_answers/public/cpp/service/spell_check_dictionary.h b/chromeos/components/quick_answers/public/cpp/service/spell_check_dictionary.h new file mode 100644 index 00000000000000..8c346211f3fcdb --- /dev/null +++ b/chromeos/components/quick_answers/public/cpp/service/spell_check_dictionary.h @@ -0,0 +1,44 @@ +// 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_PUBLIC_CPP_SERVICE_SPELL_CHECK_DICTIONARY_H_ +#define CHROMEOS_COMPONENTS_QUICK_ANSWERS_PUBLIC_CPP_SERVICE_SPELL_CHECK_DICTIONARY_H_ + +#include +#include + +#include "base/memory/weak_ptr.h" +#include "chromeos/components/quick_answers/public/mojom/spell_check.mojom.h" + +class Hunspell; + +namespace base { +class MemoryMappedFile; +} // namespace base + +namespace quick_answers { + +// Utility class for spell check ran in renderer process. +class SpellCheckDictionary : public mojom::SpellCheckDictionary { + public: + SpellCheckDictionary(); + + SpellCheckDictionary(const SpellCheckDictionary&) = delete; + SpellCheckDictionary& operator=(const SpellCheckDictionary&) = delete; + + ~SpellCheckDictionary() override; + + bool Initialize(base::File file); + + void CheckSpelling(const std::string& word, + CheckSpellingCallback callback) override; + + private: + std::unique_ptr mapped_dict_file_; + std::unique_ptr hunspell_; +}; + +} // namespace quick_answers + +#endif // CHROMEOS_COMPONENTS_QUICK_ANSWERS_PUBLIC_CPP_SERVICE_SPELL_CHECK_DICTIONARY_H_ diff --git a/chromeos/components/quick_answers/public/cpp/service/spell_check_service.cc b/chromeos/components/quick_answers/public/cpp/service/spell_check_service.cc new file mode 100644 index 00000000000000..87e55717aeba35 --- /dev/null +++ b/chromeos/components/quick_answers/public/cpp/service/spell_check_service.cc @@ -0,0 +1,33 @@ +// 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/public/cpp/service/spell_check_service.h" + +#include "base/logging.h" +#include "mojo/public/cpp/bindings/self_owned_receiver.h" + +namespace quick_answers { + +SpellCheckService::SpellCheckService( + mojo::PendingReceiver + pending_receiver) + : receiver_(this, std::move(pending_receiver)) {} + +SpellCheckService::~SpellCheckService() = default; + +void SpellCheckService::CreateDictionary(base::File file, + CreateDictionaryCallback callback) { + auto dictionary = std::make_unique(); + if (!dictionary->Initialize(std::move(file))) { + std::move(callback).Run(std::move(mojo::NullRemote())); + return; + } + + mojo::PendingRemote pending_remote; + mojo::MakeSelfOwnedReceiver(std::move(dictionary), + pending_remote.InitWithNewPipeAndPassReceiver()); + std::move(callback).Run(std::move(pending_remote)); +} + +} // namespace quick_answers diff --git a/chromeos/components/quick_answers/public/cpp/service/spell_check_service.h b/chromeos/components/quick_answers/public/cpp/service/spell_check_service.h new file mode 100644 index 00000000000000..28702fd2e77f4e --- /dev/null +++ b/chromeos/components/quick_answers/public/cpp/service/spell_check_service.h @@ -0,0 +1,41 @@ +// 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_PUBLIC_CPP_SERVICE_SPELL_CHECK_SERVICE_H_ +#define CHROMEOS_COMPONENTS_QUICK_ANSWERS_PUBLIC_CPP_SERVICE_SPELL_CHECK_SERVICE_H_ + +#include +#include + +#include "base/memory/weak_ptr.h" +#include "chromeos/components/quick_answers/public/cpp/service/spell_check_dictionary.h" +#include "chromeos/components/quick_answers/public/mojom/spell_check.mojom.h" +#include "mojo/public/cpp/bindings/pending_receiver.h" +#include "mojo/public/cpp/bindings/receiver.h" + +namespace quick_answers { + +// Utility class for spell check ran in renderer process. +class SpellCheckService : public mojom::SpellCheckService { + public: + explicit SpellCheckService( + mojo::PendingReceiver pending_receiver); + + SpellCheckService(const SpellCheckService&) = delete; + SpellCheckService& operator=(const SpellCheckService&) = delete; + + ~SpellCheckService() override; + + void CreateDictionary(base::File file, + CreateDictionaryCallback callback) override; + + private: + std::unique_ptr dictionary_; + + mojo::Receiver receiver_; +}; + +} // namespace quick_answers + +#endif // CHROMEOS_COMPONENTS_QUICK_ANSWERS_PUBLIC_CPP_SERVICE_SPELL_CHECK_SERVICE_H_ diff --git a/chromeos/components/quick_answers/public/mojom/BUILD.gn b/chromeos/components/quick_answers/public/mojom/BUILD.gn new file mode 100644 index 00000000000000..e65f81948b7587 --- /dev/null +++ b/chromeos/components/quick_answers/public/mojom/BUILD.gn @@ -0,0 +1,14 @@ +# 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. + +import("//mojo/public/tools/bindings/mojom.gni") + +mojom("mojom") { + sources = [ "spell_check.mojom" ] + + public_deps = [ + "//mojo/public/mojom/base", + "//sandbox/policy/mojom", + ] +} diff --git a/chromeos/components/quick_answers/public/mojom/OWNERS b/chromeos/components/quick_answers/public/mojom/OWNERS new file mode 100644 index 00000000000000..08850f421205f8 --- /dev/null +++ b/chromeos/components/quick_answers/public/mojom/OWNERS @@ -0,0 +1,2 @@ +per-file *.mojom=set noparent +per-file *.mojom=file://ipc/SECURITY_OWNERS diff --git a/chromeos/components/quick_answers/public/mojom/spell_check.mojom b/chromeos/components/quick_answers/public/mojom/spell_check.mojom new file mode 100644 index 00000000000000..93121c57188131 --- /dev/null +++ b/chromeos/components/quick_answers/public/mojom/spell_check.mojom @@ -0,0 +1,29 @@ +// 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. + +module quick_answers.mojom; + +import "mojo/public/mojom/base/read_only_file.mojom"; +import "sandbox/policy/mojom/sandbox.mojom"; + +// Provides a way to query hunspell in a sandboxed utility process, since +// the inputs may be untrustworthy and hunspell library is somewhat prone +// to crashes. +[ServiceSandbox=sandbox.mojom.Sandbox.kService] +interface SpellCheckService { + // Creates a new SpellCheckerDictionary instance from |dictionary_file|. + // If hunspell initialization failed, returns a null remote. + // Can be called multiple times if the dictionary file changes or the + // previous call did not success. + CreateDictionary(mojo_base.mojom.ReadOnlyFile dictionary_file) + => (pending_remote? dictionary); +}; + +// Handles spell check requests for a hunspell dictionary loaded via +// |CreateDictionary()|. +interface SpellCheckDictionary { + // Check spelling of the given word, |correctness| is true if the word is + // spelled correctly. + CheckSpelling(string word) => (bool correctness); +};