Udpate Settings mojom API for UpdateSettings request

Add UpdateSettings API for update settings ui request.

Bug: 819782
Test: Manual Test
Change-Id: Ibd7c303da98f0bc95b53ac08c365cc4276ac605f
Reviewed-on: https://chromium-review.googlesource.com/1016039
Commit-Queue: Yue Li <updowndota@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553873}

chromeos/services/assistant/assistant_manager_service.h

@@ -47,11 +47,18 @@ class AssistantManagerService : public mojom::Assistant {
   virtual AssistantSettingsManager* GetAssistantSettingsManager() = 0;
 
   using GetSettingsUiResponseCallback =
-      base::RepeatingCallback<void(const std::string&)>;
+      base::OnceCallback<void(const std::string&)>;
   // Send request for getting settings ui.
   virtual void SendGetSettingsUiRequest(
       const std::string& selector,
       GetSettingsUiResponseCallback callback) = 0;
+
+  using UpdateSettingsUiResponseCallback =
+      base::OnceCallback<void(const std::string&)>;
+  // Send request for updating settings ui.
+  virtual void SendUpdateSettingsUiRequest(
+      const std::string& update,
+      UpdateSettingsUiResponseCallback callback) = 0;
 };
 
 }  // namespace assistant

chromeos/services/assistant/assistant_manager_service_impl.cc

@@ -78,7 +78,9 @@ void AssistantManagerServiceImpl::SendGetSettingsUiRequest(
   std::string serialized_proto = SerializeGetSettingsUiRequest(selector);
   assistant_manager_internal_->SendGetSettingsUiRequest(
       serialized_proto, std::string(), [
-        callback, weak_ptr = weak_factory_.GetWeakPtr(),
+        repeating_callback =
+            base::AdaptCallbackForRepeating(std::move(callback)),
+        weak_ptr = weak_factory_.GetWeakPtr(),
         task_runner = main_thread_task_runner_
       ](const assistant_client::VoicelessResponse& response) {
         // This callback may be called from server multiple times. We should
@@ -89,7 +91,31 @@ void AssistantManagerServiceImpl::SendGetSettingsUiRequest(
               FROM_HERE,
               base::BindOnce(
                   &AssistantManagerServiceImpl::HandleGetSettingsResponse,
-                  std::move(weak_ptr), callback, settings));
+                  std::move(weak_ptr), repeating_callback, settings));
+        }
+      });
+}
+
+void AssistantManagerServiceImpl::SendUpdateSettingsUiRequest(
+    const std::string& update,
+    UpdateSettingsUiResponseCallback callback) {
+  std::string serialized_proto = SerializeUpdateSettingsUiRequest(update);
+  assistant_manager_internal_->SendUpdateSettingsUiRequest(
+      serialized_proto, std::string(), [
+        repeating_callback =
+            base::AdaptCallbackForRepeating(std::move(callback)),
+        weak_ptr = weak_factory_.GetWeakPtr(),
+        task_runner = main_thread_task_runner_
+      ](const assistant_client::VoicelessResponse& response) {
+        // This callback may be called from server multiple times. We should
+        // only process non-empty response.
+        std::string update = UnwrapUpdateSettingsUiResponse(response);
+        if (!update.empty()) {
+          task_runner->PostTask(
+              FROM_HERE,
+              base::BindOnce(
+                  &AssistantManagerServiceImpl::HandleUpdateSettingsResponse,
+                  std::move(weak_ptr), repeating_callback, update));
         }
       });
 }
@@ -231,11 +257,17 @@ std::string AssistantManagerServiceImpl::BuildUserAgent(
 }
 
 void AssistantManagerServiceImpl::HandleGetSettingsResponse(
-    GetSettingsUiResponseCallback callback,
+    base::RepeatingCallback<void(const std::string&)> callback,
     const std::string& settings) {
   callback.Run(settings);
 }
 
+void AssistantManagerServiceImpl::HandleUpdateSettingsResponse(
+    base::RepeatingCallback<void(const std::string&)> callback,
+    const std::string& result) {
+  callback.Run(result);
+}
+
 void AssistantManagerServiceImpl::OnConversationTurnStartedOnMainThread() {
   subscribers_.ForAllPtrs([](auto* ptr) { ptr->OnInteractionStarted(); });
 }

chromeos/services/assistant/assistant_manager_service_impl.h

@@ -52,6 +52,9 @@ class AssistantManagerServiceImpl
   void SendGetSettingsUiRequest(
       const std::string& selector,
       GetSettingsUiResponseCallback callback) override;
+  void SendUpdateSettingsUiRequest(
+      const std::string& update,
+      UpdateSettingsUiResponseCallback callback) override;
 
   // mojom::Assistant overrides:
   void SendTextQuery(const std::string& query) override;
@@ -85,8 +88,12 @@ class AssistantManagerServiceImpl
       assistant_client::AssistantManager* assistant_manager);
   std::string BuildUserAgent(const std::string& arc_version) const;
 
-  void HandleGetSettingsResponse(GetSettingsUiResponseCallback callback,
-                                 const std::string& settings);
+  void HandleGetSettingsResponse(
+      base::RepeatingCallback<void(const std::string&)> callback,
+      const std::string& settings);
+  void HandleUpdateSettingsResponse(
+      base::RepeatingCallback<void(const std::string&)> callback,
+      const std::string& result);
 
   void OnConversationTurnStartedOnMainThread();
   void OnConversationTurnFinishedOnMainThread(

chromeos/services/assistant/assistant_settings_manager_impl.cc

@@ -24,8 +24,19 @@ void AssistantSettingsManagerImpl::GetSettings(const std::string& selector,
          AssistantManagerService::State::RUNNING);
   // Wraps the callback into a repeating callback since the server side
   // interface requires the callback to be copyable.
-  assistant_manager_service_->SendGetSettingsUiRequest(
-      selector, base::AdaptCallbackForRepeating(std::move(callback)));
+  assistant_manager_service_->SendGetSettingsUiRequest(selector,
+                                                       std::move(callback));
+}
+
+void AssistantSettingsManagerImpl::UpdateSettings(
+    const std::string& update,
+    GetSettingsCallback callback) {
+  DCHECK(assistant_manager_service_->GetState() ==
+         AssistantManagerService::State::RUNNING);
+  // Wraps the callback into a repeating callback since the server side
+  // interface requires the callback to be copyable.
+  assistant_manager_service_->SendUpdateSettingsUiRequest(update,
+                                                          std::move(callback));
 }
 
 }  // namespace assistant

chromeos/services/assistant/assistant_settings_manager_impl.h

@@ -26,6 +26,8 @@ class AssistantSettingsManagerImpl : public AssistantSettingsManager {
   // mojom::AssistantSettingsManager overrides:
   void GetSettings(const std::string& selector,
                    GetSettingsCallback callback) override;
+  void UpdateSettings(const std::string& update,
+                      UpdateSettingsCallback callback) override;
 
   // AssistantSettingsManager overrides:
   void BindRequest(mojom::AssistantSettingsManagerRequest request) override;

chromeos/services/assistant/fake_assistant_manager_service_impl.cc

@@ -38,6 +38,10 @@ void FakeAssistantManagerServiceImpl::SendGetSettingsUiRequest(
     const std::string& selector,
     GetSettingsUiResponseCallback callback) {}
 
+void FakeAssistantManagerServiceImpl::SendUpdateSettingsUiRequest(
+    const std::string& update,
+    UpdateSettingsUiResponseCallback callback) {}
+
 void FakeAssistantManagerServiceImpl::SendTextQuery(const std::string& query) {}
 
 void FakeAssistantManagerServiceImpl::AddAssistantEventSubscriber(

chromeos/services/assistant/fake_assistant_manager_service_impl.h

@@ -32,6 +32,9 @@ class FakeAssistantManagerServiceImpl : public AssistantManagerService {
   void SendGetSettingsUiRequest(
       const std::string& selector,
       GetSettingsUiResponseCallback callback) override;
+  void SendUpdateSettingsUiRequest(
+      const std::string& update,
+      UpdateSettingsUiResponseCallback callback) override;
 
   // mojom::AssistantEvent overrides:
   void SendTextQuery(const std::string& query) override;

chromeos/services/assistant/public/mojom/settings.mojom

@@ -11,4 +11,11 @@ interface AssistantSettingsManager {
   // sub-pages requested.
   // Send a request for the settings ui sub-pages indicated by the |selector|.
   GetSettings(string selector) => (string settings);
+
+  // |update| is a serialized proto of SettingsUiUpdate, indicating what kind
+  // of updates should be applied to the settings.
+  // |result| is a serialized proto of SettingsUiUpdateResult, containing the
+  // result of updates.
+  // Send a request to update the assistant settings indicated by the |update|.
+  UpdateSettings(string update) => (string result);
 };

chromeos/services/assistant/public/proto/about_me_settings_ui.proto

@@ -25,6 +25,18 @@ message AboutMeSettingsUi {
   optional string full_name = 7;
 }
 
+// Only the provided fields will be updated.
+message AboutMeSettingsUiUpdate {
+  optional string name = 1;
+  optional string name_pronunciation = 2;
+  optional WeatherUnit weather_unit = 3;
+}
+
+message AboutMeSettingsUiUpdateResult {
+  // Localized message to display to the users regarding email optin status.
+  optional string email_optin_message = 2;
+}
+
 enum WeatherUnit {
   CELSIUS = 0;
   FAHRENHEIT = 1;

chromeos/services/assistant/public/proto/settings_ui.proto

@@ -15,6 +15,14 @@ message SettingsUi {
   optional AboutMeSettingsUi about_me_settings = 5;
 }
 
+message SettingsUiUpdate {
+  optional AboutMeSettingsUiUpdate about_me_settings_update = 4;
+}
+
+message SettingsUiUpdateResult {
+  optional AboutMeSettingsUiUpdateResult about_me_settings_update_result = 4;
+}
+
 // Determines which settings sub-pages should be requested to the server.
 message SettingsUiSelector {
   optional bool about_me_settings = 5;

chromeos/services/assistant/service.cc

@@ -179,15 +179,6 @@ void Service::GetAccessTokenCallback(const base::Optional<std::string>& token,
     assistant_manager_service_->SetAccessToken(token.value());
   }
 
-  if (!assistant_settings_manager_) {
-    assistant_settings_manager_ =
-        assistant_manager_service_.get()->GetAssistantSettingsManager();
-#if BUILDFLAG(ENABLE_CROS_LIBASSISTANT)
-    registry_.AddInterface<mojom::AssistantSettingsManager>(base::BindRepeating(
-        &Service::BindAssistantSettingsManager, base::Unretained(this)));
-#endif
-  }
-
   token_refresh_timer_->Start(FROM_HERE, expiration_time - base::Time::Now(),
                               this, &Service::RequestAccessToken);
 }
@@ -209,6 +200,11 @@ void Service::FinalizeAssistantManangerService() {
       &Service::BindAssistantConnection, base::Unretained(this)));
   client_->OnAssistantStatusChanged(true);
   DVLOG(1) << "Assistant is running";
+
+  assistant_settings_manager_ =
+      assistant_manager_service_.get()->GetAssistantSettingsManager();
+  registry_.AddInterface<mojom::AssistantSettingsManager>(base::BindRepeating(
+      &Service::BindAssistantSettingsManager, base::Unretained(this)));
 }
 
 void Service::AddAshSessionObserver() {