ambient: using new weather API

Fetch weather separately with dedicated weather API. Also change
it to a different timer so we don't need to fetch it as frequently
as the photos.

Bug: b:166160411
Test: unittest and manual test
Change-Id: Ib0bd1ca271b1b4ac8daabadf9ee72c4899231551
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425344
Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811314}

ash/ambient/ambient_access_token_controller.cc

@@ -92,7 +92,8 @@ void AmbientAccessTokenController::AccessTokenRefreshed(
     return;
   }
 
-  VLOG(1) << "Access token fetched.";
+  DVLOG(1) << "Access token fetched.";
+  DCHECK(gaia_id_.empty() || gaia_id_ == gaia_id);
   refresh_token_retry_backoff_.Reset();
   gaia_id_ = gaia_id;
   access_token_ = access_token;

ash/ambient/ambient_constants.h

@@ -24,6 +24,10 @@ constexpr base::TimeDelta kTopicFetchInterval =
 constexpr base::TimeDelta kPhotoRefreshInterval =
     base::TimeDelta::FromSeconds(60);
 
+// The default interval to refresh weather.
+constexpr base::TimeDelta kWeatherRefreshInterval =
+    base::TimeDelta::FromMinutes(5);
+
 // The batch size of topics to fetch in one request.
 // Magic number 2 is based on experiments that no curation on Google Photos.
 constexpr int kTopicsBatchSize = 2;

ash/ambient/ambient_photo_controller.cc

@@ -229,10 +229,16 @@ AmbientPhotoController::~AmbientPhotoController() = default;
 
 void AmbientPhotoController::StartScreenUpdate() {
   FetchTopics();
+  FetchWeather();
+  weather_refresh_timer_.Start(
+      FROM_HERE, kWeatherRefreshInterval,
+      base::BindRepeating(&AmbientPhotoController::FetchWeather,
+                          weak_factory_.GetWeakPtr()));
 }
 
 void AmbientPhotoController::StopScreenUpdate() {
   photo_refresh_timer_.Stop();
+  weather_refresh_timer_.Stop();
   topic_index_ = 0;
   image_refresh_started_ = false;
   retries_to_read_from_cache_ = kMaxNumberOfCachedImages;
@@ -262,6 +268,15 @@ void AmbientPhotoController::FetchTopics() {
                          weak_factory_.GetWeakPtr()));
 }
 
+void AmbientPhotoController::FetchWeather() {
+  Shell::Get()
+      ->ambient_controller()
+      ->ambient_backend_controller()
+      ->FetchWeather(base::BindOnce(
+          &AmbientPhotoController::StartDownloadingWeatherConditionIcon,
+          weak_factory_.GetWeakPtr()));
+}
+
 void AmbientPhotoController::ClearCache() {
   task_runner_->PostTask(FROM_HERE,
                          base::BindOnce(&DeletePathRecursively, GetRootPath()));
@@ -427,9 +442,9 @@ void AmbientPhotoController::OnPhotoRawDataAvailable(
     base::RepeatingClosure on_done,
     std::unique_ptr<std::string> details,
     std::unique_ptr<std::string> data) {
-  if (is_related_image)
+  if (is_related_image) {
     related_image_data_ = std::move(data);
-  else {
+  } else {
     image_data_ = std::move(data);
     image_details_ = std::move(details);
   }

ash/ambient/ambient_photo_controller.h

@@ -106,6 +106,8 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
 
   void FetchTopics();
 
+  void FetchWeather();
+
   void ScheduleFetchTopics(bool backoff);
 
   void ScheduleRefreshImage();
@@ -181,6 +183,9 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
   // The timer to refresh photos.
   base::OneShotTimer photo_refresh_timer_;
 
+  // The timer to refresh weather information.
+  base::RepeatingTimer weather_refresh_timer_;
+
   // The index of a topic to download.
   size_t topic_index_ = 0;
 

ash/ambient/ambient_photo_controller_unittest.cc

@@ -12,6 +12,7 @@
 #include "ash/ambient/model/ambient_backend_model.h"
 #include "ash/ambient/test/ambient_ash_test_base.h"
 #include "ash/public/cpp/ambient/ambient_backend_controller.h"
+#include "ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h"
 #include "ash/shell.h"
 #include "base/barrier_closure.h"
 #include "base/base_paths.h"
@@ -345,4 +346,33 @@ TEST_F(AmbientPhotoControllerTest, ShouldResumWhenHaveMoreTopics) {
   base::DeletePathRecursively(ambient_image_path);
 }
 
+TEST_F(AmbientPhotoControllerTest, ShouldStartToRefreshWeather) {
+  auto* model = photo_controller()->ambient_backend_model();
+  EXPECT_FALSE(model->show_celsius());
+  EXPECT_TRUE(model->weather_condition_icon().isNull());
+
+  WeatherInfo info;
+  info.show_celsius = true;
+  info.condition_icon_url = "https://fake-icon-url";
+  info.temp_f = 70.0f;
+  backend_controller()->SetWeatherInfo(info);
+
+  // Start to refresh weather as screen update starts.
+  photo_controller()->StartScreenUpdate();
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(model->show_celsius());
+  EXPECT_FALSE(model->weather_condition_icon().isNull());
+  EXPECT_GT(info.temp_f, 0);
+
+  // Refresh weather again after time passes.
+  info.show_celsius = false;
+  info.temp_f = -70.0f;
+  backend_controller()->SetWeatherInfo(info);
+
+  FastForwardToRefreshWeather();
+  EXPECT_FALSE(model->show_celsius());
+  EXPECT_LT(info.temp_f, 0);
+}
+
 }  // namespace ash

ash/ambient/backdrop/ambient_backend_controller_impl.cc

@@ -25,9 +25,11 @@
 #include "chromeos/assistant/internal/proto/google3/backdrop/backdrop.pb.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "components/prefs/pref_service.h"
+#include "components/user_manager/user_manager.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
+#include "services/data_decoder/public/cpp/data_decoder.h"
 #include "services/network/public/cpp/resource_request.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "services/network/public/cpp/simple_url_loader.h"
@@ -142,6 +144,30 @@ AmbientModeTopicType ToAmbientModeTopicType(
   }
 }
 
+WeatherInfo ToWeatherInfo(const base::Value& result) {
+  DCHECK(result.is_list());
+
+  WeatherInfo weather_info;
+  const auto& list_result = result.GetList();
+
+  const base::Value& condition_icon_url_value =
+      list_result[backdrop::WeatherInfo::kConditionIconUrlFieldNumber - 1];
+  if (!condition_icon_url_value.is_none())
+    weather_info.condition_icon_url = condition_icon_url_value.GetString();
+
+  const base::Value& temp_f_value =
+      list_result[backdrop::WeatherInfo::kTempFFieldNumber - 1];
+  if (!temp_f_value.is_none())
+    weather_info.temp_f = temp_f_value.GetDouble();
+
+  const base::Value& show_celsius_value =
+      list_result[backdrop::WeatherInfo::kShowCelsiusFieldNumber - 1];
+  if (!show_celsius_value.is_none())
+    weather_info.show_celsius = show_celsius_value.GetBool();
+
+  return weather_info;
+}
+
 // Helper function to save the information we got from the backdrop server to a
 // public struct so that they can be accessed by public codes.
 ScreenUpdate ToScreenUpdate(
@@ -358,6 +384,47 @@ void AmbientBackendControllerImpl::SetPhotoRefreshInterval(
       ->SetPhotoRefreshInterval(interval);
 }
 
+void AmbientBackendControllerImpl::FetchWeather(FetchWeatherCallback callback) {
+  auto response_handler =
+      [](FetchWeatherCallback callback,
+         std::unique_ptr<BackdropURLLoader> backdrop_url_loader,
+         std::unique_ptr<std::string> response) {
+        if (response && !response->empty()) {
+          auto json_handler =
+              [](FetchWeatherCallback callback,
+                 data_decoder::DataDecoder::ValueOrError result) {
+                if (result.value) {
+                  std::move(callback).Run(ToWeatherInfo(result.value.value()));
+                } else {
+                  DVLOG(1) << "Failed to parse weather json.";
+                  std::move(callback).Run(base::nullopt);
+                }
+              };
+
+          constexpr char kJsonPrefix[] = ")]}'\n";
+          data_decoder::DataDecoder::ParseJsonIsolated(
+              response->substr(strlen(kJsonPrefix)),
+              base::BindOnce(json_handler, std::move(callback)));
+        } else {
+          std::move(callback).Run(base::nullopt);
+        }
+      };
+
+  const auto* user = user_manager::UserManager::Get()->GetActiveUser();
+  DCHECK(user->HasGaiaAccount());
+  BackdropClientConfig::Request request =
+      backdrop_client_config_.CreateFetchWeatherInfoRequest(
+          user->GetAccountId().GetGaiaId(), GetClientId());
+  std::unique_ptr<network::ResourceRequest> resource_request =
+      CreateResourceRequest(request);
+  auto backdrop_url_loader = std::make_unique<BackdropURLLoader>();
+  auto* loader_ptr = backdrop_url_loader.get();
+  loader_ptr->Start(std::move(resource_request), /*request_body=*/base::nullopt,
+                    NO_TRAFFIC_ANNOTATION_YET,
+                    base::BindOnce(response_handler, std::move(callback),
+                                   std::move(backdrop_url_loader)));
+}
+
 void AmbientBackendControllerImpl::FetchScreenUpdateInfoInternal(
     int num_topics,
     OnScreenUpdateInfoFetchedCallback callback,

ash/ambient/backdrop/ambient_backend_controller_impl.h

@@ -48,6 +48,7 @@ class AmbientBackendControllerImpl : public AmbientBackendController {
       int num_albums,
       OnSettingsAndAlbumsFetchedCallback callback) override;
   void SetPhotoRefreshInterval(base::TimeDelta interval) override;
+  void FetchWeather(FetchWeatherCallback callback) override;
 
  private:
   using BackdropClientConfig = chromeos::ambient::BackdropClientConfig;

ash/ambient/test/ambient_ash_test_base.cc

@@ -249,6 +249,10 @@ void AmbientAshTestBase::FastForwardToNextImage() {
   task_environment()->FastForwardBy(1.2 * kPhotoRefreshInterval);
 }
 
+void AmbientAshTestBase::FastForwardToRefreshWeather() {
+  task_environment()->FastForwardBy(1.2 * kWeatherRefreshInterval);
+}
+
 int AmbientAshTestBase::GetNumOfActiveWakeLocks(
     device::mojom::WakeLockType type) {
   base::RunLoop run_loop;
@@ -293,6 +297,11 @@ AmbientAccessTokenController* AmbientAshTestBase::token_controller() {
   return ambient_controller()->access_token_controller_for_testing();
 }
 
+FakeAmbientBackendControllerImpl* AmbientAshTestBase::backend_controller() {
+  return static_cast<FakeAmbientBackendControllerImpl*>(
+      ambient_controller()->ambient_backend_controller());
+}
+
 void AmbientAshTestBase::FetchTopics() {
   photo_controller()->FetchTopicsForTesting();
 }

ash/ambient/test/ambient_ash_test_base.h

@@ -28,6 +28,7 @@ namespace ash {
 class AmbientAccessTokenController;
 class AmbientContainerView;
 class AmbientPhotoController;
+class FakeAmbientBackendControllerImpl;
 class MediaStringView;
 
 // The base class to test the Ambient Mode in Ash.
@@ -95,6 +96,9 @@ class AmbientAshTestBase : public AshTestBase {
   // Advance the task environment timer to load the next photo.
   void FastForwardToNextImage();
 
+  // Advance the task environment timer to load the weather info.
+  void FastForwardToRefreshWeather();
+
   // Returns the number of active wake locks of type |type|.
   int GetNumOfActiveWakeLocks(device::mojom::WakeLockType type);
 
@@ -120,6 +124,8 @@ class AmbientAshTestBase : public AshTestBase {
 
   AmbientAccessTokenController* token_controller();
 
+  FakeAmbientBackendControllerImpl* backend_controller();
+
   void FetchTopics();
 
   void FetchImage();

ash/public/cpp/ambient/ambient_backend_controller.h

@@ -108,6 +108,8 @@ class ASH_PUBLIC_EXPORT AmbientBackendController {
   using OnSettingsAndAlbumsFetchedCallback =
       base::OnceCallback<void(const base::Optional<AmbientSettings>& settings,
                               PersonalAlbums personal_albums)>;
+  using FetchWeatherCallback =
+      base::OnceCallback<void(const base::Optional<WeatherInfo>& weather_info)>;
 
   static AmbientBackendController* Get();
 
@@ -154,6 +156,9 @@ class ASH_PUBLIC_EXPORT AmbientBackendController {
 
   // Set the photo refresh interval in ambient mode.
   virtual void SetPhotoRefreshInterval(base::TimeDelta interval) = 0;
+
+  // Fetch the weather information.
+  virtual void FetchWeather(FetchWeatherCallback) = 0;
 };
 
 }  // namespace ash

ash/public/cpp/ambient/fake_ambient_backend_controller_impl.cc

@@ -85,14 +85,17 @@ void FakeAmbientBackendControllerImpl::FetchScreenUpdateInfo(
   topic.related_image_url = kFakeUrl;
   topic.topic_type = AmbientModeTopicType::kCulturalInstitute;
 
-  ash::WeatherInfo weather_info;
-  weather_info.temp_f = .0f;
-  weather_info.condition_icon_url = kFakeUrl;
-  weather_info.show_celsius = true;
-
   ash::ScreenUpdate update;
   update.next_topics.emplace_back(topic);
-  update.weather_info = weather_info;
+
+  // Only respond weather info when there is no active weather testing.
+  if (!weather_info_) {
+    ash::WeatherInfo weather_info;
+    weather_info.temp_f = .0f;
+    weather_info.condition_icon_url = kFakeUrl;
+    weather_info.show_celsius = true;
+    update.weather_info = weather_info;
+  }
 
   // Pretend to respond asynchronously.
   base::SequencedTaskRunnerHandle::Get()->PostTask(
@@ -153,6 +156,11 @@ void FakeAmbientBackendControllerImpl::SetPhotoRefreshInterval(
   NOTIMPLEMENTED();
 }
 
+void FakeAmbientBackendControllerImpl::FetchWeather(
+    FetchWeatherCallback callback) {
+  std::move(callback).Run(weather_info_);
+}
+
 void FakeAmbientBackendControllerImpl::ReplyFetchSettingsAndAlbums(
     bool success) {
   if (!pending_fetch_settings_albums_callback_)
@@ -182,4 +190,9 @@ bool FakeAmbientBackendControllerImpl::IsUpdateSettingsPending() const {
   return !pending_update_callback_.is_null();
 }
 
+void FakeAmbientBackendControllerImpl::SetWeatherInfo(
+    base::Optional<WeatherInfo> info) {
+  weather_info_ = std::move(info);
+}
+
 }  // namespace ash

ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h

@@ -40,6 +40,7 @@ class ASH_PUBLIC_EXPORT FakeAmbientBackendControllerImpl
       int num_albums,
       OnSettingsAndAlbumsFetchedCallback callback) override;
   void SetPhotoRefreshInterval(base::TimeDelta interval) override;
+  void FetchWeather(FetchWeatherCallback callback) override;
 
   // Simulate to reply the request of FetchSettingsAndAlbums().
   // If |success| is true, will return fake data.
@@ -55,10 +56,16 @@ class ASH_PUBLIC_EXPORT FakeAmbientBackendControllerImpl
   // Whether there is a pending UpdateSettings() request.
   bool IsUpdateSettingsPending() const;
 
+  // Sets the weather info that will be returned in subsequent calls to
+  // `FetchWeather`.
+  void SetWeatherInfo(base::Optional<WeatherInfo> info);
+
  private:
   OnSettingsAndAlbumsFetchedCallback pending_fetch_settings_albums_callback_;
 
   UpdateSettingsCallback pending_update_callback_;
+
+  base::Optional<WeatherInfo> weather_info_;
 };
 
 }  // namespace ash

chrome/browser/ui/ash/ambient/ambient_client_impl.cc

@@ -19,6 +19,7 @@
 #include "components/prefs/pref_service.h"
 #include "components/signin/public/identity_manager/access_token_fetcher.h"
 #include "components/signin/public/identity_manager/access_token_info.h"
+#include "components/signin/public/identity_manager/account_info.h"
 #include "components/signin/public/identity_manager/consent_level.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/signin/public/identity_manager/scope_set.h"
@@ -121,7 +122,6 @@ void AmbientClientImpl::RequestAccessToken(GetAccessTokenCallback callback) {
 
   CoreAccountInfo account_info = identity_manager->GetPrimaryAccountInfo(
       signin::ConsentLevel::kNotRequired);
-
   const signin::ScopeSet scopes{kPhotosOAuthScope, kBackdropOAuthScope};
   // TODO(b/148463064): Handle retry refresh token and multiple requests.
   // Currently only one request is allowed.
@@ -148,6 +148,15 @@ void AmbientClientImpl::RequestWakeLockProvider(
   content::GetDeviceService().BindWakeLockProvider(std::move(receiver));
 }
 
+bool AmbientClientImpl::ShouldUseProdServer() {
+  if (chromeos::features::IsAmbientModeDevUseProdEnabled())
+    return true;
+
+  auto channel = chrome::GetChannel();
+  return channel == version_info::Channel::STABLE ||
+         channel == version_info::Channel::BETA;
+}
+
 void AmbientClientImpl::GetAccessToken(
     GetAccessTokenCallback callback,
     const std::string& gaia_id,
@@ -167,11 +176,3 @@ void AmbientClientImpl::GetAccessToken(
   }
 }
 
-bool AmbientClientImpl::ShouldUseProdServer() {
-  if (chromeos::features::IsAmbientModeDevUseProdEnabled())
-    return true;
-
-  auto channel = chrome::GetChannel();
-  return channel == version_info::Channel::STABLE ||
-         channel == version_info::Channel::BETA;
-}