[OsSettingsDeepLinking] Add deep links to the Personalization Page

Adds deep links to the Personalization Page, including the change
picture and ambient mode subpages. Also fixes a bug where the ambient
mode page tests were not run.

Bug: 1084154
Change-Id: Ife0a6602e9fecdef57db116ce6dd7e74c9ce3cc7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2375730
Commit-Queue: Daniel Classon <dclasson@google.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801999}

chrome/browser/resources/settings/chromeos/ambient_mode_page/BUILD.gn

@@ -32,6 +32,7 @@ js_library("ambient_mode_page") {
   deps = [
     ":ambient_mode_browser_proxy",
     ":constants",
+    "..:deep_linking_behavior",
     "..:os_route",
     "../..:router",
     "../../prefs:prefs_behavior",
@@ -119,6 +120,7 @@ js_library("ambient_mode_page.m") {
   deps = [
     ":ambient_mode_browser_proxy.m",
     ":constants.m",
+    "..:deep_linking_behavior.m",
     "..:os_route.m",
     "../..:router.m",
     "../../prefs:prefs_behavior.m",

chrome/browser/resources/settings/chromeos/ambient_mode_page/ambient_mode_page.html

@@ -8,6 +8,7 @@
 <link rel="import" href="chrome://resources/html/web_ui_listener_behavior.html">
 <link rel="import" href="constants.html">
 <link rel="import" href="topic_source_list.html">
+<link rel="import" href="../deep_linking_behavior.html">
 <link rel="import" href="../os_route.html">
 <link rel="import" href="../../router.html">
 <link rel="import" href="../../prefs/prefs.html">
@@ -61,7 +62,8 @@
         class="primary-toggle"
         pref="{{prefs.settings.ambient_mode.enabled}}"
         label="[[getAmbientModeOnOffLabel_(
-            prefs.settings.ambient_mode.enabled.value)]]">
+            prefs.settings.ambient_mode.enabled.value)]]"
+        deep-link-focus-id$="[[Setting.kAmbientModeOnOff]]">
     </settings-toggle-button>
 
     <template is="dom-if" if="[[prefs.settings.ambient_mode.enabled.value]]">

chrome/browser/resources/settings/chromeos/ambient_mode_page/ambient_mode_page.js

@@ -10,8 +10,8 @@ Polymer({
   is: 'settings-ambient-mode-page',
 
   behaviors: [
-    I18nBehavior, PrefsBehavior, settings.RouteObserverBehavior,
-    WebUIListenerBehavior
+    DeepLinkingBehavior, I18nBehavior, PrefsBehavior,
+    settings.RouteObserverBehavior, WebUIListenerBehavior
   ],
 
   properties: {
@@ -57,6 +57,18 @@ Polymer({
       value: AmbientModeTemperatureUnit.UNKNOWN,
       observer: 'onSelectedTemperatureUnitChanged_'
     },
+
+    /**
+     * Used by DeepLinkingBehavior to focus this page's deep links.
+     * @type {!Set<!chromeos.settings.mojom.Setting>}
+     */
+    supportedSettingIds: {
+      type: Object,
+      value: () => new Set([
+        chromeos.settings.mojom.Setting.kAmbientModeOnOff,
+        chromeos.settings.mojom.Setting.kAmbientModeSource,
+      ]),
+    },
   },
 
   listeners: {
@@ -88,6 +100,33 @@ Polymer({
     );
   },
 
+  /**
+   * Overridden from DeepLinkingBehavior.
+   * @param {!chromeos.settings.mojom.Setting} settingId
+   */
+  beforeDeepLinkAttempt(settingId) {
+    if (settingId !== chromeos.settings.mojom.Setting.kAmbientModeSource) {
+      // Continue with deep link attempt.
+      return true;
+    }
+
+    // Wait for element to load.
+    Polymer.RenderStatus.afterNextRender(this, () => {
+      Polymer.dom.flush();
+
+      const topicList = this.$$('topic-source-list');
+      const listItem = topicList && topicList.$$('topic-source-item');
+      if (listItem) {
+        this.showDeepLinkElement(listItem);
+        return;
+      }
+
+      console.warn(`Element with deep link id ${settingId} not focusable.`);
+    });
+    // Stop deep link attempt since we completed it manually.
+    return false;
+  },
+
   /**
    * RouteObserverBehavior
    * @param {!settings.Route} currentRoute
@@ -99,6 +138,7 @@ Polymer({
     }
 
     this.browserProxy_.requestSettings();
+    this.attemptDeepLink();
   },
 
   /**

chrome/browser/resources/settings/chromeos/personalization_page/BUILD.gn

@@ -18,6 +18,7 @@ js_type_check("closure_compile") {
 js_library("change_picture") {
   deps = [
     ":change_picture_browser_proxy",
+    "..:deep_linking_behavior",
     "..:metrics_recorder",
     "..:os_route",
     "//third_party/polymer/v1_0/components-chromium/iron-a11y-announcer:iron-a11y-announcer-extracted",
@@ -40,6 +41,7 @@ js_library("change_picture_browser_proxy") {
 js_library("personalization_page") {
   deps = [
     ":wallpaper_browser_proxy",
+    "..:deep_linking_behavior",
     "..:os_route",
     "../..:router",
     "../../prefs",
@@ -68,6 +70,7 @@ js_library("change_picture.m") {
   sources = [ "$root_gen_dir/chrome/browser/resources/settings/chromeos/personalization_page/change_picture.m.js" ]
   deps = [
     ":change_picture_browser_proxy.m",
+    "..:deep_linking_behavior.m",
     "..:metrics_recorder.m",
     "..:os_route.m",
     "../..:router.m",
@@ -96,6 +99,7 @@ js_library("personalization_page.m") {
   sources = [ "$root_gen_dir/chrome/browser/resources/settings/chromeos/personalization_page/personalization_page.m.js" ]
   deps = [
     ":wallpaper_browser_proxy.m",
+    "..:deep_linking_behavior.m",
     "..:os_route.m",
     "../..:router.m",
     "../../prefs:prefs.m",
@@ -129,7 +133,7 @@ polymer_modulizer("change_picture") {
   namespace_rewrites = os_settings_namespace_rewrites +
                        [ "Polymer.IronA11yAnnouncer|IronA11yAnnouncer" ]
   auto_imports = os_settings_auto_imports + [
-                   "ui/webui/resources/html/assert.html|assertNotReached",
+                   "ui/webui/resources/html/assert.html|assert,assertNotReached",
                    "ui/webui/resources/polymer/v3_0/iron-a11y-announcer/iron-a11y-announcer.html|IronA11yAnnouncer",
                  ]
 }

chrome/browser/resources/settings/chromeos/personalization_page/change_picture.html

@@ -9,6 +9,7 @@
 <link rel="import" href="chrome://resources/html/web_ui_listener_behavior.html">
 <link rel="import" href="chrome://resources/html/assert.html">
 <link rel="import" href="../../i18n_setup.html">
+<link rel="import" href="../deep_linking_behavior.html">
 <link rel="import" href="../os_route.html">
 <link rel="import" href="../../router.html">
 <link rel="import" href="../../settings_shared_css.html">

chrome/browser/resources/settings/chromeos/personalization_page/change_picture.js

@@ -11,6 +11,7 @@ Polymer({
   is: 'settings-change-picture',
 
   behaviors: [
+    DeepLinkingBehavior,
     settings.RouteObserverBehavior,
     I18nBehavior,
     WebUIListenerBehavior,
@@ -66,6 +67,16 @@ Polymer({
 
     /** @private */
     oldImageLabel_: String,
+
+    /**
+     * Used by DeepLinkingBehavior to focus this page's deep links.
+     * @type {!Set<!chromeos.settings.mojom.Setting>}
+     */
+    supportedSettingIds: {
+      type: Object,
+      value: () =>
+          new Set([chromeos.settings.mojom.Setting.kChangeDeviceAccountImage]),
+    },
   },
 
   listeners: {
@@ -109,6 +120,20 @@ Polymer({
     Polymer.IronA11yAnnouncer.requestAvailability();
   },
 
+  /**
+   * Overridden from DeepLinkingBehavior.
+   * @param {!chromeos.settings.mojom.Setting} settingId
+   * @return {boolean}
+   */
+  beforeDeepLinkAttempt(settingId) {
+    assert(
+        settingId ===
+        chromeos.settings.mojom.Setting.kChangeDeviceAccountImage);
+
+    this.pictureList_.setFocus();
+    return false;
+  },
+
 
   /** @protected */
   currentRouteChanged(newRoute) {
@@ -116,6 +141,7 @@ Polymer({
       this.browserProxy_.initialize();
       this.browserProxy_.requestSelectedImage();
       this.pictureList_.setFocus();
+      this.attemptDeepLink();
     } else {
       // Ensure we deactivate the camera when we navigate away.
       this.selectedItem_ = null;

chrome/browser/resources/settings/chromeos/personalization_page/personalization_page.html

@@ -5,6 +5,7 @@
 <link rel="import" href="chrome://resources/cr_elements/cr_link_row/cr_link_row.html">
 <link rel="import" href="chrome://resources/html/i18n_behavior.html">
 <link rel="import" href="change_picture.html">
+<link rel="import" href="../deep_linking_behavior.html">
 <link rel="import" href="../os_route.html">
 <link rel="import" href="../../router.html">
 <link rel="import" href="../../i18n_setup.html">
@@ -28,7 +29,8 @@
             hidden="[[!showWallpaperRow_]]"
             on-click="openWallpaperManager_" label="$i18n{setWallpaper}"
             sub-label="$i18n{openWallpaperApp}"
-            disabled="[[isWallpaperPolicyControlled_]]" external>
+            disabled="[[isWallpaperPolicyControlled_]]" external
+            deep-link-focus-id$="[[Setting.kOpenWallpaper]]">
           <template is="dom-if" if="[[isWallpaperPolicyControlled_]]">
             <cr-policy-indicator id="wallpaperPolicyIndicator"
                 indicator-type="devicePolicy">

chrome/browser/resources/settings/chromeos/personalization_page/personalization_page.js

@@ -9,7 +9,11 @@
 Polymer({
   is: 'settings-personalization-page',
 
-  behaviors: [I18nBehavior],
+  behaviors: [
+    DeepLinkingBehavior,
+    I18nBehavior,
+    settings.RouteObserverBehavior,
+  ],
 
   properties: {
     /**
@@ -46,6 +50,15 @@ Polymer({
         return map;
       }
     },
+
+    /**
+     * Used by DeepLinkingBehavior to focus this page's deep links.
+     * @type {!Set<!chromeos.settings.mojom.Setting>}
+     */
+    supportedSettingIds: {
+      type: Object,
+      value: () => new Set([chromeos.settings.mojom.Setting.kOpenWallpaper]),
+    },
   },
 
   /** @private {?settings.WallpaperBrowserProxy} */
@@ -68,6 +81,19 @@ Polymer({
         });
   },
 
+  /**
+   * @param {!settings.Route} route
+   * @param {!settings.Route} oldRoute
+   */
+  currentRouteChanged(route, oldRoute) {
+    // Does not apply to this page.
+    if (route !== settings.routes.PERSONALIZATION) {
+      return;
+    }
+
+    this.attemptDeepLink();
+  },
+
   /**
    * @private
    */

chrome/test/data/webui/settings/chromeos/ambient_mode_page_test.js

@@ -5,10 +5,12 @@
 // clang-format off
 // #import 'chrome://os-settings/chromeos/os_settings.js';
 
-// #import {AmbientModeTopicSource, AmbientModeTemperatureUnit, AmbientModeBrowserProxyImpl, CrSettingsPrefs, Router} from 'chrome://os-settings/chromeos/os_settings.js';
+// #import {AmbientModeTopicSource, AmbientModeTemperatureUnit, AmbientModeBrowserProxyImpl, CrSettingsPrefs, routes, Router} from 'chrome://os-settings/chromeos/os_settings.js';
 // #import {TestBrowserProxy} from '../../test_browser_proxy.m.js';
 // #import {assertEquals, assertFalse, assertTrue} from '../../chai_assert.js';
 // #import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
+// #import {getDeepActiveElement} from 'chrome://resources/js/util.m.js';
+// #import {waitAfterNextRender} from 'chrome://test/test_util.m.js';
 // clang-format on
 
 /**
@@ -92,6 +94,7 @@ suite('AmbientModeHandler', function() {
   teardown(function() {
     ambientModePage.remove();
     ambientModePhotosPage.remove();
+    settings.Router.getInstance().resetRouteForTesting();
   });
 
   test('toggleAmbientMode', function() {
@@ -156,6 +159,23 @@ suite('AmbientModeHandler', function() {
     assertEquals(2, topicSourceItems.length);
   });
 
+  test('Deep link to topic sources', async () => {
+    loadTimeData.overrideValues({isDeepLinkingEnabled: true});
+    assertTrue(loadTimeData.getBoolean('isDeepLinkingEnabled'));
+
+    const params = new URLSearchParams;
+    params.append('settingId', '502');
+    settings.Router.getInstance().navigateTo(
+        settings.routes.AMBIENT_MODE, params);
+
+    const deepLinkElement =
+        ambientModePage.$$('topic-source-list').$$('topic-source-item');
+    await test_util.waitAfterNextRender(deepLinkElement);
+    assertEquals(
+        deepLinkElement, getDeepActiveElement(),
+        'Topic sources row should be focused for settingId=502.');
+  });
+
   test('hasAlbums', function() {
     ambientModePhotosPage.albums_ = [
       {albumId: 'id0', checked: true, title: 'album0'},

chrome/test/data/webui/settings/chromeos/os_settings_browsertest.js

@@ -235,11 +235,16 @@ var OSSettingsAmbientModePageTest = class extends OSSettingsBrowserTest {
   get extraLibraries() {
     return super.extraLibraries.concat([
       BROWSER_SETTINGS_PATH + '../test_browser_proxy.js',
+      BROWSER_SETTINGS_PATH + '../test_util.js',
       'ambient_mode_page_test.js',
     ]);
   }
 };
 
+TEST_F('OSSettingsAmbientModePageTest', 'AllJsTests', () => {
+  mocha.run();
+});
+
 // Tests for ambient mode photos page.
 // eslint-disable-next-line no-var
 var OSSettingsAmbientModePhotosPageTest = class extends OSSettingsBrowserTest {
@@ -1376,6 +1381,7 @@ var OSSettingsPersonalizationPageTest = class extends OSSettingsBrowserTest {
     return super.extraLibraries.concat([
       '//ui/webui/resources/js/promise_resolver.js',
       BROWSER_SETTINGS_PATH + '../test_browser_proxy.js',
+      BROWSER_SETTINGS_PATH + '../test_util.js',
       BROWSER_SETTINGS_PATH + 'chromeos/test_wallpaper_browser_proxy.js',
       'personalization_page_test.js',
     ]);

chrome/test/data/webui/settings/chromeos/personalization_page_test.js

@@ -9,6 +9,8 @@
 // #import {assertEquals, assertFalse, assertTrue} from '../../chai_assert.js';
 // #import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 // #import {TestWallpaperBrowserProxy} from './test_wallpaper_browser_proxy.m.js';
+// #import {getDeepActiveElement} from 'chrome://resources/js/util.m.js';
+// #import {waitAfterNextRender} from 'chrome://test/test_util.m.js';
 // clang-format on
 
 let personalizationPage = null;
@@ -55,6 +57,7 @@ suite('PersonalizationHandler', function() {
 
   teardown(function() {
     personalizationPage.remove();
+    settings.Router.getInstance().resetRouteForTesting();
   });
 
   test('wallpaperManager', async () => {
@@ -87,6 +90,22 @@ suite('PersonalizationHandler', function() {
     assertTrue(personalizationPage.$$('#wallpaperButton').disabled);
   });
 
+  test('Deep link to open wallpaper button', async () => {
+    loadTimeData.overrideValues({isDeepLinkingEnabled: true});
+    assertTrue(loadTimeData.getBoolean('isDeepLinkingEnabled'));
+
+    const params = new URLSearchParams;
+    params.append('settingId', '500');
+    settings.Router.getInstance().navigateTo(
+        settings.routes.PERSONALIZATION, params);
+
+    const deepLinkElement = personalizationPage.$.wallpaperButton.$$('#icon');
+    await test_util.waitAfterNextRender(deepLinkElement);
+    assertEquals(
+        deepLinkElement, getDeepActiveElement(),
+        'Wallpaper button should be focused for settingId=500.');
+  });
+
   test('changePicture', function() {
     const row = personalizationPage.$.changePictureRow;
     assertTrue(!!row);
@@ -96,6 +115,28 @@ suite('PersonalizationHandler', function() {
         settings.Router.getInstance().getCurrentRoute());
   });
 
+  test('Deep link to change account picture', async () => {
+    loadTimeData.overrideValues({isDeepLinkingEnabled: true});
+    assertTrue(loadTimeData.getBoolean('isDeepLinkingEnabled'));
+
+    const params = new URLSearchParams;
+    params.append('settingId', '503');
+    settings.Router.getInstance().navigateTo(
+        settings.routes.CHANGE_PICTURE, params);
+
+    await test_util.waitAfterNextRender(personalizationPage);
+
+    const changePicturePage = personalizationPage.$$('settings-change-picture');
+    assertTrue(!!changePicturePage);
+    const deepLinkElement = changePicturePage.$$('#pictureList')
+                                .$$('#selector')
+                                .$$('[class="iron-selected"]');
+    await test_util.waitAfterNextRender(deepLinkElement);
+    assertEquals(
+        deepLinkElement, getDeepActiveElement(),
+        'Account picture elem should be focused for settingId=503.');
+  });
+
   test('ambientMode', function() {
     const isGuest = loadTimeData.getBoolean('isGuest');
     const isAmbientModeEnabled = loadTimeData.getBoolean('isAmbientModeEnabled');