personalization: Move first Google Photos data fetch for photos.

The first Google Photos data fetch for photos previously occurred on app load. Now, the data fetch will occur only after the user has selected the Google Photos collection. A similar CL was previously done for the first Google Photos albums data fetch: https://chromium-review.googlesource.com/c/chromium/src/+/3630838 Bug: b:3630838 Change-Id: I85f0727af045873f47a8a80fc71bc0942ff5d239 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3632591 Commit-Queue: David Black <dmblack@google.com> Reviewed-by: Jeffrey Young <cowmoo@chromium.org> Cr-Commit-Position: refs/heads/main@{#1001813}
chromium · May 11, 2022 · b10e615 · b10e615
1 parent 3c611ff
commit b10e615
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 71 deletions.
diff --git a/...webui/personalization_app/resources/trusted/wallpaper/google_photos_collection_element.ts b/...webui/personalization_app/resources/trusted/wallpaper/google_photos_collection_element.ts
@@ -19,7 +19,7 @@ import {Paths, PersonalizationRouter} from '../personalization_router_element.js
 import {WithPersonalizationStore} from '../personalization_store.js';
 
 import {getTemplate} from './google_photos_collection_element.html.js';
-import {fetchGooglePhotosAlbums, initializeGooglePhotosData} from './wallpaper_controller.js';
+import {fetchGooglePhotosAlbums, fetchGooglePhotosPhotos, initializeGooglePhotosData} from './wallpaper_controller.js';
 import {getWallpaperProvider} from './wallpaper_interface_provider.js';
 
 /** Enumeration of supported tabs. */
@@ -109,6 +109,9 @@ export class GooglePhotosCollection extends WithPersonalizationStore {
   private photosByAlbumId_: Record<string, GooglePhotosPhoto[]|null|undefined>|
       undefined;
 
+  /** Whether the list of photos is currently loading. */
+  private photosLoading_: boolean|undefined;
+
   /** The currently selected tab. */
   private tab_: Tab;
 
@@ -130,6 +133,8 @@ export class GooglePhotosCollection extends WithPersonalizationStore {
     this.watch<GooglePhotosCollection['photosByAlbumId_']>(
         'photosByAlbumId_',
         state => state.wallpaper.googlePhotos.photosByAlbumId);
+    this.watch<GooglePhotosCollection['photosLoading_']>(
+        'photosLoading_', state => state.wallpaper.loading.googlePhotos.photos);
 
     this.updateFromStore();
 
@@ -151,6 +156,14 @@ export class GooglePhotosCollection extends WithPersonalizationStore {
     document.title = this.i18n('googlePhotosLabel');
     this.$.main.focus();
 
+    // When the user first selects the Google Photos collection it should result
+    // in a data fetch for the user's photos.
+    if (this.photos_ === undefined && !this.photosLoading_) {
+      this.initializeGooglePhotosDataPromise_.then(() => {
+        fetchGooglePhotosPhotos(this.wallpaperProvider_, this.getStore());
+      });
+    }
+
     // When the user first selects the Google Photos collection it should result
     // in a data fetch for the user's albums.
     if (this.albums_ === undefined && !this.albumsLoading_) {

diff --git a/ash/webui/personalization_app/resources/trusted/wallpaper/wallpaper_controller.ts b/ash/webui/personalization_app/resources/trusted/wallpaper/wallpaper_controller.ts
@@ -69,12 +69,19 @@ async function fetchAllImagesForCollections(
 export async function fetchGooglePhotosAlbum(
     provider: WallpaperProviderInterface, store: PersonalizationStore,
     albumId: string): Promise<void> {
-  // Photos should only be fetched after confirming access is allowed.
+  // Photos should only be fetched after determining whether access is allowed.
   const enabled = store.data.wallpaper.googlePhotos.enabled;
-  assert(enabled === GooglePhotosEnablementState.kEnabled);
+  assert(enabled !== undefined);
 
   store.dispatch(action.beginLoadGooglePhotosAlbumAction(albumId));
 
+  // If access is *not* allowed, short-circuit the request.
+  if (enabled !== GooglePhotosEnablementState.kEnabled) {
+    store.dispatch(action.appendGooglePhotosPhotosAction(
+        /*photos=*/ null, /*resumeToken=*/ null));
+    return;
+  }
+
   let photos: Array<GooglePhotosPhoto>|null = [];
   let resumeToken =
       store.data.wallpaper.googlePhotos.resumeTokens.photosByAlbumId[albumId] ||
@@ -162,12 +169,19 @@ async function fetchGooglePhotosEnabled(
 export async function fetchGooglePhotosPhotos(
     provider: WallpaperProviderInterface,
     store: PersonalizationStore): Promise<void> {
-  // Photos should only be fetched after confirmed access is allowed.
+  // Photos should only be fetched after determining whether access is allowed.
   const enabled = store.data.wallpaper.googlePhotos.enabled;
-  assert(enabled === GooglePhotosEnablementState.kEnabled);
+  assert(enabled !== undefined);
 
   store.dispatch(action.beginLoadGooglePhotosPhotosAction());
 
+  // If access is *not* allowed, short-circuit the request.
+  if (enabled !== GooglePhotosEnablementState.kEnabled) {
+    store.dispatch(action.appendGooglePhotosPhotosAction(
+        /*photos=*/ null, /*resumeToken=*/ null));
+    return;
+  }
+
   let photos: Array<GooglePhotosPhoto>|null = [];
   let resumeToken = store.data.wallpaper.googlePhotos.resumeTokens.photos;
 
@@ -414,25 +428,13 @@ export async function initializeBackdropData(
   await fetchAllImagesForCollections(provider, store);
 }
 
+// TODO(b:230635452): Remove this method since it is now just a thin wrapper.
 /** Fetches initial Google Photos data and saves it to the store. */
 export async function initializeGooglePhotosData(
     provider: WallpaperProviderInterface,
     store: PersonalizationStore): Promise<void> {
   // Fetch whether the user is allowed to access Google Photos.
   await fetchGooglePhotosEnabled(provider, store);
-
-  // Only proceed to fetch Google Photos data if the user is allowed.
-  const enabled = store.data.wallpaper.googlePhotos.enabled;
-  if (enabled === GooglePhotosEnablementState.kEnabled) {
-    await fetchGooglePhotosPhotos(provider, store);
-  } else {
-    const result = null;
-    const resumeToken = null;
-    store.beginBatchUpdate();
-    store.dispatch(action.beginLoadGooglePhotosPhotosAction());
-    store.dispatch(action.appendGooglePhotosPhotosAction(result, resumeToken));
-    store.endBatchUpdate();
-  }
 }
 
 /**

diff --git a/chrome/test/data/webui/chromeos/personalization_app/google_photos_collection_element_test.ts b/chrome/test/data/webui/chromeos/personalization_app/google_photos_collection_element_test.ts
@@ -346,6 +346,40 @@ suite('GooglePhotosCollectionTest', function() {
             wallpaperProvider.getCallCount('fetchGooglePhotosAlbums'), 0);
       }));
 
+  [true, false].forEach(
+      hidden => test('fetches photos on first show', async () => {
+        // Initialize |googlePhotosCollectionElement| in |hidden| state.
+        googlePhotosCollectionElement =
+            initElement(GooglePhotosCollection, {hidden});
+        await waitAfterNextRender(googlePhotosCollectionElement);
+
+        if (hidden) {
+          // Photos should *not* be fetched when hidden.
+          await new Promise<void>(resolve => setTimeout(resolve, 100));
+          assertEquals(
+              wallpaperProvider.getCallCount('fetchGooglePhotosPhotos'), 0);
+
+          // Show |googlePhotosCollectionElement|.
+          googlePhotosCollectionElement.hidden = false;
+          await waitAfterNextRender(googlePhotosCollectionElement);
+        }
+
+        // Photos *should* be fetched when shown.
+        await wallpaperProvider.whenCalled('fetchGooglePhotosPhotos');
+        wallpaperProvider.reset();
+
+        // Hide and re-show |googlePhotosCollectionElement|.
+        googlePhotosCollectionElement.hidden = true;
+        await waitAfterNextRender(googlePhotosCollectionElement);
+        googlePhotosCollectionElement.hidden = false;
+        await waitAfterNextRender(googlePhotosCollectionElement);
+
+        // Photos should *not* be fetched when re-shown.
+        await new Promise<void>(resolve => setTimeout(resolve, 100));
+        assertEquals(
+            wallpaperProvider.getCallCount('fetchGooglePhotosPhotos'), 0);
+      }));
+
   test('sets aria label', async () => {
     googlePhotosCollectionElement =
         initElement(GooglePhotosCollection, {hidden: false});

diff --git a/.../data/webui/chromeos/personalization_app/google_photos_photos_by_album_id_element_test.ts b/.../data/webui/chromeos/personalization_app/google_photos_photos_by_album_id_element_test.ts
@@ -5,7 +5,7 @@
 import 'chrome://personalization/strings.m.js';
 import 'chrome://webui-test/mojo_webui_test_support.js';
 
-import {fetchGooglePhotosAlbum, fetchGooglePhotosAlbums, GooglePhotosAlbum, GooglePhotosEnablementState, GooglePhotosPhoto, GooglePhotosPhotosByAlbumId, initializeGooglePhotosData, PersonalizationActionName, SetErrorAction, WallpaperGridItem, WallpaperLayout, WallpaperType} from 'chrome://personalization/trusted/personalization_app.js';
+import {fetchGooglePhotosAlbum, fetchGooglePhotosAlbums, fetchGooglePhotosPhotos, GooglePhotosAlbum, GooglePhotosEnablementState, GooglePhotosPhoto, GooglePhotosPhotosByAlbumId, initializeGooglePhotosData, PersonalizationActionName, SetErrorAction, WallpaperGridItem, WallpaperLayout, WallpaperType} from 'chrome://personalization/trusted/personalization_app.js';
 import {assertDeepEquals, assertEquals, assertNotEquals} from 'chrome://webui-test/chai_assert.js';
 import {waitAfterNextRender} from 'chrome://webui-test/test_util.js';
 
@@ -277,6 +277,7 @@ suite('GooglePhotosPhotosByAlbumIdTest', function() {
     // Initialize Google Photos data in the |personalizationStore|.
     await initializeGooglePhotosData(wallpaperProvider, personalizationStore);
     await fetchGooglePhotosAlbums(wallpaperProvider, personalizationStore);
+    await fetchGooglePhotosPhotos(wallpaperProvider, personalizationStore);
 
     // The wallpaper controller is expected to impose max resolution.
     album.preview.url += '=s512';
@@ -474,6 +475,7 @@ suite('GooglePhotosPhotosByAlbumIdTest', function() {
     // Initialize Google Photos data in |personalizationStore|.
     await initializeGooglePhotosData(wallpaperProvider, personalizationStore);
     await fetchGooglePhotosAlbums(wallpaperProvider, personalizationStore);
+    await fetchGooglePhotosPhotos(wallpaperProvider, personalizationStore);
     assertDeepEquals(
         await wallpaperProvider.whenCalled('fetchGooglePhotosPhotos'),
         [/*itemId=*/ null, /*albumId=*/ null, /*resumeToken=*/ null]);

diff --git a/chrome/test/data/webui/chromeos/personalization_app/google_photos_photos_element_test.ts b/chrome/test/data/webui/chromeos/personalization_app/google_photos_photos_element_test.ts
@@ -4,7 +4,7 @@
 import 'chrome://personalization/strings.m.js';
 import 'chrome://webui-test/mojo_webui_test_support.js';
 
-import {getNumberOfGridItemsPerRow, GooglePhotosPhoto, GooglePhotosPhotos, GooglePhotosPhotosSection, initializeGooglePhotosData, PersonalizationActionName, SetErrorAction, WallpaperGridItem, WallpaperLayout, WallpaperType} from 'chrome://personalization/trusted/personalization_app.js';
+import {fetchGooglePhotosPhotos, getNumberOfGridItemsPerRow, GooglePhotosPhoto, GooglePhotosPhotos, GooglePhotosPhotosSection, initializeGooglePhotosData, PersonalizationActionName, SetErrorAction, WallpaperGridItem, WallpaperLayout, WallpaperType} from 'chrome://personalization/trusted/personalization_app.js';
 import {String16} from 'chrome://resources/mojo/mojo/public/mojom/base/string16.mojom-webui.js';
 import {assertDeepEquals, assertEquals, assertNotEquals} from 'chrome://webui-test/chai_assert.js';
 import {waitAfterNextRender} from 'chrome://webui-test/test_util.js';
@@ -185,6 +185,7 @@ suite('GooglePhotosPhotosTest', function() {
 
     // Initialize Google Photos data in the |personalizationStore|.
     await initializeGooglePhotosData(wallpaperProvider, personalizationStore);
+    await fetchGooglePhotosPhotos(wallpaperProvider, personalizationStore);
 
     // Initialize |googlePhotosPhotosElement|.
     googlePhotosPhotosElement =
@@ -262,6 +263,8 @@ suite('GooglePhotosPhotosTest', function() {
                 PersonalizationActionName.SET_ERROR);
             await initializeGooglePhotosData(
                 wallpaperProvider, personalizationStore);
+            await fetchGooglePhotosPhotos(
+                wallpaperProvider, personalizationStore);
             const {error} =
                 await personalizationStore.waitForAction(
                     PersonalizationActionName.SET_ERROR) as SetErrorAction;
@@ -358,6 +361,7 @@ suite('GooglePhotosPhotosTest', function() {
 
     // Initialize Google Photos data in the |personalizationStore|.
     await initializeGooglePhotosData(wallpaperProvider, personalizationStore);
+    await fetchGooglePhotosPhotos(wallpaperProvider, personalizationStore);
     await waitAfterNextRender(googlePhotosPhotosElement);
 
     // The wallpaper controller is expected to impose max resolution.
@@ -432,6 +436,7 @@ suite('GooglePhotosPhotosTest', function() {
 
     // Initialize Google Photos data in the |personalizationStore|.
     await initializeGooglePhotosData(wallpaperProvider, personalizationStore);
+    await fetchGooglePhotosPhotos(wallpaperProvider, personalizationStore);
 
     // The wallpaper controller is expected to impose max resolution.
     photo.url.url += '=s512';
@@ -607,6 +612,7 @@ suite('GooglePhotosPhotosTest', function() {
 
     // Initialize Google Photos data in |personalizationStore|.
     await initializeGooglePhotosData(wallpaperProvider, personalizationStore);
+    await fetchGooglePhotosPhotos(wallpaperProvider, personalizationStore);
     assertDeepEquals(
         await wallpaperProvider.whenCalled('fetchGooglePhotosPhotos'),
         [/*itemId=*/ null, /*albumId=*/ null, /*resumeToken=*/ null]);
@@ -713,6 +719,7 @@ suite('GooglePhotosPhotosTest', function() {
 
     // Initialize Google Photos data in the |personalizationStore|.
     await initializeGooglePhotosData(wallpaperProvider, personalizationStore);
+    await fetchGooglePhotosPhotos(wallpaperProvider, personalizationStore);
     wallpaperProvider.reset();
 
     // Initialize |googlePhotosPhotosElement| in hidden state.
@@ -750,6 +757,7 @@ suite('GooglePhotosPhotosTest', function() {
 
     // Initialize Google Photos data in the |personalizationStore|.
     await initializeGooglePhotosData(wallpaperProvider, personalizationStore);
+    await fetchGooglePhotosPhotos(wallpaperProvider, personalizationStore);
 
     // The wallpaper controller is expected to impose max resolution.
     photo.url.url += '=s512';

diff --git a/chrome/test/data/webui/chromeos/personalization_app/personalization_app_controller_test.ts b/chrome/test/data/webui/chromeos/personalization_app/personalization_app_controller_test.ts
@@ -53,14 +53,9 @@ suite('Personalization app controller', () => {
 
       await initializeGooglePhotosData(wallpaperProvider, personalizationStore);
 
-      let expectedEnabled, expectedPhotos;
-      if (isGooglePhotosIntegrationEnabled) {
-        expectedEnabled = GooglePhotosEnablementState.kEnabled;
-        expectedPhotos = [];
-      } else {
-        expectedEnabled = GooglePhotosEnablementState.kError;
-        expectedPhotos = null;
-      }
+      const expectedEnabled = isGooglePhotosIntegrationEnabled ?
+          GooglePhotosEnablementState.kEnabled :
+          GooglePhotosEnablementState.kError;
 
       assertDeepEquals(
           [
@@ -71,14 +66,6 @@ suite('Personalization app controller', () => {
               name: 'set_google_photos_enabled',
               enabled: expectedEnabled,
             },
-            {
-              name: 'begin_load_google_photos_photos',
-            },
-            {
-              name: 'append_google_photos_photos',
-              photos: expectedPhotos,
-              resumeToken: null,
-            },
           ],
           personalizationStore.actions);
 
@@ -116,38 +103,6 @@ suite('Personalization app controller', () => {
                 resumeTokens: {albums: null, photos: null, photosByAlbumId: {}},
               },
             },
-            // BEGIN_LOAD_GOOGLE_PHOTOS_PHOTOS.
-            {
-              'wallpaper.loading.googlePhotos': {
-                enabled: false,
-                albums: false,
-                photos: true,
-                photosByAlbumId: {},
-              },
-              'wallpaper.googlePhotos': {
-                enabled: expectedEnabled,
-                albums: undefined,
-                photos: undefined,
-                photosByAlbumId: {},
-                resumeTokens: {albums: null, photos: null, photosByAlbumId: {}},
-              },
-            },
-            // APPEND_GOOGLE_PHOTOS_PHOTOS.
-            {
-              'wallpaper.loading.googlePhotos': {
-                enabled: false,
-                albums: false,
-                photos: false,
-                photosByAlbumId: {},
-              },
-              'wallpaper.googlePhotos': {
-                enabled: expectedEnabled,
-                albums: undefined,
-                photos: expectedPhotos,
-                photosByAlbumId: {},
-                resumeTokens: {albums: null, photos: null, photosByAlbumId: {}},
-              },
-            },
           ],
           personalizationStore.states.map(filterAndFlattenState(
               ['wallpaper.googlePhotos', 'wallpaper.loading.googlePhotos'])));
@@ -170,7 +125,6 @@ suite('Personalization app controller', () => {
     }];
 
     wallpaperProvider.setGooglePhotosAlbums([album]);
-    wallpaperProvider.setGooglePhotosPhotos(photos);
     wallpaperProvider.setGooglePhotosPhotosByAlbumId(album.id, photos);
 
     // Attempts to `fetchGooglePhotosAlbum()` will fail unless the entire list
@@ -221,7 +175,7 @@ suite('Personalization app controller', () => {
                   preview: album.preview,
                 },
               ],
-              photos: photos,
+              photos: undefined,
               photosByAlbumId: {},
               resumeTokens: {albums: null, photos: null, photosByAlbumId: {}},
             },
@@ -244,7 +198,7 @@ suite('Personalization app controller', () => {
                   preview: album.preview,
                 },
               ],
-              photos: photos,
+              photos: undefined,
               photosByAlbumId: {
                 [album.id]: photos,
               },