Skip to content

Commit

Permalink
personalization: enable D/L theme transition for Personalization Hub
Browse files Browse the repository at this point in the history
In tablet mode, while changing wallpaper, the window needs to be set as
transparent so as to preview the wallpaper in full screen. It conflicts
with the D/L transition. This commit fixes the issue by setting the
window back to opaque after exiting full-screen review.

To test:
1. launch in tablet mode (--force-tablet-mode=touch_view) with
--enable-features=WallpaperWebUI,WallpaperFullScreenPreview, \
PersonalizationHub,DarkLightMode
2. Open the Personalization Hub
3. Under the Wallpaper Section, choose between Dark/Light theme. The
   desktop icons should not show up.
4. Choose a different wallpaper, full-screen preview of the wallpaper
   is still active.
5. Go back to the Personalization Hub, and choose D/L theme again. The
   transition animation should be the same as step 3.

BUG=b:225059638

Change-Id: Ib5dbed521fb35a758b7857132332b2d47e7720b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3550018
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: Jerry Liu <pzliu@google.com>
Cr-Commit-Position: refs/heads/main@{#988072}
  • Loading branch information
lpzjerry authored and Chromium LUCI CQ committed Apr 1, 2022
1 parent 990e3ab commit c610c8a
Show file tree
Hide file tree
Showing 12 changed files with 133 additions and 4 deletions.
5 changes: 5 additions & 0 deletions ash/webui/personalization_app/mojom/personalization_app.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,11 @@ interface WallpaperProvider {
// to call multiple times.
MakeTransparent();

// Called to reverse `MakeTransparent()`. Being called while exiting
// fullscreen preview to avoid unexpected transparency. Safe to call
// multiple times, even before `MakeTransparent()` is called.
MakeOpaque();

// Fetch a list of WallpaperCollection objects from the Backdrop API. Will
// be displayed to the user to allow them to select an individual collection
// to view in more detail. |collections| will be null on error.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {assert} from 'chrome://resources/js/assert.m.js';

import {PersonalizationStore} from './personalization_store.js';
import {setFullscreenEnabledAction} from './wallpaper/wallpaper_actions.js';
import {getWallpaperProvider} from './wallpaper/wallpaper_interface_provider.js';

/**
* @fileoverview provides useful functions for e2e browsertests.
Expand All @@ -17,10 +18,21 @@ function enterFullscreen() {
store.dispatch(setFullscreenEnabledAction(true));
}

function makeTransparent() {
const wallpaperProvider = getWallpaperProvider();
wallpaperProvider.makeTransparent();
}

declare global {
interface Window {
personalizationTestApi: {enterFullscreen: () => void};
personalizationTestApi: {
enterFullscreen: () => void,
makeTransparent: () => void,
};
}
}

window.personalizationTestApi = {enterFullscreen};
window.personalizationTestApi = {
enterFullscreen,
makeTransparent
};
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,9 @@ export async function selectWallpaper(
const {tabletMode} = await provider.isInTabletMode();
const shouldPreview =
tabletMode && loadTimeData.getBoolean('fullScreenPreviewEnabled');
if (shouldPreview) {
provider.makeTransparent();
}
store.endBatchUpdate();
const {success} = await (() => {
if (isWallpaperImage(image)) {
Expand Down Expand Up @@ -354,12 +357,14 @@ export async function updateDailyRefreshWallpaper(
export async function confirmPreviewWallpaper(
provider: WallpaperProviderInterface): Promise<void> {
await provider.confirmPreviewWallpaper();
provider.makeOpaque();
}

/** Cancel preview wallpaper and show the previous wallpaper. */
export async function cancelPreviewWallpaper(
provider: WallpaperProviderInterface): Promise<void> {
await provider.cancelPreviewWallpaper();
provider.makeOpaque();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export function setWallpaperProviderForTesting(
export function getWallpaperProvider(): WallpaperProviderInterface {
if (!wallpaperProvider) {
wallpaperProvider = WallpaperProvider.getRemote();
wallpaperProvider.makeTransparent();
}
return wallpaperProvider;
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ class FakePersonalizationAppWallpaperProvider

void MakeTransparent() override {}

void MakeOpaque() override {}

void FetchCollections(FetchCollectionsCallback callback) override;

void FetchImagesForCollection(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "ash/webui/personalization_app/personalization_app_url_constants.h"
#include "base/callback.h"
#include "base/files/file_path.h"
#include "base/memory/raw_ptr.h"
#include "base/path_service.h"
#include "base/run_loop.h"
#include "base/scoped_observation.h"
Expand All @@ -35,6 +36,8 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/aura/window.h"
#include "ui/aura/window_observer.h"
#include "ui/base/class_property.h"
#include "ui/compositor/layer.h"
#include "ui/display/test/display_manager_test_api.h"
#include "ui/gfx/geometry/rect.h"
Expand Down Expand Up @@ -121,6 +124,53 @@ void AssertExpectedDebugImage(const SkBitmap& bitmap) {
<< error_bounding_rect.ToString();
}

template <typename T>
class WindowPropertyWaiter : public aura::WindowObserver {
public:
WindowPropertyWaiter(aura::Window* window, const ui::ClassProperty<T>* key)
: window_(window), key_(key) {}
~WindowPropertyWaiter() override = default;

void Wait() {
base::RunLoop loop;
quit_closure_ = loop.QuitClosure();

make_transparent_observation_.Observe(window_);

loop.Run();
}

void OnWindowPropertyChanged(aura::Window* window,
const void* key,
intptr_t old) override {
if (key != key_) {
return;
}

if (quit_closure_) {
std::move(quit_closure_).Run();
make_transparent_observation_.Reset();
}
}

private:
base::ScopedObservation<aura::Window, aura::WindowObserver>
make_transparent_observation_{this};
base::OnceClosure quit_closure_;
aura::Window* window_;
const raw_ptr<const ui::ClassProperty<T>> key_;
};

void CallJavascriptAndWaitForPropertyChange(content::WebContents* web_contents,
const std::u16string& javascript) {
WindowPropertyWaiter<bool> window_property_waiter(
web_contents->GetTopLevelNativeWindow(),
chromeos::kWindowManagerManagesOpacityKey);
web_contents->GetMainFrame()->ExecuteJavaScriptForTests(javascript,
base::DoNothing());
window_property_waiter.Wait();
}

class WallpaperChangeWaiter : public ash::WallpaperControllerObserver {
public:
WallpaperChangeWaiter() = default;
Expand Down Expand Up @@ -240,6 +290,9 @@ IN_PROC_BROWSER_TEST_P(PersonalizationAppIntegrationTest,
Browser* browser;
content::WebContents* web_contents = LaunchAppAtWallpaperSubpage(&browser);

CallJavascriptAndWaitForPropertyChange(
web_contents, u"personalizationTestApi.makeTransparent();");

EXPECT_TRUE(web_contents->GetTopLevelNativeWindow()->GetTransparent());
EXPECT_FALSE(web_contents->GetTopLevelNativeWindow()->GetProperty(
chromeos::kWindowManagerManagesOpacityKey));
Expand All @@ -252,6 +305,9 @@ IN_PROC_BROWSER_TEST_P(PersonalizationAppIntegrationTest,
content::WebContents* web_contents = LaunchAppAtWallpaperSubpage(&browser);
aura::Window* window = web_contents->GetTopLevelNativeWindow();

CallJavascriptAndWaitForPropertyChange(
web_contents, u"personalizationTestApi.makeTransparent();");

ash::WindowBackdrop* window_backdrop = ash::WindowBackdrop::Get(window);
EXPECT_EQ(ash::WindowBackdrop::BackdropMode::kDisabled,
window_backdrop->mode());
Expand All @@ -265,6 +321,9 @@ IN_PROC_BROWSER_TEST_P(PersonalizationAppIntegrationTest,
Browser* browser;
content::WebContents* web_contents = LaunchAppAtWallpaperSubpage(&browser);

CallJavascriptAndWaitForPropertyChange(
web_contents, u"personalizationTestApi.makeTransparent();");

BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
EXPECT_EQ(SK_ColorTRANSPARENT,
browser_view->contents_web_view()->GetBackground()->get_color());
Expand Down Expand Up @@ -301,6 +360,9 @@ IN_PROC_BROWSER_TEST_P(PersonalizationAppIntegrationTest,
Browser* browser;
content::WebContents* web_contents = LaunchAppAtWallpaperSubpage(&browser);

CallJavascriptAndWaitForPropertyChange(
web_contents, u"personalizationTestApi.makeTransparent();");

WallpaperChangeWaiter wallpaper_changer;
wallpaper_changer.SetWallpaperAndWait();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,18 @@ void PersonalizationAppWallpaperProviderImpl::MakeTransparent() {
->SetBackgroundVisible(false);
}

void PersonalizationAppWallpaperProviderImpl::MakeOpaque() {
auto* web_contents = web_ui_->GetWebContents();

// Reversing `contents_web_view` is sufficient to make the view opaque,
// as `window_backdrop`, `top_level_window` and `web_contents` are not
// highly impactful to the animated theme change effect.
static_cast<ContentsWebView*>(BrowserView::GetBrowserViewForNativeWindow(
web_contents->GetTopLevelNativeWindow())
->contents_web_view())
->SetBackgroundVisible(true);
}

void PersonalizationAppWallpaperProviderImpl::FetchCollections(
FetchCollectionsCallback callback) {
pending_collections_callbacks_.push_back(std::move(callback));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ class PersonalizationAppWallpaperProviderImpl
// see the chosen wallpaper. This is safe to call multiple times in a row.
void MakeTransparent() override;

// Configure the window to be opaque so that after exiting the "full screen
// preview" mode, the transparent background will be reversed. This is safe
// to call multiple times in a row.
void MakeOpaque() override;

void FetchCollections(FetchCollectionsCallback callback) override;

void FetchImagesForCollection(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,7 @@ bool PersonalizationSystemAppDelegate::IsAppEnabled() const {
bool PersonalizationSystemAppDelegate::ShouldShowInLauncher() const {
return false;
}

bool PersonalizationSystemAppDelegate::ShouldAnimateThemeChanges() const {
return true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class PersonalizationSystemAppDelegate : public web_app::SystemWebAppDelegate {
bool ShouldCaptureNavigations() const override;
bool IsAppEnabled() const override;
bool ShouldShowInLauncher() const override;
bool ShouldAnimateThemeChanges() const override;
};

// Return a WebAppInstallInfo used to install the app.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import 'chrome://personalization/strings.m.js';
import 'chrome://webui-test/mojo_webui_test_support.js';

import {fetchCollections, fetchGooglePhotosAlbum, fetchLocalData, getLocalImages, GooglePhotosAlbum, GooglePhotosEnablementState, GooglePhotosPhoto, initializeBackdropData, initializeGooglePhotosData, selectWallpaper} from 'chrome://personalization/trusted/personalization_app.js';
import {cancelPreviewWallpaper, fetchCollections, fetchGooglePhotosAlbum, fetchLocalData, getLocalImages, GooglePhotosAlbum, GooglePhotosEnablementState, GooglePhotosPhoto, initializeBackdropData, initializeGooglePhotosData, selectWallpaper} from 'chrome://personalization/trusted/personalization_app.js';
import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
import {assertDeepEquals, assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';

Expand Down Expand Up @@ -670,6 +670,12 @@ suite('full screen mode', () => {
assertEquals(wallpaperProvider.images![0]!.assetId, assetId);

await selectWallpaperPromise;
assertEquals(
0, wallpaperProvider.getCallCount('makeTransparent'),
'makeTransparent is not called when fullscreen preview is off');
assertEquals(
0, wallpaperProvider.getCallCount('makeOpaque'),
'makeOpaque is not called when fullscreen preview is off');

assertFalse(personalizationStore.data.wallpaper.fullscreen);
}
Expand All @@ -680,6 +686,9 @@ suite('full screen mode', () => {
// Now with flag turned on.
loadTimeData.overrideValues({[fullscreenPreviewFeature]: true});

assertEquals(0, wallpaperProvider.getCallCount('makeTransparent'));
assertEquals(0, wallpaperProvider.getCallCount('makeOpaque'));

const selectWallpaperPromise = selectWallpaper(
wallpaperProvider.images![0]!, wallpaperProvider,
personalizationStore);
Expand All @@ -690,8 +699,16 @@ suite('full screen mode', () => {
assertEquals(wallpaperProvider.images![0]!.assetId, assetId);

await selectWallpaperPromise;
assertEquals(
1, wallpaperProvider.getCallCount('makeTransparent'),
'makeTransparent is called while calling selectWallpaper');

assertTrue(personalizationStore.data.wallpaper.fullscreen);

await cancelPreviewWallpaper(wallpaperProvider);
assertEquals(
1, wallpaperProvider.getCallCount('makeOpaque'),
'makeOpaque is called while calling cancelPreviewWallpaper');
}
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export class TestWallpaperProvider extends
constructor() {
super([
'makeTransparent',
'makeOpaque',
'fetchCollections',
'fetchImagesForCollection',
'fetchGooglePhotosAlbums',
Expand Down Expand Up @@ -137,6 +138,10 @@ export class TestWallpaperProvider extends
this.methodCalled('makeTransparent');
}

makeOpaque() {
this.methodCalled('makeOpaque');
}

fetchCollections() {
this.methodCalled('fetchCollections');
return Promise.resolve({collections: this.collections_});
Expand Down

0 comments on commit c610c8a

Please sign in to comment.