ambient: fix weather info missing.

Previously we leave the weather settings as their default values on the
server side, which causes an unpredictable behavior on whether server
will return weather info to us in the response. This CL fixed this issue
by setting these values explicitly to notify server that we are always
expecting weather info to be included in the response.

Related CL:
https://chrome-internal-review.googlesource.com/c/chrome/assistant/+/3215310

Bug: b/158630188
Test: manually
Change-Id: I0e38e9fa46d22e36a6e5f6e2f42f84818acea433
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2363914
Commit-Queue: Meilin Wang <meilinw@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802702}

ash/ambient/ambient_controller.cc

@@ -18,6 +18,7 @@
 #include "ash/public/cpp/ambient/ambient_metrics.h"
 #include "ash/public/cpp/ambient/ambient_prefs.h"
 #include "ash/public/cpp/ambient/ambient_ui_model.h"
+#include "ash/public/cpp/ambient/common/ambient_settings.h"
 #include "ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h"
 #include "ash/public/cpp/assistant/controller/assistant_ui_controller.h"
 #include "ash/public/cpp/shell_window_ids.h"
@@ -264,6 +265,16 @@ void AmbientController::OnAutoShowTimeOut() {
   ambient_ui_model_.SetUiVisibility(AmbientUiVisibility::kShown);
 }
 
+void AmbientController::OnActiveUserPrefServiceChanged(
+    PrefService* pref_service) {
+  // Only registers for primary prefs as Ambient Mode is only available for
+  // primary profile.
+  PrefService* primary_user_prefs =
+      Shell::Get()->session_controller()->GetPrimaryUserPrefService();
+  if (primary_user_prefs == pref_service)
+    RegisterPrefChanges(primary_user_prefs);
+}
+
 void AmbientController::OnLockStateChanged(bool locked) {
   if (!IsAmbientModeEnabled()) {
     VLOG(1) << "Ambient mode is not allowed.";
@@ -533,6 +544,31 @@ void AmbientController::StopRefreshingImages() {
   ambient_photo_controller_.StopScreenUpdate();
 }
 
+void AmbientController::RegisterPrefChanges(PrefService* pref_service) {
+  DCHECK(pref_service);
+
+  // Registers preference changes.
+  pref_change_registrar_ = std::make_unique<PrefChangeRegistrar>();
+  pref_change_registrar_->Init(pref_service);
+  pref_change_registrar_->Add(
+      ash::ambient::prefs::kAmbientModeEnabled,
+      base::BindRepeating(&AmbientController::OnEnabledStateChanged,
+                          weak_ptr_factory_.GetWeakPtr()));
+}
+
+void AmbientController::OnEnabledStateChanged() {
+  auto enabled = pref_change_registrar_->prefs()->GetBoolean(
+      ash::ambient::prefs::kAmbientModeEnabled);
+
+  // Initiates settings for ambient service upon enabled.
+  if (enabled) {
+    ambient_backend_controller_->InitSettings(base::BindOnce([](bool success) {
+      if (!success)
+        LOG(ERROR) << "Failed to initiate settings for ambient service.";
+    }));
+  }
+}
+
 void AmbientController::set_backend_controller_for_testing(
     std::unique_ptr<AmbientBackendController> backend_controller) {
   ambient_backend_controller_ = std::move(backend_controller);

ash/ambient/ambient_controller.h

@@ -21,12 +21,16 @@
 #include "base/memory/weak_ptr.h"
 #include "base/scoped_observer.h"
 #include "chromeos/dbus/power/power_manager_client.h"
+#include "components/prefs/pref_change_registrar.h"
+#include "components/prefs/pref_service.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "services/device/public/mojom/wake_lock.mojom.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_observer.h"
 
+class PrefChangeRegistrar;
 class PrefRegistrySimple;
+class PrefService;
 
 namespace ash {
 
@@ -55,6 +59,7 @@ class ASH_EXPORT AmbientController
   void OnAmbientUiVisibilityChanged(AmbientUiVisibility visibility) override;
 
   // SessionObserver:
+  void OnActiveUserPrefServiceChanged(PrefService* pref_service) override;
   void OnLockStateChanged(bool locked) override;
 
   // PowerStatus::Observer:
@@ -136,6 +141,11 @@ class ASH_EXPORT AmbientController
 
   void CloseWidget(bool immediately);
 
+  void RegisterPrefChanges(PrefService* pref_service);
+
+  // Invoked when the |kAmbientModeEnabled| pref state changed.
+  void OnEnabledStateChanged();
+
   AmbientContainerView* get_container_view_for_testing() {
     return container_view_;
   }
@@ -174,6 +184,9 @@ class ASH_EXPORT AmbientController
 
   bool is_screen_off_ = false;
 
+  // Observes user profile prefs for ambient.
+  std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;
+
   base::WeakPtrFactory<AmbientController> weak_ptr_factory_{this};
   DISALLOW_COPY_AND_ASSIGN(AmbientController);
 };

ash/ambient/backdrop/ambient_backend_controller_impl.cc

@@ -234,6 +234,20 @@ void AmbientBackendControllerImpl::FetchScreenUpdateInfo(
       weak_factory_.GetWeakPtr(), num_topics, std::move(callback)));
 }
 
+void AmbientBackendControllerImpl::InitSettings(
+    UpdateSettingsCallback callback) {
+  AmbientSettings settings;
+
+  // Inits weather-related settings. Leaving this settings as default could
+  // result in unpredictable behavior (b/158630188).
+  // Note that right now the weather info is designed to be always shown on
+  // ambient screen, and we don't expose an option in ambient Settings for
+  // users to switch it off.
+  settings.show_weather = true;
+
+  UpdateSettings(settings, std::move(callback));
+}
+
 void AmbientBackendControllerImpl::GetSettings(GetSettingsCallback callback) {
   Shell::Get()->ambient_controller()->RequestAccessToken(
       base::BindOnce(&AmbientBackendControllerImpl::StartToGetSettings,

ash/ambient/backdrop/ambient_backend_controller_impl.h

@@ -30,6 +30,7 @@ class AmbientBackendControllerImpl : public AmbientBackendController {
   void FetchScreenUpdateInfo(
       int num_topics,
       OnScreenUpdateInfoFetchedCallback callback) override;
+  void InitSettings(UpdateSettingsCallback callback) override;
   void GetSettings(GetSettingsCallback callback) override;
   void UpdateSettings(const AmbientSettings& settings,
                       UpdateSettingsCallback callback) override;

ash/public/cpp/ambient/ambient_backend_controller.h

@@ -116,6 +116,9 @@ class ASH_PUBLIC_EXPORT AmbientBackendController {
       int num_topics,
       OnScreenUpdateInfoFetchedCallback callback) = 0;
 
+  // Sets the initial settings to the backdrop server.
+  virtual void InitSettings(UpdateSettingsCallback callback) = 0;
+
   // Get ambient mode Settings from server.
   virtual void GetSettings(GetSettingsCallback callback) = 0;
 

ash/public/cpp/ambient/common/ambient_settings.h

@@ -79,6 +79,10 @@ struct ASH_PUBLIC_EXPORT AmbientSettings {
   // Only selected album.
   std::vector<std::string> selected_album_ids;
 
+  // This setting is not exposed to the user as right now we are expecting
+  // weather info to always show on ambient.
+  bool show_weather = true;
+
   AmbientModeTemperatureUnit temperature_unit =
       AmbientModeTemperatureUnit::kFahrenheit;
 };

ash/public/cpp/ambient/fake_ambient_backend_controller_impl.cc

@@ -86,6 +86,13 @@ void FakeAmbientBackendControllerImpl::FetchScreenUpdateInfo(
       FROM_HERE, base::BindOnce(std::move(callback), update));
 }
 
+void FakeAmbientBackendControllerImpl::InitSettings(
+    UpdateSettingsCallback callback) {
+  // Post task to simulate an async response.
+  base::SequencedTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(std::move(callback), /*success=*/true));
+}
+
 void FakeAmbientBackendControllerImpl::GetSettings(
     GetSettingsCallback callback) {
   // Pretend to respond asynchronously.

ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h

@@ -21,6 +21,7 @@ class ASH_PUBLIC_EXPORT FakeAmbientBackendControllerImpl
   void FetchScreenUpdateInfo(
       int num_topics,
       OnScreenUpdateInfoFetchedCallback callback) override;
+  void InitSettings(UpdateSettingsCallback callback) override;
   void GetSettings(GetSettingsCallback callback) override;
   void UpdateSettings(const AmbientSettings& settings,
                       UpdateSettingsCallback callback) override;