lp: Add InstallBasePack method to Language Packs Mojo API.

Add a separate method for installing the base pack for a language pack.
Unlike the original design, we will *not* automatically install the
base pack anymore when calling InstallLanguagePack. The base pack
must be installed manually via InstallBasePack.

This improves the ergonomics for clients that don't use base packs.
It is also difficult to misuse, as you cannot use a base pack
without the information returned by InstallBasePack.

Change-Id: I22bd529bf01ab9a5801db6b8063f7ffb0b1492a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3616009
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Michael Cui <mlcui@google.com>
Reviewed-by: Chad Duffin <chadduffin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1034945}

chromeos/language/language_packs/language_pack_manager.h

@@ -111,8 +111,6 @@ class LanguagePackManager : public DlcserviceClient::Observer {
   // Installs the Language Pack.
   // It takes a callback that will be triggered once the operation is done.
   // A state is passed to the callback.
-  // TODO(crbug.com/1320137): If |feature_id| has a corresponding Base Pack,
-  // then the Base Pack should be installed first.
   void InstallPack(const std::string& feature_id,
                    const std::string& locale,
                    OnInstallCompleteCallback callback);

chromeos/language/language_packs/language_packs_impl.cc

@@ -12,6 +12,7 @@
 
 namespace chromeos::language_packs {
 
+using ::chromeos::language::mojom::BasePackInfo;
 using ::chromeos::language::mojom::FeatureId;
 using ::chromeos::language::mojom::LanguagePackInfo;
 using ::chromeos::language::mojom::LanguagePacks;
@@ -63,6 +64,22 @@ void OnOperationComplete(LanguagePacksImpl::GetPackInfoCallback mojo_callback,
   std::move(mojo_callback).Run(std::move(info));
 }
 
+// Called when InstallBasePack() from Language Packs is complete.
+void OnInstallBasePackComplete(
+    LanguagePacksImpl::InstallBasePackCallback mojo_callback,
+    const PackResult& pack_result) {
+  auto info = BasePackInfo::New();
+  info->pack_state = GetPackStateFromStatusCode(pack_result.pack_state);
+  if (pack_result.pack_state == PackResult::INSTALLED) {
+    info->path = pack_result.path;
+  }
+
+  base::UmaHistogramEnumeration(
+      "ChromeOS.LanguagePacks.Mojo.BasePackStateResponse", info->pack_state);
+
+  std::move(mojo_callback).Run(std::move(info));
+}
+
 }  // namespace
 
 LanguagePacksImpl::LanguagePacksImpl() = default;
@@ -120,4 +137,23 @@ void LanguagePacksImpl::InstallPack(FeatureId feature_id,
   }
 }
 
+void LanguagePacksImpl::InstallBasePack(FeatureId feature_id,
+                                        InstallBasePackCallback mojo_callback) {
+  base::UmaHistogramEnumeration(
+      "ChromeOS.LanguagePacks.Mojo.InstallBasePack.Feature", feature_id);
+
+  LanguagePackManager* lp = LanguagePackManager::GetInstance();
+  const absl::optional<std::string> pack_id =
+      ConvertMojoFeatureToPackId(feature_id);
+
+  if (pack_id.has_value()) {
+    lp->InstallBasePack(*pack_id, base::BindOnce(&OnInstallBasePackComplete,
+                                                 std::move(mojo_callback)));
+  } else {
+    auto info = BasePackInfo::New();
+    info->pack_state = PackState::ERROR;
+    std::move(mojo_callback).Run(std::move(info));
+  }
+}
+
 }  // namespace chromeos::language_packs

chromeos/language/language_packs/language_packs_impl.h

@@ -35,6 +35,8 @@ class LanguagePacksImpl : public chromeos::language::mojom::LanguagePacks {
   void InstallPack(chromeos::language::mojom::FeatureId feature_id,
                    const std::string& language,
                    InstallPackCallback callback) override;
+  void InstallBasePack(chromeos::language::mojom::FeatureId feature_id,
+                       InstallBasePackCallback callback) override;
 
  private:
   mojo::ReceiverSet<chromeos::language::mojom::LanguagePacks> receivers_;

chromeos/language/public/mojom/language_packs.mojom

@@ -36,18 +36,24 @@ enum PackState {
   INSTALLED = 3
 };
 
-// This struct holds information that allows clients to use a Pack.
+// This struct holds information that allows clients to use a Language Pack.
 struct LanguagePackInfo {
   PackState pack_state;
   string path;
 };
 
+// This struct holds information that allows clients to use a Base Pack.
+struct BasePackInfo {
+  PackState pack_state;
+  string path;
+};
+
 // Interface for managing Language Packs.
 // Lives in the browser process and it allows clients to get information
 // about a Language Pack or to install one.
 // Language Packs are mounted to the user partition once they are installed and
 // this interface allows to get the path to the files.
-// Next ordinal: 2
+// Next ordinal: 3
 interface LanguagePacks {
   // Gets information about the current state of a Language Pack.
   // Takes the id of the feature (for example handwriting) and the language.
@@ -58,4 +64,13 @@ interface LanguagePacks {
   // Takes the id of the feature (for example handwriting) and the language.
   InstallPack@1(FeatureId feature_id, string language) =>
       (LanguagePackInfo info);
+
+  // Requests to install the Base Pack for a `feature_id`.
+  // The Base Pack contains the dependencies needed for a Language Pack to
+  // function correctly.
+  // Takes the id of the feature (for example handwriting).
+  // If there's no Base Pack for the specified feature, then `info` will have an
+  // ERROR PackState.
+  InstallBasePack@2(FeatureId feature_id) =>
+      (BasePackInfo info);
 };

tools/metrics/histograms/metadata/chromeos/histograms.xml

@@ -1183,6 +1183,16 @@ incoming reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="ChromeOS.LanguagePacks.Mojo.BasePackStateResponse"
+    enum="LanguagePackMojoPackState" expires_after="2022-12-11">
+  <owner>mlcui@google.com</owner>
+  <owner>cros-borders-eng@google.com</owner>
+  <summary>
+    Records the state of what state a base pack is in (whether it's installed or
+    not) when the language packs Mojo interface responds to an IPC.
+  </summary>
+</histogram>
+
 <histogram name="ChromeOS.LanguagePacks.Mojo.GetPackInfo.Feature"
     enum="LanguagePackMojoFeatureId" expires_after="2022-12-04">
   <owner>mlcui@google.com</owner>
@@ -1193,6 +1203,16 @@ incoming reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="ChromeOS.LanguagePacks.Mojo.InstallBasePack.Feature"
+    enum="LanguagePackMojoFeatureId" expires_after="2022-12-11">
+  <owner>mlcui@google.com</owner>
+  <owner>cros-borders-eng@google.com</owner>
+  <summary>
+    Records which feature was requested by callers of the language packs Mojo
+    interface's &quot;InstallBasePack&quot; IPC call.
+  </summary>
+</histogram>
+
 <histogram name="ChromeOS.LanguagePacks.Mojo.InstallPack.Feature"
     enum="LanguagePackMojoFeatureId" expires_after="2022-12-11">
   <owner>mlcui@google.com</owner>