personalization: Take no action on failed Google Photos wallpaper req…

…uest

Currently, if a request fails when trying to fetch the URL for a Google
Photos wallpaper, we reset to the default wallpaper. We only want to
reset the wallpaper if the photo does not exist. We don't want to reset
the wallpaper if the request failed entirely, so we need a way to
differentiate between those two error states in the controller.
Otherwise, the user disconnecting from WiFi could cause the wallpaper to
reset unnecessarily.

This would be a rare occurrence right now, but once an upcoming CL
enables periodic staleness checking for Google Photos wallpapers, it
would happen any time a user is disconnected from WiFi for a substantial
period of time.

Bug: b:226198384
Change-Id: I1e60971a1ace376cf6b26a421fda2f0cdf072d39
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3543355
Reviewed-by: Colin Kincaid <ckincaid@chromium.org>
Reviewed-by: David Black <dmblack@google.com>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: Angus McLean <angusmclean@chromium.org>
Cr-Commit-Position: refs/heads/main@{#985654}

ash/public/cpp/wallpaper/wallpaper_controller_client.h

@@ -71,7 +71,8 @@ class ASH_PUBLIC_EXPORT WallpaperControllerClient {
       FetchImagesForCollectionCallback callback) = 0;
 
   using FetchGooglePhotosPhotoCallback = base::OnceCallback<void(
-      mojo::StructPtr<ash::personalization_app::mojom::GooglePhotosPhoto>)>;
+      mojo::StructPtr<ash::personalization_app::mojom::GooglePhotosPhoto>,
+      bool success)>;
   virtual void FetchGooglePhotosPhoto(
       const AccountId& account_id,
       const std::string& id,

ash/wallpaper/test_wallpaper_controller_client.cc

@@ -101,11 +101,13 @@ void TestWallpaperControllerClient::FetchGooglePhotosPhoto(
   base::Time::Exploded exploded_time{2011, 6, 3, 15, 12, 0, 0, 0};
   DCHECK(base::Time::FromUTCExploded(exploded_time, &time));
   if (fetch_google_photos_photo_fails_) {
-    std::move(callback).Run(nullptr);
+    std::move(callback).Run(nullptr, /*success=*/false);
   } else {
-    std::move(callback).Run(personalization_app::mojom::GooglePhotosPhoto::New(
-        id, "test_name", base::TimeFormatFriendlyDate(time),
-        GURL("https://google.com/picture.png")));
+    std::move(callback).Run(
+        personalization_app::mojom::GooglePhotosPhoto::New(
+            id, "test_name", base::TimeFormatFriendlyDate(time),
+            GURL("https://google.com/picture.png")),
+        /*success=*/true);
   }
 }
 

ash/wallpaper/wallpaper_controller_impl.cc

@@ -2203,10 +2203,17 @@ void WallpaperControllerImpl::SetOnlineWallpaperImpl(
 void WallpaperControllerImpl::OnGooglePhotosPhotoFetched(
     const GooglePhotosWallpaperParams& params,
     SetWallpaperCallback callback,
-    ash::personalization_app::mojom::GooglePhotosPhotoPtr photo) {
-  // TODO(angusmclean): Detect whether the photo doesn't exist or if the request
-  // simply failed. If the request failed, load from cache or exit as
-  // appropriate. If the photo doesn't exist, continue to below.
+    ash::personalization_app::mojom::GooglePhotosPhotoPtr photo,
+    bool success) {
+  // It should be impossible for us to get back a photo successfully from
+  // a request that failed.
+  DCHECK(success || !photo);
+  // If the request failed, there's nothing to do here, since we can't update
+  // the wallpaper but also don't want to delete the cache.
+  if (!success) {
+    std::move(callback).Run(false);
+    return;
+  }
   if (photo.is_null()) {
     // The photo doesn't exist, or has been deleted. If this photo is the
     // current wallpaper, we need to reset to the default.

ash/wallpaper/wallpaper_controller_impl.h

@@ -469,7 +469,8 @@ class ASH_EXPORT WallpaperControllerImpl
   void OnGooglePhotosPhotoFetched(
       const GooglePhotosWallpaperParams& params,
       SetWallpaperCallback callback,
-      ash::personalization_app::mojom::GooglePhotosPhotoPtr photo);
+      ash::personalization_app::mojom::GooglePhotosPhotoPtr photo,
+      bool success);
 
   void GetGooglePhotosWallpaperFromCacheOrDownload(
       const GooglePhotosWallpaperParams& params,

chrome/browser/ash/wallpaper_handlers/wallpaper_handlers.cc

@@ -37,6 +37,7 @@
 #include "google_apis/gaia/google_service_auth_error.h"
 #include "net/base/load_flags.h"
 #include "net/base/url_util.h"
+#include "net/http/http_status_code.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"
@@ -591,6 +592,12 @@ void GooglePhotosFetcher<T>::AddRequestAndStartIfNecessary(
           signin::ConsentLevel::kSignin);
 }
 
+template <typename T>
+absl::optional<base::Value> GooglePhotosFetcher<T>::CreateErrorResponse(
+    int error_code) {
+  return absl::nullopt;
+}
+
 template <typename T>
 void GooglePhotosFetcher<T>::OnTokenReceived(
     const GURL& service_url,
@@ -635,11 +642,15 @@ void GooglePhotosFetcher<T>::OnJsonReceived(
     LOG(ERROR) << "Google Photos API request to " << service_url.spec()
                << " failed.";
     auto* response_info = url_loaders_[service_url]->ResponseInfo();
+    absl::optional<base::Value> error_response;
     if (response_info && response_info->headers) {
       LOG(ERROR) << "HTTP response code: "
                  << response_info->headers->response_code();
+      error_response =
+          CreateErrorResponse(response_info->headers->response_code());
+      return;
     }
-    OnResponseReady(service_url, start_time, absl::nullopt);
+    OnResponseReady(service_url, start_time, std::move(error_response));
     return;
   }
 
@@ -859,6 +870,19 @@ void GooglePhotosPhotosFetcher::AddRequestAndStartIfNecessary(
                                                      std::move(callback));
 }
 
+absl::optional<base::Value> GooglePhotosPhotosFetcher::CreateErrorResponse(
+    int error_code) {
+  // If the server gives us 404, that means the request succeeded, but no
+  // photos with the given attributes exist. We return an empty list of photos
+  // to communicate this back to the caller.
+  if (error_code == net::HTTP_NOT_FOUND) {
+    absl::optional<base::Value> empty_list(base::Value::Type::DICT);
+    empty_list->SetKey("item", base::Value(base::Value::Type::LIST));
+    return empty_list;
+  }
+  return absl::nullopt;
+}
+
 GooglePhotosPhotosCbkArgs GooglePhotosPhotosFetcher::ParseResponse(
     const base::Value::Dict* response) {
   auto parsed_response =

chrome/browser/ash/wallpaper_handlers/wallpaper_handlers.h

@@ -176,6 +176,10 @@ class GooglePhotosFetcher : public signin::IdentityManager::Observer {
   // Returns the count of results contained within the specified `result`.
   virtual absl::optional<size_t> GetResultCount(const T& result) = 0;
 
+  // Contains logic for different HTTP error codes that we receive, as they can
+  // carry information on the state of the user's Google Photos library.
+  virtual absl::optional<base::Value> CreateErrorResponse(int error_code);
+
  private:
   void OnTokenReceived(const GURL& service_url,
                        base::TimeTicks start_time,
@@ -308,6 +312,7 @@ class GooglePhotosPhotosFetcher
 
  protected:
   // GooglePhotosFetcher:
+  absl::optional<base::Value> CreateErrorResponse(int error_code) override;
   GooglePhotosPhotosCbkArgs ParseResponse(
       const base::Value::Dict* response) override;
   absl::optional<size_t> GetResultCount(

chrome/browser/ash/web_applications/personalization_app/personalization_app_wallpaper_provider_impl.cc

@@ -762,10 +762,12 @@ void PersonalizationAppWallpaperProviderImpl::FindAttributionInCollection(
 void PersonalizationAppWallpaperProviderImpl::SendGooglePhotosAttribution(
     const ash::WallpaperInfo& info,
     const GURL& wallpaper_data_url,
-    mojo::StructPtr<ash::personalization_app::mojom::GooglePhotosPhoto> photo) {
+    mojo::StructPtr<ash::personalization_app::mojom::GooglePhotosPhoto> photo,
+    bool success) {
   std::vector<std::string> attribution;
-  if (!photo.is_null())
+  if (!photo.is_null()) {
     attribution.push_back(photo->name);
+  }
   NotifyWallpaperChanged(ash::personalization_app::mojom::CurrentWallpaper::New(
       wallpaper_data_url, attribution, info.layout, info.type,
       /*key=*/info.location));

chrome/browser/ash/web_applications/personalization_app/personalization_app_wallpaper_provider_impl.h

@@ -223,8 +223,8 @@ class PersonalizationAppWallpaperProviderImpl
   void SendGooglePhotosAttribution(
       const ash::WallpaperInfo& info,
       const GURL& wallpaper_data_url,
-      mojo::StructPtr<ash::personalization_app::mojom::GooglePhotosPhoto>
-          photo);
+      mojo::StructPtr<ash::personalization_app::mojom::GooglePhotosPhoto> photo,
+      bool success);
 
   // Called when the user sets an image, or cancels/confirms preview wallpaper.
   // If a new image is set in preview mode, will minimize all windows except the

chrome/browser/ui/ash/wallpaper_controller_client_impl.cc

@@ -829,11 +829,16 @@ void WallpaperControllerClientImpl::OnGooglePhotosPhotoFetched(
     FetchGooglePhotosPhotoCallback callback,
     ash::personalization_app::mojom::FetchGooglePhotosPhotosResponsePtr
         response) {
-  if (!response->photos.has_value() || response->photos.value().size() != 1) {
-    std::move(callback).Run(nullptr);
-    return;
+  // If we have a `GooglePhotosPhoto`, pass that along. Otherwise, indicate to
+  // `callback` whether the the request succeeded or failed, since `callback`
+  // can take action if the `GooglePhotosPhoto` with the given id has been
+  // deleted.
+  if (response->photos.has_value() && response->photos.value().size() == 1) {
+    std::move(callback).Run(std::move(response->photos.value()[0]),
+                            /*success=*/true);
+  } else {
+    std::move(callback).Run(nullptr, /*success=*/response->photos.has_value());
   }
-  std::move(callback).Run(std::move(response->photos.value()[0]));
 }
 
 void WallpaperControllerClientImpl::ObserveVolumeManagerForAccountId(