Add AcceptComposeResult hooks

Add a new method to ComposeDialogPageHandler to accept the compose
result and run the Autofill callback.

This CL removes the ComposeDialogCallback which is no longer used and
replaced by AcceptComposeResult.


Bug: b:301370241
Change-Id: Icae4248c741cb25560638d95552fb4c9ee0b5d5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4936783
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: edchin <edchin@google.com>
Commit-Queue: Trevor Perrier <perrier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1212521}

chrome/browser/compose/chrome_compose_client.cc

@@ -82,8 +82,9 @@ void ChromeComposeClient::ShowComposeDialog(
     const autofill::FormFieldData& trigger_field,
     std::optional<autofill::AutofillClient::PopupScreenLocation>
         popup_screen_location,
-    ComposeDialogCallback callback) {
-  SaveFieldAndCreateComposeStateIfEmpty(trigger_field.global_id());
+    ComposeCallback callback) {
+  SaveFieldAndCreateComposeStateIfEmpty(trigger_field.global_id(),
+                                        std::move(callback));
   if (!skip_show_dialog_for_test_) {
     // The bounds given by autofill are relative to the top level frame. Here we
     // offset by the WebContents container to make up for that.
@@ -96,17 +97,21 @@ void ChromeComposeClient::ShowComposeDialog(
 }
 
 void ChromeComposeClient::SaveFieldAndCreateComposeStateIfEmpty(
-    const autofill::FieldGlobalId& field_id) {
+    const autofill::FieldGlobalId& field_id,
+    ComposeCallback callback) {
   last_compose_field_id_ = field_id;
   auto it = sessions_.find(last_compose_field_id_);
   if (it != sessions_.end()) {
-    // Already have a session for this field ID.
+    // Update existing session
+    auto& existing_session = *it->second;
+    existing_session.SetComposeResultCallback(std::move(callback));
     return;
   }
 
   sessions_.emplace(
       last_compose_field_id_,
-      std::make_unique<ComposeSession>(&GetWebContents(), GetModelExecutor()));
+      std::make_unique<ComposeSession>(&GetWebContents(), GetModelExecutor(),
+                                       std::move(callback)));
 }
 
 compose::ComposeManager& ChromeComposeClient::GetManager() {

chrome/browser/compose/chrome_compose_client.h

@@ -46,7 +46,7 @@ class ChromeComposeClient
       const autofill::FormFieldData& trigger_field,
       std::optional<autofill::AutofillClient::PopupScreenLocation>
           popup_screen_location,
-      ComposeDialogCallback callback) override;
+      ComposeCallback callback) override;
 
   void BindComposeDialog(
       mojo::PendingReceiver<compose::mojom::ComposeDialogPageHandler> handler,
@@ -74,7 +74,8 @@ class ChromeComposeClient
 
   // Creates a compose state for `field_id` if it does not exist.
   void SaveFieldAndCreateComposeStateIfEmpty(
-      const autofill::FieldGlobalId& field_id);
+      const autofill::FieldGlobalId& field_id,
+      ComposeCallback callback);
 
   compose::ComposeManagerImpl manager_;
 

chrome/browser/compose/compose_session.cc

@@ -31,11 +31,13 @@
 
 ComposeSession::ComposeSession(
     content::WebContents* web_contents,
-    optimization_guide::OptimizationGuideModelExecutor* executor)
+    optimization_guide::OptimizationGuideModelExecutor* executor,
+    ComposeCallback callback)
     : executor_(executor),
       handler_receiver_(this),
       web_contents_(web_contents),
       weak_ptr_factory_(this) {
+  callback_ = std::move(callback);
   state_ = compose::mojom::ComposeState::New();
   state_->style = compose::mojom::StyleModifiers::New();
 }
@@ -135,3 +137,9 @@ void ComposeSession::RequestInitialState(RequestInitialStateCallback callback) {
 void ComposeSession::SaveWebUIState(const std::string& webui_state) {
   state_->webui_state = webui_state;
 }
+
+void ComposeSession::AcceptComposeResult() {
+  CHECK(state_->response->status != compose::mojom::ComposeStatus::kOk);
+  std::move(callback_).Run(base::UTF8ToUTF16(state_->response->result));
+  // TODO(b/301370241): Make sure the WebUI or browser calls CloseUI.
+}

chrome/browser/compose/compose_session.h

@@ -36,8 +36,13 @@ class WebContents;
 //  constructor, and the `executor` MUST outlive that WebContents.
 class ComposeSession : public compose::mojom::ComposeDialogPageHandler {
  public:
+  // The callback to Autofill. When run, it fills the passed string into the
+  // form field on which it was triggered.
+  using ComposeCallback = base::OnceCallback<void(const std::u16string&)>;
+
   ComposeSession(content::WebContents* web_contents,
-                 optimization_guide::OptimizationGuideModelExecutor* executor);
+                 optimization_guide::OptimizationGuideModelExecutor* executor,
+                 ComposeCallback callback = base::NullCallback());
   ~ComposeSession() override;
 
   // Binds this to a Compose webui.
@@ -61,6 +66,15 @@ class ComposeSession : public compose::mojom::ComposeDialogPageHandler {
   // disk or processed by the Browser Process at all.
   void SaveWebUIState(const std::string& webui_state) override;
 
+  // Indicates that the compose result should be accepted by Autofill.
+  void AcceptComposeResult() override;
+
+  // Non-ComposeDialogPageHandler Methods
+
+  void SetComposeResultCallback(ComposeCallback callback) {
+    callback_ = std::move(callback);
+  }
+
  private:
   void ProcessError(const std::string& message);
   void SaveNewComposeRequest(compose::mojom::StyleModifiersPtr style);
@@ -81,6 +95,9 @@ class ComposeSession : public compose::mojom::ComposeDialogPageHandler {
   // `this`.
   raw_ptr<content::WebContents> web_contents_;
 
+  // A callback to Autofill that triggers filling the field.
+  ComposeCallback callback_;
+
   base::WeakPtrFactory<ComposeSession> weak_ptr_factory_;
 };
 

chrome/browser/resources/compose/compose_api_proxy.ts

@@ -8,6 +8,7 @@ import {ComposeDialogCallbackRouter, ComposeDialogPageHandlerFactory, ComposeDia
 export interface ComposeApiProxy {
   compose(style: StyleModifiers, input: string): void;
   getRouter(): ComposeDialogCallbackRouter;
+  acceptComposeResult(): void;
 }
 
 export class ComposeApiProxyImpl implements ComposeApiProxy {
@@ -43,4 +44,9 @@ export class ComposeApiProxyImpl implements ComposeApiProxy {
   getRouter() {
     return this.router;
   }
+
+  /** @override */
+  acceptComposeResult() {
+    this.composeDialogPageHandler.acceptComposeResult();
+  }
 }

chrome/common/compose/compose.mojom

@@ -87,6 +87,10 @@ interface ComposeDialogPageHandler {
   // unsafe to parse in the browser process.
   SaveWebUIState(string webui_state);
 
+  // Inform the Browser that the response has been accepted by the user.
+  // Should only be called if the ComposeResponse received is valid.
+  AcceptComposeResult();
+
   // Asks the native handler for state information needed for opening the
   // compose dialog for the last field the user selected compose on.
   RequestInitialState() => (OpenMetadata initial_state);

chrome/test/data/webui/compose/compose_app_test.ts

@@ -37,6 +37,8 @@ class TestingApiProxy extends TestBrowserProxy implements ComposeApiProxy {
   setComposeOutput(output: string) {
     this.composeOutput_ = output;
   }
+
+  acceptComposeResult() {}
 }
 
 suite('ComposeApp', () => {

components/compose/core/browser/compose_client.h

@@ -19,24 +19,21 @@ class ComposeManager;
 // An interface for embedder actions, e.g. Chrome on Desktop.
 class ComposeClient {
  public:
-  // TODO(b/301609035): Add more parameters and potentially move to delegate or
-  // service class.
-  struct QueryParams {
-    std::u16string query;
-  };
+  // The callback to Autofill. When run, it fills the passed string into the
+  // form field on which it was triggered.
+  using ComposeCallback = base::OnceCallback<void(const std::u16string&)>;
 
   virtual ~ComposeClient() = default;
 
   // Returns the `ComposeManager` associated with this client.
   virtual ComposeManager& GetManager() = 0;
 
-  using ComposeDialogCallback = base::OnceCallback<void(const QueryParams&)>;
   virtual void ShowComposeDialog(
       autofill::AutofillComposeDelegate::UiEntryPoint ui_entry_point,
       const autofill::FormFieldData& trigger_field,
       std::optional<autofill::AutofillClient::PopupScreenLocation>
           popup_screen_location,
-      ComposeDialogCallback callback) = 0;
+      ComposeCallback callback) = 0;
 };
 
 }  // namespace compose

components/compose/core/browser/compose_manager_impl.cc

@@ -54,17 +54,8 @@ void ComposeManagerImpl::OpenCompose(
     std::optional<PopupScreenLocation> popup_screen_location,
     ComposeCallback callback) {
   CHECK(IsEnabled());
-  // TODO(b/301609035): Either pass a weak pointer or make sure that
-  // the dialog never outlives the tab. (Should be a given once the
-  // bubble destroys itself prior to WebContents destruction.)
   client_->ShowComposeDialog(ui_entry_point, trigger_field,
-                             popup_screen_location,
-                             base::BindOnce(
-                                 [](ComposeCallback callback,
-                                    const ComposeClient::QueryParams& params) {
-                                   std::move(callback).Run(params.query);
-                                 },
-                                 std::move(callback)));
+                             popup_screen_location, std::move(callback));
 }
 
 void ComposeManagerImpl::OpenComposeFromContextMenuCallback(