0

personalization: rework loading local images

Previously, on collections page, loaded up to three local image
thumbnails one at a time. This caused a lot of churn during initial
load as polymer recalculated several times.

New behavior is to only send local image data as a chunk once
three local images have finished loading.

Also remove paper-spinner-lite from collections page in preparation
for loading rework.

BUG=b/181697575
TEST=browser_tests --gtest_filter="PersonalizationApp*"

Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Change-Id: I58d24f743ae927aafddcf8c02a481ef6afda9ec1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3041370
Commit-Queue: Jeffrey Young <cowmoo@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#905343}
This commit is contained in:
Jeffrey Young
2021-07-26 18:29:23 +00:00
committed by Chromium LUCI CQ
parent d8167cb5f0
commit 8e3b827f16
7 changed files with 108 additions and 111 deletions
chrome/test/data/webui/chromeos/personalization_app
chromeos/components/personalization_app/resources

@ -33,30 +33,15 @@ export function WallpaperCollectionsTest() {
wallpaperCollectionsElement = null;
});
test('shows loading on startup', async () => {
wallpaperCollectionsElement = initElement(WallpaperCollections.is);
const spinner = wallpaperCollectionsElement.shadowRoot.querySelector(
'paper-spinner-lite');
assertTrue(!!spinner);
assertTrue(spinner.active);
const iframe =
wallpaperCollectionsElement.shadowRoot.querySelector('iframe');
assertTrue(iframe.hidden);
});
test('shows wallpaper collections when loaded', async () => {
test('sends wallpaper collections when loaded', async () => {
const {sendCollections: sendCollectionsPromise} =
promisifyIframeFunctionsForTesting();
wallpaperCollectionsElement = initElement(WallpaperCollections.is);
const spinner = wallpaperCollectionsElement.shadowRoot.querySelector(
'paper-spinner-lite');
assertTrue(!!spinner);
assertTrue(spinner.active);
personalizationStore.data.loading = {collections: false};
personalizationStore.data.loading = {
...personalizationStore.data.loading,
collections: false
};
personalizationStore.data.backdrop.collections =
wallpaperProvider.collections;
personalizationStore.notifyObservers();
@ -65,8 +50,6 @@ export function WallpaperCollectionsTest() {
const [target, data] = await sendCollectionsPromise;
await waitAfterNextRender(wallpaperCollectionsElement);
assertFalse(spinner.active);
const iframe =
wallpaperCollectionsElement.shadowRoot.querySelector('iframe');
assertFalse(iframe.hidden);
@ -81,6 +64,7 @@ export function WallpaperCollectionsTest() {
images: {},
};
personalizationStore.data.loading = {
...personalizationStore.data.loading,
collections: false,
images: {},
};
@ -123,6 +107,7 @@ export function WallpaperCollectionsTest() {
wallpaperCollectionsElement = initElement(WallpaperCollections.is);
personalizationStore.data.loading = {
...personalizationStore.data.loading,
collections: false,
local: {images: false}
};
@ -146,21 +131,19 @@ export function WallpaperCollectionsTest() {
test('shows error when fails to load', async () => {
wallpaperCollectionsElement = initElement(WallpaperCollections.is);
const spinner = wallpaperCollectionsElement.shadowRoot.querySelector(
'paper-spinner-lite');
assertTrue(spinner.active);
// No error displayed while loading.
const error =
wallpaperCollectionsElement.shadowRoot.querySelector('#error');
assertTrue(error.hidden);
personalizationStore.data.loading = {collections: false};
personalizationStore.data.loading = {
...personalizationStore.data.loading,
collections: false,
};
personalizationStore.data.backdrop.collections = null;
personalizationStore.notifyObservers();
await waitAfterNextRender(wallpaperCollectionsElement);
assertFalse(spinner.active);
assertFalse(error.hidden);
// Iframe should be hidden if there is an error.
@ -225,13 +208,13 @@ export function WallpaperCollectionsTest() {
});
test(
'sends the first local images that successfully load thumbnails',
'sends the first three local images that successfully load thumbnails',
async () => {
// Set up store data. Local image list is loaded, but thumbnails are
// still loading in.
personalizationStore.data.loading.local.images = false;
personalizationStore.data.local.images = [];
for (let i = 0; i < kMaximumLocalImagePreviews * 2; i++) {
for (let i = 0; i < kMaximumLocalImagePreviews; i++) {
personalizationStore.data.local.images.push(
{id: {high: BigInt(i * 2), low: BigInt(i)}, name: `local-${i}`});
personalizationStore.data.loading.local.data[`${i * 2},${i}`] = true;
@ -241,7 +224,7 @@ export function WallpaperCollectionsTest() {
wallpaperProvider.collections;
personalizationStore.data.loading.collections = false;
let {sendLocalImages, sendLocalImageData} =
const {sendLocalImages, sendLocalImageData} =
promisifyIframeFunctionsForTesting();
wallpaperCollectionsElement = initElement(WallpaperCollections.is);
@ -249,22 +232,19 @@ export function WallpaperCollectionsTest() {
await sendLocalImages;
// No thumbnails loaded so none sent.
assertEquals(0, wallpaperCollectionsElement.sentLocalImages_.size);
assertFalse(wallpaperCollectionsElement.didSendLocalImageData_);
// First thumbnail loads in.
personalizationStore.data.loading.local.data = {'0,0': false};
personalizationStore.data.local.data = {'0,0': 'local_data_0'};
personalizationStore.notifyObservers();
// This thumbnail should have just loaded in.
let sent = await sendLocalImageData;
assertDeepEquals(
['0,0'], Array.from(wallpaperCollectionsElement.sentLocalImages_));
assertEquals('0,0', unguessableTokenToString(sent[1].id));
assertEquals('local_data_0', sent[2]);
await wallpaperCollectionsElement.iframePromise_;
await waitAfterNextRender(wallpaperCollectionsElement);
sendLocalImageData =
promisifyIframeFunctionsForTesting().sendLocalImageData;
// Should not have sent any image data since more thumbnails are still
// loading.
assertFalse(wallpaperCollectionsElement.didSendLocalImageData_);
// Second thumbnail fails loading. Third succeeds.
personalizationStore.data.loading.local.data = {
@ -279,14 +259,12 @@ export function WallpaperCollectionsTest() {
};
personalizationStore.notifyObservers();
sent = await sendLocalImageData;
// '4,2' successfully loaded and '2,1' did not. '4,2' should have been
// sent to iframe.
assertDeepEquals(
['0,0', '4,2'],
Array.from(wallpaperCollectionsElement.sentLocalImages_));
// 2 thumbnails have now loaded. 1 failed. But there are no more
// remaining to try loading, should send local image data anyway.
const [_, sentData] = await sendLocalImageData;
assertEquals('4,2', unguessableTokenToString(sent[1].id));
assertEquals('local_data_2', sent[2]);
assertTrue(wallpaperCollectionsElement.didSendLocalImageData_);
assertDeepEquals(
{'0,0': 'local_data_0', '4,2': 'local_data_2'}, sentData);
});
}

@ -67,10 +67,10 @@ export let SendImagesEvent;
export let SendLocalImagesEvent;
/**
* Sends local image data keyed by stringified local image id.
* @typedef {{
* type: EventType,
* id: !mojoBase.mojom.UnguessableToken,
* data: string,
* data: !Object<string, string>,
* }}
*/
export let SendLocalImageDataEvent;

@ -61,13 +61,13 @@ export function sendLocalImages(target, images) {
}
/**
* Sends image data keyed by stringified image id.
* @param {!Window} target
* @param {!chromeos.personalizationApp.mojom.LocalImage} image
* @param {!string} data
* @param {!Object<string, string>} data
*/
export function sendLocalImageData(target, image, data) {
export function sendLocalImageData(target, data) {
/** @type {!SendLocalImageDataEvent} */
const event = {type: EventType.SEND_LOCAL_IMAGE_DATA, id: image.id, data};
const event = {type: EventType.SEND_LOCAL_IMAGE_DATA, data};
target.postMessage(event, untrustedOrigin);
}
@ -167,7 +167,9 @@ export function validateReceivedData(event, expectedEventType) {
* @type {
* SendCollectionsEvent|
* SendImagesEvent|
* SendSelectedWallpaperAssetIdEvent
* SendSelectedWallpaperAssetIdEvent|
* SendLocalImagesEvent|
* SendLocalImageDataEvent
* }
*/
const data = event.data;
@ -175,6 +177,9 @@ export function validateReceivedData(event, expectedEventType) {
case EventType.SEND_COLLECTIONS:
assert(isNonEmptyArray(data.collections), 'Expected collections array');
return data.collections;
case EventType.SEND_LOCAL_IMAGE_DATA:
assert(typeof data.data === 'object', 'Expected data object');
return data.data;
case EventType.SEND_LOCAL_IMAGES:
case EventType.SEND_IMAGES:
// Images array may be empty.

@ -131,9 +131,11 @@ js_library("wallpaper_breadcrumb_element") {
js_library("wallpaper_collections_element") {
deps = [
":personalization_controller",
":personalization_store",
":styles",
"../common:constants",
"//third_party/polymer/v3_0/components-chromium/paper-spinner:paper-spinner-lite",
"../common:iframe_api",
"../common:utils",
"//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
]
}

@ -1,9 +1,8 @@
<style include="trusted-style">
</style>
<paper-spinner-lite active="[[collectionsLoading_]]"></paper-spinner-lite>
<!-- TODO(b/181697575) handle error cases and update error string to UI spec -->
<p hidden$="[[!hasError_]]" id="error">error</p>
<iframe id="collections-iframe" frameBorder="0" hidden$="[[!showCollections_]]"
<iframe id="collections-iframe" frameBorder="0" hidden$="[[hasError_]]"
src="chrome-untrusted://personalization/untrusted/collections.html"
role="main" aria-label$="[[i18n('wallpaperCollections')]]">
</iframe>

@ -8,13 +8,11 @@
* objects.
*/
import 'chrome://resources/polymer/v3_0/iron-list/iron-list.js';
import 'chrome://resources/polymer/v3_0/paper-spinner/paper-spinner-lite.js';
import './styles.js';
import {afterNextRender, html} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {kMaximumLocalImagePreviews} from '../common/constants.js';
import {sendCollections, sendImageCounts, sendLocalImageData, sendLocalImages} from '../common/iframe_api.js';
import {isNonEmptyArray, promisifyOnload, unguessableTokenToString} from '../common/utils.js';
import {kMaximumLocalImagePreviews} from '../common/constants.js';
import {getWallpaperProvider} from './mojo_interface_provider.js';
import {initializeBackdropData, initializeLocalData} from './personalization_controller.js';
import {WithPersonalizationStore} from './personalization_store.js';
@ -68,6 +66,7 @@ export class WallpaperCollections extends WithPersonalizationStore {
*/
collections_: {
type: Array,
observer: 'onCollectionsChanged_',
},
/** @private */
@ -95,6 +94,15 @@ export class WallpaperCollections extends WithPersonalizationStore {
observer: 'onLocalImagesChanged_',
},
/**
* Stores a mapping of local image id to loading status.
* @private
* @type {!Object<string, boolean>}
*/
localImageDataLoading_: {
type: Object,
},
/**
* Stores a mapping of local image id to thumbnail data.
* @private
@ -102,7 +110,6 @@ export class WallpaperCollections extends WithPersonalizationStore {
*/
localImageData_: {
type: Object,
observer: 'onLocalImageDataChanged_',
},
/** @private */
@ -112,18 +119,13 @@ export class WallpaperCollections extends WithPersonalizationStore {
// polymer knows when to re-run the computation.
computed: 'computeHasError_(collections_, collectionsLoading_)',
},
/** @private */
showCollections_: {
type: Boolean,
computed: 'computeShowCollections_(collections_, collectionsLoading_)',
observer: 'onShowCollectionsChanged_',
},
};
}
static get observers() {
return ['onLocalImageDataChanged_(localImages_, localImageData_)'];
return [
'onLocalImageDataChanged_(localImages_, localImageData_, localImageDataLoading_)',
];
}
constructor() {
@ -134,11 +136,9 @@ export class WallpaperCollections extends WithPersonalizationStore {
promisifyOnload(this, 'collections-iframe', afterNextRender));
/**
* Stores a set of local image ids that have already sent thumbnail data to
* untrusted.
* @type {!Set<string>}
* @type {boolean}
*/
this.sentLocalImages_ = new Set();
this.didSendLocalImageData_ = false;
}
/** @override */
@ -149,6 +149,7 @@ export class WallpaperCollections extends WithPersonalizationStore {
this.watch('images_', state => state.backdrop.images);
this.watch('localImages_', state => state.local.images);
this.watch('localImageData_', state => state.local.data);
this.watch('localImageDataLoading_', state => state.loading.local.data);
this.updateFromStore();
const store = this.getStore();
initializeBackdropData(this.wallpaperProvider_, store);
@ -163,28 +164,18 @@ export class WallpaperCollections extends WithPersonalizationStore {
* @return {boolean}
*/
computeHasError_(collections, loading) {
return !loading && !this.computeShowCollections_(collections, loading);
}
/**
* @private
* @param {?Array<!chromeos.personalizationApp.mojom.WallpaperCollection>}
* collections
* @param {boolean} loading
* @return {boolean}
*/
computeShowCollections_(collections, loading) {
return !loading && isNonEmptyArray(collections);
return !loading && !isNonEmptyArray(collections);
}
/**
* Send updated wallpaper collections to the iframe.
* @param {?boolean} value
* @param {?Array<!chromeos.personalizationApp.mojom.WallpaperCollection>}
* collections
*/
async onShowCollectionsChanged_(value) {
if (value) {
async onCollectionsChanged_(collections) {
if (isNonEmptyArray(collections)) {
const iframe = await this.iframePromise_;
sendCollectionsFunction(iframe.contentWindow, this.collections_);
sendCollectionsFunction(iframe.contentWindow, collections);
}
}
@ -225,27 +216,46 @@ export class WallpaperCollections extends WithPersonalizationStore {
* Send up to |maximumImageThumbnailsCount| image thumbnails to untrusted.
* @param {?Array<!chromeos.personalizationApp.mojom.LocalImage>} images
* @param {?Object<string, string>} imageData
* @param {?Object<string, boolean>} imageDataLoading
*/
async onLocalImageDataChanged_(images, imageData) {
if (!Array.isArray(images) || !imageData) {
async onLocalImageDataChanged_(images, imageData, imageDataLoading) {
if (!Array.isArray(images) || !imageData || !imageDataLoading ||
this.didSendLocalImageData_) {
return;
}
const iframe = await this.iframePromise_;
for (const image of images) {
if (this.sentLocalImages_.size >= kMaximumLocalImagePreviews) {
return;
}
const key = unguessableTokenToString(image.id);
if (this.sentLocalImages_.has(key)) {
continue;
}
const data = imageData[key];
if (data) {
sendLocalImageDataFunction(
/** @type {!Window} */ (iframe.contentWindow), image, data);
this.sentLocalImages_.add(key);
}
/** @type !Array<string> */
const successfullyLoaded =
images.map(image => unguessableTokenToString(image.id)).filter(key => {
const doneLoading = imageDataLoading[key] === false;
const success = !!imageData[key];
return success && doneLoading;
});
function shouldSendImageData() {
// All images (up to |kMaximumLocalImagePreviews|) have loaded.
const didLoadMaximum = successfullyLoaded.length >=
Math.min(kMaximumLocalImagePreviews, images.length);
return didLoadMaximum ||
// No more images to load so send now even if some failed.
images.every(
image => imageDataLoading[unguessableTokenToString(image.id)] ===
false);
};
if (shouldSendImageData()) {
const data =
successfullyLoaded.filter((_, i) => i < kMaximumLocalImagePreviews)
.reduce((result, key) => {
result[key] = imageData[key];
return result;
}, {});
const iframe = await this.iframePromise_;
sendLocalImageDataFunction(
/** @type {!Window} */ (iframe.contentWindow), data);
this.didSendLocalImageData_ = true;
}
}
}

@ -219,10 +219,13 @@ class CollectionsGrid extends PolymerElement {
}
break;
case EventType.SEND_LOCAL_IMAGE_DATA:
this.localImageData_ = {
...this.localImageData_,
[unguessableTokenToString(message.data.id)]: message.data.data,
};
try {
this.localImageData_ =
validateReceivedData(message, EventType.SEND_LOCAL_IMAGE_DATA);
} catch (e) {
console.warn('Invalid local image data received', e);
this.localImageData_ = {};
}
break;
default:
console.error(`Unexpected event type ${message.data.type}`);