Skip to content

Commit

Permalink
personalization: re-fetch screensaver albums
Browse files Browse the repository at this point in the history
When user navigates back to the albums subpage, begin a
request to refresh available google photos albums.
When the user is on ambient subpage, focuses another window,
and then comes back to personalization app, also begin another
request to refresh google photos albums. This allows a user
to edit albums live in photos.google.com and see immediate updates
in personalization app.

BUG=b:238198024
TEST=browser_tests --gtest_filter="*PersonalizationAppAmbient*"

Change-Id: I3a545c8f503cce757e38f93ff5a6d56beed79b41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3792352
Commit-Queue: Jeffrey Young <cowmoo@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1031145}
  • Loading branch information
cwmoo740 authored and Chromium LUCI CQ committed Aug 3, 2022
1 parent fc3765d commit 9f62fa6
Show file tree
Hide file tree
Showing 15 changed files with 179 additions and 37 deletions.
2 changes: 1 addition & 1 deletion ash/ambient/backdrop/ambient_backend_controller_impl.cc
Expand Up @@ -723,7 +723,7 @@ void AmbientBackendControllerImpl::OnGetGooglePhotosAlbumsPreview(
std::vector<GURL> preview_urls;
for (const std::string& preview_url :
get_google_photos_albums_preview_response.preview_url()) {
preview_urls.push_back(GURL(preview_url));
preview_urls.emplace_back(preview_url);
}
std::move(callback).Run(preview_urls);
}
Expand Down
3 changes: 3 additions & 0 deletions ash/webui/personalization_app/mojom/personalization_app.mojom
Expand Up @@ -529,6 +529,9 @@ interface AmbientProvider {

// Record that the user viewed the Ambient mode page.
SetPageViewed();

// Request updated Ambient mode settings and albums.
FetchSettingsAndAlbums();
};

// The supported preset colors for keyboard backlight.
Expand Down
Expand Up @@ -78,6 +78,7 @@ export class AlbumsSubpage extends WithPersonalizationStore {
this.watch<AlbumsSubpage['ambientModeEnabled_']>(
'ambientModeEnabled_', state => state.ambient.ambientModeEnabled);
this.updateFromStore();
getAmbientProvider().fetchSettingsAndAlbums();
}

private shouldShowContent_(): boolean {
Expand Down
Expand Up @@ -9,8 +9,9 @@ import {PersonalizationStore} from '../personalization_store.js';

import {setAlbumsAction, setAmbientModeEnabledAction, setAnimationThemeAction, setGooglePhotosAlbumsPreviewsAction, setTemperatureUnitAction, setTopicSourceAction} from './ambient_actions.js';
import {getAmbientProvider} from './ambient_interface_provider.js';
import {isRecentHighlightsAlbum} from './utils.js';

/** @fileoverview listens for updates on color mode changes. */
/** @fileoverview listens for updates on ambient mode changes. */

let instance: AmbientObserver|null = null;

Expand All @@ -31,8 +32,13 @@ export class AmbientObserver implements AmbientObserverInterface {
}
}

private receiver_: AmbientObserverReceiver =
this.initReceiver_(getAmbientProvider());
private receiver_: AmbientObserverReceiver;

constructor() {
const provider = getAmbientProvider();
this.receiver_ = this.initReceiver_(provider);
provider.fetchSettingsAndAlbums();
}

private initReceiver_(ambientProvider: AmbientProviderInterface):
AmbientObserverReceiver {
Expand Down Expand Up @@ -63,6 +69,21 @@ export class AmbientObserver implements AmbientObserverInterface {

onAlbumsChanged(albums: AmbientModeAlbum[]) {
const store = PersonalizationStore.getInstance();

// Prevent recent highlights album from constantly changing preview image
// when albums are refreshed during a single session.
const oldRecentHighlightsAlbum =
(store.data.ambient.albums ||
[]).find(album => isRecentHighlightsAlbum(album));
if (oldRecentHighlightsAlbum) {
const newRecentHighlightsAlbum =
albums.find(album => isRecentHighlightsAlbum(album));
if (newRecentHighlightsAlbum) {
// Edit by reference.
newRecentHighlightsAlbum.url = oldRecentHighlightsAlbum.url;
}
}

store.dispatch(setAlbumsAction(albums));
}

Expand Down
Expand Up @@ -77,6 +77,10 @@ export class AmbientSubpage extends WithPersonalizationStore {
private temperatureUnit_: TemperatureUnit|null = null;
private topicSource_: TopicSource|null = null;

// Refetch albums if the user is currently viewing ambient subpage, focuses
// another window, and then re-focuses personalization app.
private onFocus_ = () => getAmbientProvider().fetchSettingsAndAlbums();

override ready() {
// Pre-scroll to prevent visual jank when focusing the toggle row.
window.scrollTo(0, 0);
Expand Down Expand Up @@ -111,6 +115,13 @@ export class AmbientSubpage extends WithPersonalizationStore {
this.updateFromStore();

getAmbientProvider().setPageViewed();

window.addEventListener('focus', this.onFocus_);
}

override disconnectedCallback() {
super.disconnectedCallback();
window.removeEventListener('focus', this.onFocus_);
}

private onClickAmbientModeButton_(event: Event) {
Expand Down
Expand Up @@ -65,7 +65,7 @@ export {KeyboardBacklight} from './keyboard_backlight/keyboard_backlight_element
export {setKeyboardBacklightProviderForTesting} from './keyboard_backlight/keyboard_backlight_interface_provider.js';
export {KeyboardBacklightObserver} from './keyboard_backlight/keyboard_backlight_observer.js';
export {Actions, DismissErrorAction, dismissErrorAction, PersonalizationActionName, SetErrorAction} from './personalization_actions.js';
export {AmbientModeAlbum, AmbientObserverInterface, AmbientObserverRemote, AmbientProviderInterface, AnimationTheme, BacklightColor, CurrentWallpaper, DefaultUserImage, FetchGooglePhotosAlbumsResponse, FetchGooglePhotosPhotosResponse, GooglePhotosAlbum, GooglePhotosEnablementState, GooglePhotosPhoto, KeyboardBacklightObserverInterface, KeyboardBacklightObserverRemote, KeyboardBacklightProviderInterface, OnlineImageType, TemperatureUnit, ThemeObserverInterface, ThemeObserverRemote, ThemeProviderInterface, TopicSource, UserImage, UserImageObserverInterface, UserImageObserverRemote, UserInfo, UserProviderInterface, WallpaperCollection, WallpaperImage, WallpaperLayout, WallpaperObserverInterface, WallpaperObserverRemote, WallpaperProviderInterface, WallpaperType} from './personalization_app.mojom-webui.js';
export * from './personalization_app.mojom-webui.js';
export {PersonalizationBreadcrumb} from './personalization_breadcrumb_element.js';
export {PersonalizationMain} from './personalization_main_element.js';
export {reduce} from './personalization_reducers.js';
Expand Down
Expand Up @@ -53,6 +53,7 @@ class FakePersonalizationAppAmbientProvider
ash::AmbientModeTopicSource topic_source,
bool selected) override {}
void SetPageViewed() override {}
void FetchSettingsAndAlbums() override {}

private:
mojo::Receiver<ash::personalization_app::mojom::AmbientProvider>
Expand Down
Expand Up @@ -55,6 +55,7 @@ class MockPersonalizationAppAmbientProvider
bool selected),
(override));
MOCK_METHOD(void, SetPageViewed, (), (override));
MOCK_METHOD(void, FetchSettingsAndAlbums, (), (override));
};

class MockPersonalizationAppKeyboardBacklightProvider
Expand Down
Expand Up @@ -120,8 +120,6 @@ void PersonalizationAppAmbientProviderImpl::SetAmbientObserver(
OnAnimationThemeChanged();

ResetLocalSettings();
// Will notify WebUI when fetches successfully.
FetchSettingsAndAlbums();
}

void PersonalizationAppAmbientProviderImpl::SetAmbientModeEnabled(
Expand Down Expand Up @@ -230,6 +228,24 @@ void PersonalizationAppAmbientProviderImpl::SetPageViewed() {
page_viewed_ = true;
}

void PersonalizationAppAmbientProviderImpl::FetchSettingsAndAlbums() {
// If there is an ongoing update, do not fetch. If update succeeds, it will
// update the UI with the new settings. If update fails, it will restore
// previous settings and update UI.
if (is_updating_backend_) {
has_pending_fetch_request_ = true;
return;
}

// TODO(b/161044021): Add a helper function to get all the albums. Currently
// only load 100 latest modified albums.
ash::AmbientBackendController::Get()->FetchSettingsAndAlbums(
kBannerWidthPx, kBannerHeightPx, /*num_albums=*/100,
base::BindOnce(
&PersonalizationAppAmbientProviderImpl::OnSettingsAndAlbumsFetched,
read_weak_factory_.GetWeakPtr()));
}

void PersonalizationAppAmbientProviderImpl::OnAmbientModeEnabledChanged() {
const bool enabled = IsAmbientModeEnabled();
if (ambient_observer_remote_.is_bound()) {
Expand Down Expand Up @@ -420,24 +436,6 @@ void PersonalizationAppAmbientProviderImpl::UpdateUIWithCachedSettings(
has_pending_fetch_request_ = false;
}

void PersonalizationAppAmbientProviderImpl::FetchSettingsAndAlbums() {
// If there is an ongoing update, do not fetch. If update succeeds, it will
// update the UI with the new settings. If update fails, it will restore
// previous settings and update UI.
if (is_updating_backend_) {
has_pending_fetch_request_ = true;
return;
}

// TODO(b/161044021): Add a helper function to get all the albums. Currently
// only load 100 latest modified albums.
ash::AmbientBackendController::Get()->FetchSettingsAndAlbums(
kBannerWidthPx, kBannerHeightPx, /*num_albums=*/100,
base::BindOnce(
&PersonalizationAppAmbientProviderImpl::OnSettingsAndAlbumsFetched,
read_weak_factory_.GetWeakPtr()));
}

void PersonalizationAppAmbientProviderImpl::OnSettingsAndAlbumsFetched(
const absl::optional<ash::AmbientSettings>& settings,
ash::PersonalAlbums personal_albums) {
Expand Down
Expand Up @@ -56,6 +56,7 @@ class PersonalizationAppAmbientProviderImpl
ash::AmbientModeTopicSource topic_source,
bool selected) override;
void SetPageViewed() override;
void FetchSettingsAndAlbums() override;

// Notify WebUI the latest values.
void OnAmbientModeEnabledChanged();
Expand Down Expand Up @@ -86,8 +87,6 @@ class PersonalizationAppAmbientProviderImpl
// `success` is true when update successfully.
void UpdateUIWithCachedSettings(bool success);

// Fetch settings and albums from Backdrop server.
void FetchSettingsAndAlbums();
void OnSettingsAndAlbumsFetched(
const absl::optional<ash::AmbientSettings>& settings,
ash::PersonalAlbums personal_albums);
Expand Down
Expand Up @@ -227,7 +227,10 @@ class PersonalizationAppAmbientProviderImplTest : public testing::Test {
ambient_provider_->SetAnimationTheme(animation_theme);
}

void FetchSettings() { ambient_provider_->FetchSettingsAndAlbums(); }
void FetchSettings() {
ambient_provider_remote()->FetchSettingsAndAlbums();
ambient_provider_remote().FlushForTesting();
}

void UpdateSettings() {
if (!ambient_provider_->settings_)
Expand Down Expand Up @@ -381,7 +384,7 @@ TEST_F(PersonalizationAppAmbientProviderImplTest,
EXPECT_TRUE(pref_service);
pref_service->SetBoolean(ash::ambient::prefs::kAmbientModeEnabled, false);
SetAmbientObserver();
ambient_provider_remote().FlushForTesting();
FetchSettings();
EXPECT_FALSE(ObservedAmbientModeEnabled());

pref_service->SetBoolean(ash::ambient::prefs::kAmbientModeEnabled, true);
Expand All @@ -393,7 +396,7 @@ TEST_F(PersonalizationAppAmbientProviderImplTest,
TEST_F(PersonalizationAppAmbientProviderImplTest,
ShouldCallOnAnimationThemeChanged) {
SetAmbientObserver();
ambient_provider_remote().FlushForTesting();
FetchSettings();
SetAnimationTheme(ash::AmbientAnimationTheme::kSlideshow);
EXPECT_EQ(ash::AmbientAnimationTheme::kSlideshow, ObservedAnimationTheme());
histogram_tester().ExpectBucketCount(kAmbientModeAnimationThemeHistogramName,
Expand All @@ -410,9 +413,8 @@ TEST_F(PersonalizationAppAmbientProviderImplTest,

TEST_F(PersonalizationAppAmbientProviderImplTest,
ShouldCallOnTopicSourceChanged) {
// Will fetch settings when observer is set.
SetAmbientObserver();
ambient_provider_remote().FlushForTesting();
FetchSettings();
ReplyFetchSettingsAndAlbums(/*success=*/true);
EXPECT_EQ(ash::AmbientModeTopicSource::kGooglePhotos, ObservedTopicSource());
EXPECT_FALSE(ObservedGooglePhotosAlbumsPreviews().empty());
Expand All @@ -422,9 +424,8 @@ TEST_F(PersonalizationAppAmbientProviderImplTest,
}

TEST_F(PersonalizationAppAmbientProviderImplTest, ShouldCallOnAlbumsChanged) {
// Will fetch settings when observer is set.
SetAmbientObserver();
ambient_provider_remote().FlushForTesting();
FetchSettings();
ReplyFetchSettingsAndAlbums(/*success=*/true);
auto albums = ObservedAlbums();
// The fake albums are set in FakeAmbientBackendControllerImpl. Hidden setting
Expand All @@ -435,9 +436,8 @@ TEST_F(PersonalizationAppAmbientProviderImplTest, ShouldCallOnAlbumsChanged) {

TEST_F(PersonalizationAppAmbientProviderImplTest,
ShouldCallOnTemperatureUnitChanged) {
// Will fetch settings when observer is set.
SetAmbientObserver();
ambient_provider_remote().FlushForTesting();
FetchSettings();
ReplyFetchSettingsAndAlbums(/*success=*/true);
EXPECT_EQ(ash::AmbientModeTemperatureUnit::kCelsius,
ObservedTemperatureUnit());
Expand Down
Expand Up @@ -50,6 +50,7 @@ ts_library("build_ts") {
"wallpaper_grid_item_element_test.ts",
"wallpaper_images_element_test.ts",
"wallpaper_observer_test.ts",
"ambient_observer_test.ts",
"wallpaper_preview_element_test.ts",
"wallpaper_selected_element_test.ts",
]
Expand Down
@@ -0,0 +1,100 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'chrome://personalization/strings.m.js';
import 'chrome://webui-test/mojo_webui_test_support.js';

import {AmbientActionName, AmbientModeAlbum, AmbientObserver, emptyState, SetAlbumsAction, TopicSource} from 'chrome://personalization/js/personalization_app.js';
import {assertDeepEquals, assertEquals} from 'chrome://webui-test/chai_assert.js';

import {baseSetup} from './personalization_app_test_utils.js';
import {TestAmbientProvider} from './test_ambient_interface_provider.js';
import {TestPersonalizationStore} from './test_personalization_store.js';

suite('AmbientObserverTest', function() {
let ambientProvider: TestAmbientProvider;
let personalizationStore: TestPersonalizationStore;

setup(() => {
const mocks = baseSetup();
ambientProvider = mocks.ambientProvider;
personalizationStore = mocks.personalizationStore;
AmbientObserver.initAmbientObserverIfNeeded();
});

teardown(() => {
AmbientObserver.shutdown();
});

test('requests fetchSettingsAndAlbums on first load', async () => {
await ambientProvider.whenCalled('fetchSettingsAndAlbums');
});

test('sets albums in store', async () => {
personalizationStore.setReducersEnabled(true);
// Make sure state starts as expected.
assertDeepEquals(emptyState(), personalizationStore.data);
assertEquals(null, personalizationStore.data.ambient.albums);

personalizationStore.expectAction(AmbientActionName.SET_ALBUMS);
ambientProvider.ambientObserverRemote!.onAlbumsChanged(
ambientProvider.albums);

const {albums} = await personalizationStore.waitForAction(
AmbientActionName.SET_ALBUMS) as SetAlbumsAction;

assertDeepEquals(ambientProvider.albums, albums);
});

test('keeps recent highlights preview image', async () => {
const initialAlbums: AmbientModeAlbum[] = [
{
id: 'RecentHighlights',
checked: false,
numberOfPhotos: 210,
title: 'Recent Highlights title',
description: 'Recent Highlights description',
topicSource: TopicSource.kGooglePhotos,
url: {url: 'asdf'},
},
{
id: 'abcdef',
checked: false,
numberOfPhotos: 3,
title: 'Another album',
description: 'Another album description',
topicSource: TopicSource.kGooglePhotos,
url: {url: 'qwerty'},
},
];
personalizationStore.data.ambient.albums = initialAlbums;

personalizationStore.expectAction(AmbientActionName.SET_ALBUMS);

ambientProvider.ambientObserverRemote!.onAlbumsChanged([
{
...initialAlbums[0]!,
url: {
url: 'new-recent-highlights-url',
},
},
{
...initialAlbums[1]!,
url: {
url: 'new-regular-album-url',
},
},
]);

const {albums} = await personalizationStore.waitForAction(
AmbientActionName.SET_ALBUMS) as SetAlbumsAction;

assertEquals('RecentHighlights', albums[0]!.id);
assertEquals(
'asdf', albums[0]!.url.url, 'kept original url for recent highlights');
assertEquals(
'new-regular-album-url', albums[1]!.url.url,
'used updated regular album url');
});
});
Expand Up @@ -38,7 +38,8 @@ var PersonalizationAppComponentBrowserTest = class extends PolymerTest {
}
};

[['AmbientPreviewTest', 'ambient_preview_element_test.js'],
[['AmbientObserverTest', 'ambient_observer_test.js'],
['AmbientPreviewTest', 'ambient_preview_element_test.js'],
['AmbientSubpageTest', 'ambient_subpage_element_test.js'],
['AvatarCameraTest', 'avatar_list_element_test.js'],
['AvatarListTest', 'avatar_list_element_test.js'],
Expand Down

0 comments on commit 9f62fa6

Please sign in to comment.