Remove dependency of SupplementalModel from EngineInterface.

Changes the owner of supplemental model from SessionHandler to Engine.
Remove spellchecker operations as it is no longer used.

PiperOrigin-RevId: 703360331

src/engine/BUILD.bazel

@@ -32,6 +32,7 @@ load(
     "mozc_cc_library",
     "mozc_cc_test",
     "mozc_select",
+    "mozc_select_enable_supplemental_model",
 )
 
 package(default_visibility = ["//:__subpackages__"])
@@ -82,7 +83,6 @@ mozc_cc_library(
     name = "engine_interface",
     hdrs = ["engine_interface.h"],
     deps = [
-        ":supplemental_model_interface",
         "//converter:converter_interface",
         "//protocol:engine_builder_cc_proto",
         "@com_google_absl//absl/status",
@@ -204,7 +204,9 @@ mozc_cc_library(
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
-    ],
+    ] + mozc_select_enable_supplemental_model([
+        "//supplemental_model:supplemental_model_factory",
+    ]),
 )
 
 mozc_cc_test(
@@ -234,7 +236,6 @@ mozc_cc_library(
     hdrs = ["engine_mock.h"],
     deps = [
         ":engine_interface",
-        ":supplemental_model_interface",
         "//converter:converter_interface",
         "//testing:gunit",
         "@com_google_absl//absl/status",

src/engine/engine.cc

@@ -54,6 +54,7 @@
 #include "rewriter/rewriter.h"
 #include "rewriter/rewriter_interface.h"
 
+
 namespace mozc {
 
 absl::StatusOr<std::unique_ptr<Engine>> Engine::CreateDesktopEngine(
@@ -117,12 +118,8 @@ absl::Status Engine::Init(std::unique_ptr<engine::Modules> modules,
 
   RETURN_IF_NULL(modules);
 
-  // Keeps the previous supplemental_model if exists.
-  const engine::SupplementalModelInterface *supplemental_model =
-      modules_->GetSupplementalModel();
-
   modules_ = std::move(modules);
-  modules_->SetSupplementalModel(supplemental_model);
+
 
   immutable_converter_ = std::make_unique<ImmutableConverter>(*modules_);
   RETURN_IF_NULL(immutable_converter_);
@@ -284,4 +281,12 @@ bool Engine::SendEngineReloadRequest(const EngineReloadRequest &request) {
   return true;
 }
 
+bool Engine::SendSupplementalModelReloadRequest(
+    const EngineReloadRequest &request) {
+  if (modules_ && modules_->GetSupplementalModel()) {
+    modules_->GetMutableSupplementalModel()->LoadAsync(request);
+  }
+  return true;
+}
+
 }  // namespace mozc

src/engine/engine.h

@@ -137,17 +137,14 @@ class Engine : public EngineInterface {
                         : minimal_engine_.GetPosList();
   }
 
-  void SetSupplementalModel(
-      const engine::SupplementalModelInterface *supplemental_model) override {
-    modules_->SetSupplementalModel(supplemental_model);
-  }
-
   // For testing only.
   engine::Modules *GetModulesForTesting() const { return modules_.get(); }
 
   // Maybe reload a new data manager. Returns true if reloaded.
   bool MaybeReloadEngine(EngineReloadResponse *response) override;
   bool SendEngineReloadRequest(const EngineReloadRequest &request) override;
+  bool SendSupplementalModelReloadRequest(
+      const EngineReloadRequest &request) override;
 
   void SetDataLoaderForTesting(std::unique_ptr<DataLoader> loader) {
     loader_ = std::move(loader);
@@ -170,6 +167,7 @@ class Engine : public EngineInterface {
   std::unique_ptr<DataLoader> loader_;
   std::unique_ptr<engine::Modules> modules_;
   std::unique_ptr<ImmutableConverterInterface> immutable_converter_;
+  std::unique_ptr<engine::SupplementalModelInterface> supplemental_model_;
 
   // TODO(noriyukit): Currently predictor and rewriter are created by this class
   // but owned by converter_. Since this class creates these two, it'd be better

src/engine/engine_interface.h

@@ -37,7 +37,6 @@
 #include "absl/status/status.h"
 #include "absl/strings/string_view.h"
 #include "converter/converter_interface.h"
-#include "engine/supplemental_model_interface.h"
 #include "protocol/engine_builder.pb.h"
 
 namespace mozc {
@@ -93,16 +92,17 @@ class EngineInterface {
   // Gets the user POS list.
   virtual std::vector<std::string> GetPosList() const { return {}; }
 
-  virtual void SetSupplementalModel(
-      const engine::SupplementalModelInterface *supplemental_model) {}
-
   // Maybe reload a new data manager. Returns true if reloaded.
   virtual bool MaybeReloadEngine(EngineReloadResponse *response) {
     return false;
   }
   virtual bool SendEngineReloadRequest(const EngineReloadRequest &request) {
     return false;
   }
+  virtual bool SendSupplementalModelReloadRequest(
+      const EngineReloadRequest &request) {
+    return false;
+  }
 
  protected:
   EngineInterface() = default;

src/engine/engine_mock.h

@@ -38,7 +38,6 @@
 #include "absl/strings/string_view.h"
 #include "converter/converter_interface.h"
 #include "engine/engine_interface.h"
-#include "engine/supplemental_model_interface.h"
 #include "testing/gmock.h"
 
 namespace mozc {
@@ -56,8 +55,6 @@ class MockEngine : public EngineInterface {
   MOCK_METHOD(bool, ClearUnusedUserPrediction, (), (override));
   MOCK_METHOD(bool, ReloadAndWait, (), (override));
   MOCK_METHOD(std::vector<std::string>, GetPosList, (), (const, override));
-  MOCK_METHOD(void, SetSupplementalModel,
-              (const engine::SupplementalModelInterface *), (override));
 };
 
 }  // namespace mozc

src/engine/engine_test.cc

@@ -113,12 +113,10 @@ class EngineTest : public ::testing::Test {
 };
 
 TEST_F(EngineTest, ReloadModulesTest) {
+  auto modules = std::make_unique<engine::Modules>();
   SupplementalModelForTesting supplemental_model;
-  engine_->SetSupplementalModel(&supplemental_model);
-  EXPECT_EQ(engine_->GetModulesForTesting()->GetSupplementalModel(),
-            &supplemental_model);
+  modules->SetSupplementalModel(&supplemental_model);
 
-  auto modules = std::make_unique<engine::Modules>();
   CHECK_OK(modules->Init(std::make_unique<testing::MockDataManager>()));
 
   const bool is_mobile = true;

src/engine/modules.h

@@ -116,8 +116,13 @@ class Modules {
   const engine::SupplementalModelInterface *GetSupplementalModel() const {
     return supplemental_model_;
   }
+
+  engine::SupplementalModelInterface *GetMutableSupplementalModel() {
+    return supplemental_model_;
+  }
+
   void SetSupplementalModel(
-      const engine::SupplementalModelInterface *supplemental_model) {
+      engine::SupplementalModelInterface *supplemental_model) {
     supplemental_model_ = supplemental_model;
   }
 
@@ -137,11 +142,8 @@ class Modules {
       single_kanji_prediction_aggregator_;
   ZeroQueryDict zero_query_dict_;
   ZeroQueryDict zero_query_number_dict_;
-
-  // SupplementalModel used for homonym correction.
-  // Module doesn't have the ownership of supplemental_model_,
-  // SessionHandler owns this this instance. (usually a singleton object).
-  const engine::SupplementalModelInterface *supplemental_model_ = nullptr;
+  // The owner of supplemental_model_ is Engine.
+  engine::SupplementalModelInterface *supplemental_model_ = nullptr;
 };
 
 }  // namespace engine

src/prediction/dictionary_prediction_aggregator_test.cc

@@ -426,7 +426,7 @@ class MockDataAndAggregator {
     return *aggregator_;
   }
   void set_supplemental_model(
-      const engine::SupplementalModelInterface *supplemental_model) {
+      engine::SupplementalModelInterface *supplemental_model) {
     modules_.SetSupplementalModel(supplemental_model);
   }
 
@@ -1619,13 +1619,11 @@ TEST_F(DictionaryPredictionAggregatorTest,
   Segment *segment = segments.add_segment();
 
   constexpr size_t kMaxSize = 100;
-  const ConversionRequest suggestion_convreq =
-      CreateConversionRequest(
+  const ConversionRequest suggestion_convreq = CreateConversionRequest(
       {.request_type = ConversionRequest::SUGGESTION,
        .max_dictionary_prediction_candidates_size = kMaxSize});
-      CreateSuggestionConversionRequest();
-  const ConversionRequest prediction_convreq =
-      CreateConversionRequest(
+  CreateSuggestionConversionRequest();
+  const ConversionRequest prediction_convreq = CreateConversionRequest(
       {.request_type = ConversionRequest::PREDICTION,
        .max_dictionary_prediction_candidates_size = kMaxSize});
 

src/prediction/user_history_predictor_test.cc

@@ -442,7 +442,7 @@ class UserHistoryPredictorTest : public testing::TestWithTempUserProfile {
   }
 
   void SetSupplementalModel(
-      const engine::SupplementalModelInterface *supplemental_model) {
+      engine::SupplementalModelInterface *supplemental_model) {
     data_and_predictor_->modules.SetSupplementalModel(supplemental_model);
   }
 

src/session/BUILD.bazel

@@ -313,8 +313,6 @@ mozc_cc_library(
     ] + mozc_select_enable_session_watchdog([
         "//base:process",
         ":session_watch_dog",
-    ]) + mozc_select_enable_supplemental_model([
-        "//supplemental_model:supplemental_model_factory",
     ]),
 )
 

src/session/session_handler.cc

@@ -53,7 +53,6 @@
 #include "config/config_handler.h"
 #include "dictionary/user_dictionary_session_handler.h"
 #include "engine/engine_interface.h"
-#include "engine/supplemental_model_interface.h"
 #include "protocol/commands.pb.h"
 #include "protocol/config.pb.h"
 #include "protocol/engine_builder.pb.h"
@@ -70,7 +69,6 @@
 #include "session/session_watch_dog.h"
 #endif  // MOZC_DISABLE_SESSION_WATCHDOG
 
-
 // TODO(b/275437228): Convert this to use `absl::Duration`. Note that existing
 // clients assume a negative value means we do not timeout at all.
 ABSL_FLAG(int32_t, timeout, -1,
@@ -169,7 +167,6 @@ SessionHandler::SessionHandler(std::unique_ptr<EngineInterface> engine)
     return;
   }
 
-
   // Everything is OK.
   is_available_ = true;
 }
@@ -372,9 +369,6 @@ bool SessionHandler::EvalCommand(commands::Command *command) {
     case commands::Input::NO_OPERATION:
       eval_succeeded = NoOperation(command);
       break;
-    case commands::Input::CHECK_SPELLING:
-      eval_succeeded = CheckSpelling(command);
-      break;
     case commands::Input::RELOAD_SPELL_CHECKER:
       eval_succeeded = ReloadSupplementalModel(command);
       break;
@@ -673,29 +667,16 @@ bool SessionHandler::SendEngineReloadRequest(commands::Command *command) {
 
 bool SessionHandler::NoOperation(commands::Command *command) { return true; }
 
-bool SessionHandler::CheckSpelling(commands::Command *command) {
-  if (!command->input().has_check_spelling_request() ||
-      command->input().check_spelling_request().text().empty()) {
-    return true;
-  }
-
-  if (supplemental_model_) {
-    auto response = supplemental_model_->CheckSpelling(
-        command->input().check_spelling_request());
-    auto *stored = command->mutable_output()->mutable_check_spelling_response();
-    if (response.has_value()) {
-      *stored = std::move(*response);
-    }
-  }
-
-  return true;
-}
-
 bool SessionHandler::ReloadSupplementalModel(commands::Command *command) {
-  if (supplemental_model_) {
-    supplemental_model_->LoadAsync(command->input().engine_reload_request());
+  if (!command->input().has_engine_reload_request()) {
+    return false;
+  }
+  if (!engine_->SendSupplementalModelReloadRequest(
+          command->input().engine_reload_request())) {
     return false;
   }
+  command->mutable_output()->mutable_engine_reload_response()->set_status(
+      EngineReloadResponse::ACCEPTED);
   return true;
 }
 

src/session/session_handler.h

@@ -42,7 +42,6 @@
 #include "composer/table.h"
 #include "dictionary/user_dictionary_session_handler.h"
 #include "engine/engine_interface.h"
-#include "engine/supplemental_model_interface.h"
 #include "protocol/commands.pb.h"
 #include "protocol/config.pb.h"
 #include "session/common.h"
@@ -89,7 +88,6 @@ class SessionHandler : public SessionHandlerInterface {
   FRIEND_TEST(SessionHandlerTest, KeyMapTest);
   FRIEND_TEST(SessionHandlerTest, EngineUpdateSuccessfulScenarioTest);
   FRIEND_TEST(SessionHandlerTest, EngineRollbackDataTest);
-  FRIEND_TEST(SessionHandlerTest, CheckSpellingTest);
 
   using SessionMap =
       mozc::storage::LruCache<SessionID, std::unique_ptr<session::Session>>;
@@ -129,7 +127,6 @@ class SessionHandler : public SessionHandlerInterface {
   bool SendUserDictionaryCommand(commands::Command *command);
   bool SendEngineReloadRequest(commands::Command *command);
   bool NoOperation(commands::Command *command);
-  bool CheckSpelling(commands::Command *command);
   bool ReloadSupplementalModel(commands::Command *command);
   bool GetServerVersion(commands::Command *command) const;
 
@@ -157,7 +154,6 @@ class SessionHandler : public SessionHandlerInterface {
   std::unique_ptr<const commands::Request> request_;
   std::unique_ptr<const config::Config> config_;
   std::unique_ptr<keymap::KeyMapManager> key_map_manager_;
-  std::unique_ptr<engine::SupplementalModelInterface> supplemental_model_;
 
   absl::BitGen bitgen_;
 };

src/session/session_handler_test.cc

@@ -58,7 +58,6 @@
 #include "engine/minimal_engine.h"
 #include "engine/mock_data_engine_factory.h"
 #include "engine/modules.h"
-#include "engine/supplemental_model_interface.h"
 #include "protocol/commands.pb.h"
 #include "protocol/config.pb.h"
 #include "session/internal/keymap.h"
@@ -69,7 +68,6 @@
 #include "testing/mozctest.h"
 #include "usage_stats/usage_stats_testing_util.h"
 
-
 ABSL_DECLARE_FLAG(int32_t, max_session_size);
 ABSL_DECLARE_FLAG(int32_t, create_session_min_interval);
 ABSL_DECLARE_FLAG(int32_t, last_command_timeout);
@@ -818,5 +816,4 @@ TEST_F(SessionHandlerTest, ReloadFromMinimalEngine) {
   EXPECT_EQ(handler.GetDataVersion(), mock_version_);
 }
 
-
 }  // namespace mozc