Skip to content

Commit

Permalink
[M103 merge] personalization: skip re-select default image
Browse files Browse the repository at this point in the history
If the device default image is already selected, do not issue
another request to select it when clicked by user.
Fixes a bug where loading state gets stuck when re-selecting
default wallpaper.

BUG=b:231958982
TEST=browser_tests --gtest_filter="*PersonalizationAppController*"

(cherry picked from commit 9c77912)

Change-Id: I9aa305f174175b8c5bf7348fc5d6beb3ce15689e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3646696
Reviewed-by: Tao Wu <wutao@chromium.org>
Commit-Queue: Jeffrey Young <cowmoo@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1004543}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3655264
Auto-Submit: Jeffrey Young <cowmoo@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/branch-heads/5060@{#147}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
cwmoo740 authored and Chromium LUCI CQ committed May 20, 2022
1 parent d664551 commit 9082392
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export {setUserProviderForTesting} from './user/user_interface_provider.js';
export {UserPreview} from './user/user_preview_element.js';
export {UserSubpage} from './user/user_subpage_element.js';
export {GetUserMediaProxy, getWebcamUtils, setWebcamUtilsForTesting} from './user/webcam_utils_proxy.js';
export {getImageKey, isDefaultImage, isFilePath} from './utils.js';
export {getImageKey, isDefaultImage, isFilePath, isGooglePhotosPhoto, isWallpaperImage} from './utils.js';
export {GooglePhotosAlbums} from './wallpaper/google_photos_albums_element.js';
export {GooglePhotosCollection} from './wallpaper/google_photos_collection_element.js';
export {GooglePhotosPhotosByAlbumId} from './wallpaper/google_photos_photos_by_album_id_element.js';
Expand Down
19 changes: 18 additions & 1 deletion ash/webui/personalization_app/resources/trusted/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {String16} from 'chrome://resources/mojo/mojo/public/mojom/base/string16.
import {Url} from 'chrome://resources/mojo/url/mojom/url.mojom-webui.js';

import {DefaultImageSymbol, DisplayableImage, kDefaultImageSymbol} from '../common/constants.js';
import {AmbientModeAlbum, GooglePhotosPhoto, TopicSource, WallpaperImage, WallpaperLayout} from '../trusted/personalization_app.mojom-webui.js';
import {AmbientModeAlbum, CurrentWallpaper, GooglePhotosPhoto, TopicSource, WallpaperImage, WallpaperLayout, WallpaperType} from '../trusted/personalization_app.mojom-webui.js';


export function isWallpaperImage(obj: any): obj is WallpaperImage {
Expand Down Expand Up @@ -50,6 +50,23 @@ export function getImageKey(image: DisplayableImage): string|
return image.id;
}

/**
* Compare an image from the list of selectable images with the currently
* selected user wallpaper.
* @param image a selectable image that the user can choose
* @param selected currently selected user walpaper
* @return boolean whether they are considered the same image
*/
export function isImageEqualToSelected(
image: DisplayableImage, selected: CurrentWallpaper): boolean {
if (isDefaultImage(image)) {
// Special case for default images. Mojom generated code for type
// |CurrentWallpaper.key| cannot include javascript symbols.
return selected.type === WallpaperType.kDefault;
}
return getImageKey(image) === selected.key;
}

/**
* Subtly different than |getImageKey|, which returns just the file part of the
* path. |getPathOrSymbol| returns the whole path for local images.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {DisplayableImage} from '../../common/constants.js';
import {isNonEmptyArray} from '../../common/utils.js';
import {GooglePhotosAlbum, GooglePhotosEnablementState, GooglePhotosPhoto, WallpaperCollection, WallpaperLayout, WallpaperProviderInterface, WallpaperType} from '../personalization_app.mojom-webui.js';
import {PersonalizationStore} from '../personalization_store.js';
import {appendMaxResolutionSuffix, getImageKey, isDefaultImage, isFilePath, isGooglePhotosPhoto, isWallpaperImage} from '../utils.js';
import {appendMaxResolutionSuffix, isDefaultImage, isFilePath, isGooglePhotosPhoto, isImageEqualToSelected, isWallpaperImage} from '../utils.js';

import * as action from './wallpaper_actions.js';

Expand Down Expand Up @@ -280,7 +280,7 @@ export async function selectWallpaper(
store: PersonalizationStore,
layout: WallpaperLayout = WallpaperLayout.kCenterCropped): Promise<void> {
const currentWallpaper = store.data.wallpaper.currentSelected;
if (currentWallpaper && currentWallpaper.key === getImageKey(image)) {
if (currentWallpaper && isImageEqualToSelected(image, currentWallpaper)) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
import 'chrome://personalization/strings.m.js';
import 'chrome://webui-test/mojo_webui_test_support.js';

import {cancelPreviewWallpaper, DefaultImageSymbol, fetchCollections, fetchGooglePhotosAlbum, fetchGooglePhotosAlbums, fetchLocalData, getDefaultImageThumbnail, getImageKey, getLocalImages, GooglePhotosAlbum, GooglePhotosEnablementState, GooglePhotosPhoto, initializeBackdropData, initializeGooglePhotosData, isFilePath, kDefaultImageSymbol, selectWallpaper, WallpaperImage} from 'chrome://personalization/trusted/personalization_app.js';
import {cancelPreviewWallpaper, DefaultImageSymbol, DisplayableImage, fetchCollections, fetchGooglePhotosAlbum, fetchGooglePhotosAlbums, fetchLocalData, getDefaultImageThumbnail, getImageKey, getLocalImages, GooglePhotosAlbum, GooglePhotosEnablementState, GooglePhotosPhoto, initializeBackdropData, initializeGooglePhotosData, isDefaultImage, isFilePath, isGooglePhotosPhoto, isWallpaperImage, kDefaultImageSymbol, selectWallpaper, WallpaperType} from 'chrome://personalization/trusted/personalization_app.js';
import {assertNotReached} from 'chrome://resources/js/assert_ts.js';
import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
import {FilePath} from 'chrome://resources/mojo/mojo/public/mojom/base/file_path.mojom-webui.js';
import {assertDeepEquals, assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';
Expand Down Expand Up @@ -787,10 +788,25 @@ suite('does not respond to re-selecting the current wallpaper', () => {
wallpaperProvider.isInTabletModeResponse = false;
});

function getImageType(image: DisplayableImage): WallpaperType {
if (isDefaultImage(image)) {
return WallpaperType.kDefault;
}
if (isGooglePhotosPhoto(image)) {
return WallpaperType.kOnceGooglePhotos;
}
if (isWallpaperImage(image)) {
return WallpaperType.kOnline;
}
if (isFilePath(image)) {
return WallpaperType.kCustomized;
}
assertNotReached('unknown wallpaper type');
}

// Selects `image` as the wallpaper twice and verifies that the second attempt
// quits early because there is no work to do.
async function testReselectWallpaper(image: WallpaperImage|FilePath|
GooglePhotosPhoto) {
async function testReselectWallpaper(image: DisplayableImage) {
const selectWallpaperActions = [
{
name: 'begin_select_image',
Expand All @@ -811,11 +827,13 @@ suite('does not respond to re-selecting the current wallpaper', () => {
assertDeepEquals(personalizationStore.actions, selectWallpaperActions);

// Complete the pending selection as would happen in production code.
const selected = personalizationStore.data.wallpaper.pendingSelected;
assertEquals(selected, image);
const pendingSelected = personalizationStore.data.wallpaper.pendingSelected;
assertEquals(pendingSelected, image);
personalizationStore.data.wallpaper.currentSelected = {
key: getImageKey(image)
key: getImageKey(image),
type: getImageType(image),
};
personalizationStore.data.wallpaper.pendingSelected = null;

// Select the same wallpaper and verify that no further actions are taken.
await selectWallpaper(image, wallpaperProvider, personalizationStore);
Expand Down Expand Up @@ -862,6 +880,13 @@ suite('does not respond to re-selecting the current wallpaper', () => {

await testReselectWallpaper(image);
});

test('re-selects default image', async () => {
// Reset the history of actions and prior states, but keep the current
// state.
personalizationStore.reset(personalizationStore.data);
await testReselectWallpaper(kDefaultImageSymbol);
});
});

suite('updates default image', () => {
Expand Down

0 comments on commit 9082392

Please sign in to comment.