Add compose error case for when the optimization guide service is una…

…vailable. Bug: b/302748108 Change-Id: I374af3abf163ecd4a1708aeb145069c4ee29e04e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4937619 Commit-Queue: Anthony Cui <cuianthony@chromium.org> Auto-Submit: Justin DeWitt <dewittj@chromium.org> Reviewed-by: Anthony Cui <cuianthony@chromium.org> Cr-Commit-Position: refs/heads/main@{#1209552}
Denger-Network · Oct 13, 2023 · 08857b9 · 08857b9
1 parent 2a01adf
commit 08857b9
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 12 deletions.
diff --git a/chrome/browser/compose/chrome_compose_client.cc b/chrome/browser/compose/chrome_compose_client.cc
@@ -19,8 +19,10 @@
 #include "components/compose/proto/compose_metadata.pb.h"
 #include "components/optimization_guide/core/optimization_guide_features.h"
 #include "components/optimization_guide/core/optimization_guide_util.h"
+#include "components/strings/grit/components_strings.h"
 #include "content/public/browser/web_contents_user_data.h"
 #include "mojo/public/cpp/bindings/callback_helpers.h"
+#include "ui/base/l10n/l10n_util.h"
 
 ChromeComposeClient::ChromeComposeClient(content::WebContents* web_contents)
     : content::WebContentsUserData<ChromeComposeClient>(*web_contents),
@@ -43,14 +45,16 @@ void ChromeComposeClient::Compose(compose::mojom::StyleModifiersPtr style,
                                   const std::string& input,
                                   ComposeCallback callback) {
   // TODO(b/300974056): Move this to the overall feature-enabled check.
-  if (!base::FeatureList::IsEnabled(
+  auto* model_executor = GetModelExecutor();
+  if (!model_executor ||
+      !base::FeatureList::IsEnabled(
           optimization_guide::features::kOptimizationGuideModelExecution)) {
     std::move(callback).Run(compose::mojom::ComposeResponse::New(
-        compose::mojom::ComposeStatus::kError, ""));
+        compose::mojom::ComposeStatus::kError,
+        l10n_util::GetStringUTF8(IDS_COMPOSE_CONFIGURATION_ERROR)));
     return;
   }
-  auto* model_executor = GetModelExecutor();
-  DCHECK(model_executor) << "Unable to acquire model executor.";
+
   compose_proto::ComposeRequest request;
   request.set_user_input(input);
   request.set_tone(ComposeTone(style->tone));
@@ -105,17 +109,13 @@ compose::ComposeManager& ChromeComposeClient::GetManager() {
 
 optimization_guide::OptimizationGuideModelExecutor*
 ChromeComposeClient::GetModelExecutor() {
-  if (model_executor_for_test_) {
-    return model_executor_for_test_;
-  }
-
-  return OptimizationGuideKeyedServiceFactory::GetForProfile(
-      Profile::FromBrowserContext(GetWebContents().GetBrowserContext()));
+  return model_executor_for_test_.value_or(
+      OptimizationGuideKeyedServiceFactory::GetForProfile(
+          Profile::FromBrowserContext(GetWebContents().GetBrowserContext())));
 }
 
 void ChromeComposeClient::SetModelExecutorForTest(
     optimization_guide::OptimizationGuideModelExecutor* model_executor) {
-  CHECK(model_executor);
   model_executor_for_test_ = model_executor;
 }
 

diff --git a/chrome/browser/compose/chrome_compose_client.h b/chrome/browser/compose/chrome_compose_client.h
@@ -70,7 +70,7 @@ class ChromeComposeClient
       handler_receiver_;
   std::unique_ptr<mojo::Remote<compose::mojom::ComposeDialog>> dialog_remote_;
 
-  raw_ptr<optimization_guide::OptimizationGuideModelExecutor>
+  std::optional<optimization_guide::OptimizationGuideModelExecutor*>
       model_executor_for_test_;
 
   // The unique renderer ID of the last field the user selected compose on.

diff --git a/chrome/browser/compose/chrome_compose_client_unittest.cc b/chrome/browser/compose/chrome_compose_client_unittest.cc
@@ -189,3 +189,15 @@ TEST_F(ChromeComposeClientTest, TestOptimizationGuideDisabled) {
   compose::mojom::ComposeResponsePtr result = test_future.Take();
   EXPECT_EQ(compose::mojom::ComposeStatus::kError, result->status);
 }
+
+TEST_F(ChromeComposeClientTest, TestNoModelExecutor) {
+  client().SetModelExecutorForTest(nullptr);
+  EXPECT_CALL(model_executor(), ExecuteModel(_, _, _)).Times(0);
+  base::test::TestFuture<compose::mojom::ComposeResponsePtr> test_future;
+  auto style_modifiers = compose::mojom::StyleModifiers::New();
+  page_handler()->Compose(std::move(style_modifiers), "a user typed this",
+                          test_future.GetCallback());
+
+  compose::mojom::ComposeResponsePtr result = test_future.Take();
+  EXPECT_EQ(compose::mojom::ComposeStatus::kError, result->status);
+}
diff --git a/components/compose_strings.grdp b/components/compose_strings.grdp
@@ -7,4 +7,7 @@
   <message name="IDS_COMPOSE_SUGGESTION_LABEL" desc="The label beneath the main text in the Autofill suggestion that a user sees adter interacting with a supported text element." translateable="false">
   Powered by Google
   </message>
+  <message name="IDS_COMPOSE_CONFIGURATION_ERROR" desc="The error text shown when Compose is not available due to an internal configuration error." translateable="false">
+  Configuration error. Try again later.
+  </message>
 </grit-part>