Skip to content

Commit

Permalink
personalization: Do not show screensaver if not allowed
Browse files Browse the repository at this point in the history
Will not show ambient subpage, if users go to
chrome://personalization/ambient when they could not yet. Will show
the hub main page instead.

Bug: b:223941242
Test: added new tests
Change-Id: I15c6112a19651ac22a52ae000ce7b4ac9b606028
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3527271
Reviewed-by: Jeffrey Young <cowmoo@chromium.org>
Commit-Queue: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/main@{#985143}
  • Loading branch information
Tao Wu authored and Chromium LUCI CQ committed Mar 25, 2022
1 parent 1d6e4b5 commit ed685a8
Show file tree
Hide file tree
Showing 11 changed files with 204 additions and 34 deletions.
1 change: 1 addition & 0 deletions ash/ambient/test/test_ambient_client.h
Expand Up @@ -28,6 +28,7 @@ class ASH_PUBLIC_EXPORT TestAmbientClient : public AmbientClient {

// AmbientClient:
bool IsAmbientModeAllowed() override;
void SetAmbientModeAllowedForTesting(bool allowed) override {}
void RequestAccessToken(GetAccessTokenCallback callback) override;
void DownloadImage(const std::string& url,
ash::ImageDownloader::DownloadCallback callback) override;
Expand Down
2 changes: 2 additions & 0 deletions ash/public/cpp/ambient/ambient_client.h
Expand Up @@ -41,6 +41,8 @@ class ASH_PUBLIC_EXPORT AmbientClient {
// Return whether the ambient mode is allowed for the user.
virtual bool IsAmbientModeAllowed() = 0;

virtual void SetAmbientModeAllowedForTesting(bool allowed) = 0;

// Return the gaia and access token associated with the active user's profile.
virtual void RequestAccessToken(GetAccessTokenCallback callback) = 0;

Expand Down
8 changes: 8 additions & 0 deletions ash/webui/personalization_app/personalization_app_ui.cc
Expand Up @@ -5,6 +5,7 @@
#include "ash/webui/personalization_app/personalization_app_ui.h"

#include "ash/constants/ash_features.h"
#include "ash/public/cpp/ambient/ambient_client.h"
#include "ash/webui/grit/ash_personalization_app_resources.h"
#include "ash/webui/grit/ash_personalization_app_resources_map.h"
#include "ash/webui/personalization_app/personalization_app_ambient_provider.h"
Expand Down Expand Up @@ -33,6 +34,11 @@ GURL GetGooglePhotosURL() {
return GURL(kGooglePhotosURL);
}

bool IsAmbientModeAllowed() {
return ash::AmbientClient::Get() &&
ash::AmbientClient::Get()->IsAmbientModeAllowed();
}

void AddResources(content::WebUIDataSource* source) {
source->AddResourcePath("", IDR_ASH_PERSONALIZATION_APP_TRUSTED_INDEX_HTML);
source->AddResourcePaths(base::make_span(
Expand Down Expand Up @@ -192,6 +198,8 @@ void AddBooleans(content::WebUIDataSource* source) {

source->AddBoolean("isDarkLightModeEnabled",
features::IsDarkLightModeEnabled());

source->AddBoolean("isAmbientModeAllowed", IsAmbientModeAllowed());
}

} // namespace
Expand Down
Expand Up @@ -62,13 +62,16 @@
<personalization-theme></personalization-theme>
</template>
</wallpaper-preview>
<ambient-preview main-page>
<div id="ambientLabel">
<p>$i18n{screensaverLabel}</p>
<cr-button id="ambientSubpageLink" on-click="onClickAmbientSubpageLink_">
<iron-icon icon="cr:chevron-right" aria-hidden="true"></iron-icon>
</cr-button>
</div>
</ambient-preview>
<template is="dom-if" if="[[isAmbientModeAllowed_()]]">
<ambient-preview main-page>
<div id="ambientLabel">
<p>$i18n{screensaverLabel}</p>
<cr-button id="ambientSubpageLink"
on-click="onClickAmbientSubpageLink_">
<iron-icon icon="cr:chevron-right" aria-hidden="true"></iron-icon>
</cr-button>
</div>
</ambient-preview>
</template>
</div>
</div>
Expand Up @@ -9,7 +9,7 @@

import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
import {html} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {Paths, PersonalizationRouter} from './personalization_router_element.js';
import {isAmbientModeAllowed, Paths, PersonalizationRouter} from './personalization_router_element.js';
import {WithPersonalizationStore} from './personalization_store.js';

export class PersonalizationMain extends WithPersonalizationStore {
Expand All @@ -34,6 +34,10 @@ export class PersonalizationMain extends WithPersonalizationStore {
return loadTimeData.getBoolean('isDarkLightModeEnabled');
}

private isAmbientModeAllowed_(): boolean {
return isAmbientModeAllowed();
}

private onClickUserSubpageLink_() {
PersonalizationRouter.instance().goToRoute(Paths.User);
}
Expand Down
Expand Up @@ -29,6 +29,10 @@ export function isPersonalizationHubEnabled(): boolean {
return loadTimeData.getBoolean('isPersonalizationHubEnabled');
}

export function isAmbientModeAllowed(): boolean {
return loadTimeData.getBoolean('isAmbientModeAllowed');
}

export class PersonalizationRouter extends PolymerElement {
static get is() {
return 'personalization-router';
Expand All @@ -42,6 +46,7 @@ export class PersonalizationRouter extends PolymerElement {
return {
path_: {
type: String,
observer: 'onPathChanged_',
},

query_: {
Expand Down Expand Up @@ -119,11 +124,19 @@ export class PersonalizationRouter extends PolymerElement {
}

private shouldShowRootPage_(path: string|null): boolean {
return isPersonalizationHubEnabled() && path === Paths.Root;
if (!isPersonalizationHubEnabled()) {
return false;
}

// If the ambient mode is not allowed, will not show Ambient/AmbientAlbums
// subpages.
return (path === Paths.Root) ||
(!!path && path.startsWith(Paths.Ambient) && !isAmbientModeAllowed());
}

private shouldShowAmbientSubpage_(path: string|null): boolean {
return isPersonalizationHubEnabled() && !!path?.startsWith(Paths.Ambient);
return isPersonalizationHubEnabled() && !!path?.startsWith(Paths.Ambient) &&
isAmbientModeAllowed();
}

private shouldShowUserSubpage_(path: string|null): boolean {
Expand All @@ -137,6 +150,18 @@ export class PersonalizationRouter extends PolymerElement {
private shouldShowBreadcrumb_(path: string|null): boolean {
return path !== Paths.Root;
}

/**
* When navigating to Ambient/AmbientAlbums subpages, but the ambient mode is
* not allowed, will not show Ambient/AmbientAlbums subpages. Reset path to
* root in this case.
*/
private onPathChanged_(path: string|null) {
if (!!path && path.startsWith(Paths.Ambient) && !isAmbientModeAllowed()) {
// Reset the path to root.
this.setProperties({path_: Paths.Root, queryParams_: {}});
}
}
}

customElements.define(PersonalizationRouter.is, PersonalizationRouter);
Expand Up @@ -9,6 +9,7 @@
GEN('#include "ash/webui/personalization_app/test/personalization_app_browsertest_fixture.h"');

GEN('#include "ash/constants/ash_features.h"');
GEN('#include "ash/public/cpp/ambient/ambient_client.h"');
GEN('#include "chromeos/constants/chromeos_features.h"');
GEN('#include "content/public/test/browser_test.h"');

Expand Down Expand Up @@ -91,29 +92,8 @@ TEST_F('PersonalizationAppBrowserTest', 'HasRootPageUrl', () => {
const wallpaperPreview = document.querySelector('personalization-router')
.shadowRoot.querySelector('personalization-main')
.shadowRoot.querySelector('wallpaper-preview');
const ambientPreview = document.querySelector('personalization-router')
.shadowRoot.querySelector('personalization-main')
.shadowRoot.querySelector('ambient-preview');
assertTrue(!!userPreview);
assertTrue(!!wallpaperPreview);
assertTrue(!!ambientPreview);
testDone();
});

TEST_F('PersonalizationAppBrowserTest', 'ShowsAmbientPreview', () => {
const preview = document.querySelector('personalization-router')
.shadowRoot.querySelector('personalization-main')
.shadowRoot.querySelector('ambient-preview');
assertTrue(!!preview);
testDone();
});

TEST_F('PersonalizationAppBrowserTest', 'ShowsAmbientSubpageLink', () => {
const ambientSubpageLink =
document.querySelector('personalization-router')
.shadowRoot.querySelector('personalization-main')
.shadowRoot.querySelector('#ambientSubpageLink');
assertTrue(!!ambientSubpageLink);
testDone();
});

Expand Down Expand Up @@ -146,6 +126,74 @@ TEST_F('PersonalizationAppBrowserTest', 'ShowsUserInfo', async () => {
testDone();
});

class PersonalizationAppAmbientModeAllowedBrowserTest extends
PersonalizationAppBrowserTest {
/** @override */
get testGenPreamble() {
return () => {
GEN('ash::AmbientClient::Get()->SetAmbientModeAllowedForTesting(true);');
};
}
}

this[PersonalizationAppAmbientModeAllowedBrowserTest.name] =
PersonalizationAppAmbientModeAllowedBrowserTest;

TEST_F(
'PersonalizationAppAmbientModeAllowedBrowserTest', 'ShowsAmbientPreview',
() => {
const preview = document.querySelector('personalization-router')
.shadowRoot.querySelector('personalization-main')
.shadowRoot.querySelector('ambient-preview');
assertTrue(!!preview);
testDone();
});

TEST_F(
'PersonalizationAppAmbientModeAllowedBrowserTest',
'ShowsAmbientSubpageLink', () => {
const ambientSubpageLink =
document.querySelector('personalization-router')
.shadowRoot.querySelector('personalization-main')
.shadowRoot.querySelector('#ambientSubpageLink');
assertTrue(!!ambientSubpageLink);
testDone();
});

class PersonalizationAppAmbientModeDisllowedBrowserTest extends
PersonalizationAppBrowserTest {
/** @override */
get testGenPreamble() {
return () => {
GEN('ash::AmbientClient::Get()->SetAmbientModeAllowedForTesting(false);');
};
}
}

this[PersonalizationAppAmbientModeDisllowedBrowserTest.name] =
PersonalizationAppAmbientModeDisllowedBrowserTest;

TEST_F(
'PersonalizationAppAmbientModeDisllowedBrowserTest',
'NotShowAmbientPreview', () => {
const preview = document.querySelector('personalization-router')
.shadowRoot.querySelector('personalization-main')
.shadowRoot.querySelector('ambient-preview');
assertFalse(!!preview);
testDone();
});

TEST_F(
'PersonalizationAppAmbientModeDisllowedBrowserTest',
'NotShowAmbientSubpageLink', () => {
const ambientSubpageLink =
document.querySelector('personalization-router')
.shadowRoot.querySelector('personalization-main')
.shadowRoot.querySelector('#ambientSubpageLink');
assertFalse(!!ambientSubpageLink);
testDone();
});

class WallpaperSubpageBrowserTest extends PersonalizationAppBrowserTest {
/** @override */
get browsePreload() {
Expand Down
8 changes: 8 additions & 0 deletions chrome/browser/ui/ash/ambient/ambient_client_impl.cc
Expand Up @@ -111,6 +111,10 @@ AmbientClientImpl::~AmbientClientImpl() = default;
bool AmbientClientImpl::IsAmbientModeAllowed() {
DCHECK(chromeos::features::IsAmbientModeEnabled());

if (is_allowed_for_testing_.has_value()) {
return is_allowed_for_testing_.value();
}

if (ash::DemoSession::IsDeviceInDemoMode())
return false;

Expand Down Expand Up @@ -141,6 +145,10 @@ bool AmbientClientImpl::IsAmbientModeAllowed() {
return true;
}

void AmbientClientImpl::SetAmbientModeAllowedForTesting(bool allowed) {
is_allowed_for_testing_ = allowed;
}

void AmbientClientImpl::RequestAccessToken(GetAccessTokenCallback callback) {
auto* profile = GetProfileForActiveUser();
DCHECK(profile);
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/ui/ash/ambient/ambient_client_impl.h
Expand Up @@ -11,6 +11,7 @@
#include "ash/public/cpp/ambient/ambient_client.h"
#include "ash/public/cpp/image_downloader.h"
#include "base/memory/weak_ptr.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

class GoogleServiceAuthError;

Expand All @@ -27,6 +28,7 @@ class AmbientClientImpl : public ash::AmbientClient {

// ash::AmbientClient:
bool IsAmbientModeAllowed() override;
void SetAmbientModeAllowedForTesting(bool allowed) override;
void RequestAccessToken(GetAccessTokenCallback callback) override;
void DownloadImage(const std::string& url,
ash::ImageDownloader::DownloadCallback callback) override;
Expand All @@ -50,6 +52,7 @@ class AmbientClientImpl : public ash::AmbientClient {

std::map<base::UnguessableToken, std::unique_ptr<signin::AccessTokenFetcher>>
token_fetchers_;
absl::optional<bool> is_allowed_for_testing_;
base::WeakPtrFactory<AmbientClientImpl> weak_factory_{this};
};

Expand Down
Expand Up @@ -4,7 +4,9 @@

import {PersonalizationMain} from 'chrome://personalization/trusted/personalization_main_element.js';
import {Paths, PersonalizationRouter} from 'chrome://personalization/trusted/personalization_router_element.js';
import {assertDeepEquals, assertEquals} from 'chrome://webui-test/chai_assert.js';
import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
import {assertDeepEquals, assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';
import {waitAfterNextRender} from 'chrome://webui-test/test_util.js';

import {initElement, teardownElement} from './personalization_app_test_utils.js';

Expand Down Expand Up @@ -41,6 +43,7 @@ export function PersonalizationMainTest() {
});

test('links to ambient subpage', async () => {
loadTimeData.overrideValues({'isAmbientModeAllowed': true});
personalizationMainElement = initElement(PersonalizationMain);
const original = PersonalizationRouter.instance;
const goToRoutePromise = new Promise<[Paths, Object]>(resolve => {
Expand All @@ -61,4 +64,35 @@ export function PersonalizationMainTest() {
assertEquals(Paths.Ambient, path);
assertDeepEquals({}, queryParams);
});

test('no links to ambient subpage', async () => {
loadTimeData.overrideValues({'isAmbientModeAllowed': false});
personalizationMainElement = initElement(PersonalizationMain);

const ambientSubpageLink =
personalizationMainElement!.shadowRoot!.getElementById(
'ambientSubpageLink')!;
assertFalse(!!ambientSubpageLink);
});

test('has ambient preview', async () => {
loadTimeData.overrideValues({'isAmbientModeAllowed': true});
personalizationMainElement = initElement(PersonalizationMain);
await waitAfterNextRender(personalizationMainElement);

const preview = personalizationMainElement!.shadowRoot!.querySelector(
'ambient-preview')!;
assertTrue(!!preview);
});

test('has no ambient preview', async () => {
loadTimeData.overrideValues({'isAmbientModeAllowed': false});
personalizationMainElement = initElement(PersonalizationMain);
await waitAfterNextRender(personalizationMainElement);


const preview = personalizationMainElement!.shadowRoot!.querySelector(
'ambient-preview')!;
assertFalse(!!preview);
});
}

0 comments on commit ed685a8

Please sign in to comment.