[MD settings] startup url list to a subpage

This CL moves the on startup settings off of the main settings page and
into a subpage.

Bug: 653376
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I46dd91fbd3e1bc4b28b6465ad0a3a08b7bee1927
Reviewed-on: https://chromium-review.googlesource.com/557971Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Commit-Queue: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485130}

chrome/app/settings_strings.grdp

@@ -1636,6 +1636,15 @@
   <message name="IDS_SETTINGS_ON_STARTUP" desc="Name of the on startup page.">
     On startup
   </message>
+  <message name="IDS_SETTINGS_ON_STARTUP_DESCRIPTION" desc="Secondary line for IDS_SETTINGS_ON_STARTUP_MANAGE describing the OnStartup features.">
+    Restore tabs or open specific pages
+  </message>
+  <message name="IDS_SETTINGS_ON_STARTUP_MANAGE" desc="Button label to open subpage for OnStartup URLs and heading for that subpage.">
+    Manage on startup pages
+  </message>
+  <message name="IDS_SETTINGS_ON_STARTUP_PAGES" desc="Label for list of OnStartup URLs.">
+    Pages to open on startup
+  </message>
   <message name="IDS_SETTINGS_ON_STARTUP_OPEN_NEW_TAB" desc="Radio button option to open the new tab page.">
     Open the New Tab page
   </message>

chrome/browser/resources/settings/on_startup_page/compiled_resources2.gyp

@@ -14,7 +14,7 @@
     {
       'target_name': 'on_startup_page',
       'dependencies': [
-        'on_startup_browser_proxy',
+        '../compiled_resources2.gyp:route',
       ],
       'includes': ['../../../../../third_party/closure_compiler/compile_js2.gypi'],
     },
@@ -27,6 +27,7 @@
         '<(DEPTH)/ui/webui/resources/js/cr/ui/compiled_resources2.gyp:focus_without_ink',
         '<(DEPTH)/ui/webui/resources/cr_elements/compiled_resources2.gyp:cr_scrollable_behavior',
         '<(EXTERNS_GYP):settings_private',
+        'on_startup_browser_proxy',
         'startup_urls_page_browser_proxy',
         'startup_url_entry',
       ],

chrome/browser/resources/settings/on_startup_page/on_startup_page.html

 <link rel="import" href="chrome://resources/html/polymer.html">
 
-<link rel="import" href="../controls/controlled_radio_button.html">
-<link rel="import" href="../controls/extension_controlled_indicator.html">
-<link rel="import" href="../controls/settings_radio_group.html">
-<link rel="import" href="on_startup_browser_proxy.html">
-<link rel="import" href="startup_urls_page.html">
 <link rel="import" href="../settings_shared_css.html">
+<link rel="import" href="startup_urls_page.html">
 
 <dom-module id="settings-on-startup-page">
   <template>
-    <style include="settings-shared">
-      .block {
-        display: block;
-      }
-    </style>
-    <div class="settings-box block first">
-      <settings-radio-group id="onStartupRadioGroup"
-          pref="{{prefs.session.restore_on_startup}}">
-        <controlled-radio-button name="[[prefValues_.OPEN_NEW_TAB]]"
-            pref="[[prefs.session.restore_on_startup]]"
-            label="$i18n{onStartupOpenNewTab}"
-            no-extension-indicator>
-        </controlled-radio-button>
-        <template is="dom-if" if="[[showIndicator_(
-            ntpExtension_, prefs.session.restore_on_startup.value)]]">
-          <extension-controlled-indicator
-              extension-id="[[ntpExtension_.id]]"
-              extension-name="[[ntpExtension_.name]]"
-              extension-can-be-disabled="[[ntpExtension_.canBeDisabled]]"
-              on-extension-disable="getNtpExtension_">
-          </extension-controlled-indicator>
-        </template>
-        <controlled-radio-button name="[[prefValues_.CONTINUE]]"
-            pref="[[prefs.session.restore_on_startup]]"
-            label="$i18n{onStartupContinue}">
-        </controlled-radio-button>
-        <controlled-radio-button name="[[prefValues_.OPEN_SPECIFIC]]"
-            pref="[[prefs.session.restore_on_startup]]"
-            label="$i18n{onStartupOpenSpecific}">
-        </controlled-radio-button>
-      </settings-radio-group>
-    </div>
-    <template is="dom-if"
-        if="[[showStartupUrls_(prefs.session.restore_on_startup.value)]]">
-      <settings-startup-urls-page prefs="[[prefs]]">
-      </settings-startup-urls-page>
-    </template>
+    <style include="settings-shared"></style>
+    <settings-animated-pages id="pages" section="onStartup"
+        focus-config="[[focusConfig_]]">
+      <neon-animatable route-path="default">
+        <div class="settings-box first two-line"
+            id="manage-startup-urls-subpage-trigger"
+            on-tap="onManageStartupUrls_" actionable>
+          <div class="start">
+            $i18n{onStartupManage}
+            <div class="secondary">$i18n{onStartupDescription}</div>
+          </div>
+          <button class="subpage-arrow" is="paper-icon-button-light"
+              aria-label="$i18n{onStartupManage}"></button>
+        </div>
+      </neon-animatable>
+      <template is="dom-if" route-path="/startupUrls">
+        <settings-subpage
+            associated-control="[[$$('#manage-startup-urls-subpage-trigger')]]"
+            page-title="$i18n{onStartupManage}">
+          <settings-startup-urls-page prefs="{{prefs}}">
+          </settings-startup-urls-page>
+        </settings-subpage>
+      </template>
+    </settings-animated-pages>
   </template>
   <script src="on_startup_page.js"></script>
 </dom-module>

chrome/browser/resources/settings/on_startup_page/on_startup_page.js

@@ -5,72 +5,16 @@
 /**
  * @fileoverview
  * 'settings-on-startup-page' is a settings page.
- *
- * Example:
- *
- *    <neon-animated-pages>
- *      <settings-on-startup-page prefs="{{prefs}}">
- *      </settings-on-startup-page>
- *      ... other pages ...
- *    </neon-animated-pages>
  */
 Polymer({
   is: 'settings-on-startup-page',
 
   properties: {
-    prefs: {
-      type: Object,
-      notify: true,
-    },
-
-    /** @private {?NtpExtension} */
-    ntpExtension_: Object,
-
-    /**
-     * Enum values for the 'session.restore_on_startup' preference.
-     * @private {!Object<string, number>}
-     */
-    prefValues_: {
-      readOnly: true,
-      type: Object,
-      value: {
-        CONTINUE: 1,
-        OPEN_NEW_TAB: 5,
-        OPEN_SPECIFIC: 4,
-      },
-    },
-  },
-
-  /** @override */
-  attached: function() {
-    this.getNtpExtension_();
+    prefs: Object,
   },
 
   /** @private */
-  getNtpExtension_: function() {
-    settings.OnStartupBrowserProxyImpl.getInstance().getNtpExtension().then(
-        function(ntpExtension) {
-          this.ntpExtension_ = ntpExtension;
-        }.bind(this));
-  },
-
-  /**
-   * @param {?NtpExtension} ntpExtension
-   * @param {number} restoreOnStartup Value of prefs.session.restore_on_startup.
-   * @return {boolean}
-   * @private
-   */
-  showIndicator_: function(ntpExtension, restoreOnStartup) {
-    return !!ntpExtension && restoreOnStartup == this.prefValues_.OPEN_NEW_TAB;
-  },
-
-  /**
-   * Determine whether to show the user defined startup pages.
-   * @param {number} restoreOnStartup Enum value from prefValues_.
-   * @return {boolean} Whether the open specific pages is selected.
-   * @private
-   */
-  showStartupUrls_: function(restoreOnStartup) {
-    return restoreOnStartup == this.prefValues_.OPEN_SPECIFIC;
+  onManageStartupUrls_: function() {
+    settings.navigateTo(settings.Route.STARTUP_URLS);
   },
 });

chrome/browser/resources/settings/on_startup_page/startup_urls_page.html

@@ -8,8 +8,11 @@
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-list/iron-list.html">
 <link rel="import" href="chrome://resources/html/web_ui_listener_behavior.html">
 <link rel="import" href="chrome://resources/cr_elements/cr_scrollable_behavior.html">
+<link rel="import" href="../controls/controlled_radio_button.html">
 <link rel="import" href="../controls/extension_controlled_indicator.html">
+<link rel="import" href="../controls/settings_radio_group.html">
 <link rel="import" href="../settings_shared_css.html">
+<link rel="import" href="on_startup_browser_proxy.html">
 <link rel="import" href="startup_url_dialog.html">
 <link rel="import" href="startup_url_entry.html">
 <link rel="import" href="startup_urls_page_browser_proxy.html">
@@ -30,44 +33,79 @@
         cursor: default;
       }
     </style>
-    <div id="outer" class="layout vertical flex vertical-list">
-      <div id="container" class="scroll-container" scrollable>
-        <iron-list items="[[startupPages_]]" scroll-target="container"
-            preserve-focus>
-          <template>
-            <settings-startup-url-entry model="[[item]]"
-                tabindex$="[[tabIndex]]" iron-list-tab-index="[[tabIndex]]"
-                last-focused="{{lastFocused_}}"
-                editable="[[shouldAllowUrlsEdit_(
-                    prefs.session.startup_urls.enforcement)]]">
-            </settings-startup-url-entry>
+    <div class="settings-box first">
+      <h2>$i18n{onStartupPages}</h2>
+    </div>
+    <div class="settings-box first">
+      <div class="start">
+        <settings-radio-group id="onStartupRadioGroup"
+            pref="{{prefs.session.restore_on_startup}}">
+          <controlled-radio-button name="[[prefValues_.OPEN_NEW_TAB]]"
+              pref="[[prefs.session.restore_on_startup]]"
+              label="$i18n{onStartupOpenNewTab}"
+              no-extension-indicator>
+          </controlled-radio-button>
+          <template is="dom-if" if="[[showIndicator_(
+              ntpExtension_, prefs.session.restore_on_startup.value)]]">
+            <extension-controlled-indicator
+                extension-id="[[ntpExtension_.id]]"
+                extension-name="[[ntpExtension_.name]]"
+                extension-can-be-disabled="[[ntpExtension_.canBeDisabled]]"
+                on-extension-disable="getNtpExtension_">
+            </extension-controlled-indicator>
           </template>
-        </iron-list>
+          <controlled-radio-button name="[[prefValues_.CONTINUE]]"
+              pref="[[prefs.session.restore_on_startup]]"
+              label="$i18n{onStartupContinue}">
+          </controlled-radio-button>
+          <controlled-radio-button name="[[prefValues_.OPEN_SPECIFIC]]"
+              pref="[[prefs.session.restore_on_startup]]"
+              label="$i18n{onStartupOpenSpecific}">
+          </controlled-radio-button>
+        </settings-radio-group>
       </div>
-      <template is="dom-if" if="[[shouldAllowUrlsEdit_(
-          prefs.session.startup_urls.enforcement)]]" restamp>
-        <div class="list-item" id="addPage">
-          <a is="action-link" class="list-button" on-tap="onAddPageTap_">
-            $i18n{onStartupAddNewPage}
-          </a>
-        </div>
-        <div class="list-item" id="useCurrentPages">
-          <a is="action-link" class="list-button"
-              on-tap="onUseCurrentPagesTap_">
-            $i18n{onStartupUseCurrent}
-          </a>
-        </div>
-      </template>
-      <template is="dom-if" if="[[prefs.session.startup_urls.extensionId]]"
-          restamp>
-        <extension-controlled-indicator
-            extension-id="[[prefs.session.startup_urls.extensionId]]"
-            extension-name="[[prefs.session.startup_urls.controlledByName]]"
-            extension-can-be-disabled="[[
-                prefs.session.startup_urls.extensionCanBeDisabled]]">
-        </extension-controlled-indicator>
-      </template>
     </div>
+    <template is="dom-if"
+        if="[[showStartupUrls_(prefs.session.restore_on_startup.value)]]">
+      <div id="outer" class="layout vertical flex vertical-list">
+        <div id="container" class="scroll-container" scrollable>
+          <iron-list items="[[startupPages_]]" scroll-target="container"
+              preserve-focus>
+            <template>
+              <settings-startup-url-entry model="[[item]]"
+                  tabindex$="[[tabIndex]]" iron-list-tab-index="[[tabIndex]]"
+                  last-focused="{{lastFocused_}}"
+                  editable="[[shouldAllowUrlsEdit_(
+                      prefs.session.startup_urls.enforcement)]]">
+              </settings-startup-url-entry>
+            </template>
+          </iron-list>
+        </div>
+        <template is="dom-if" if="[[shouldAllowUrlsEdit_(
+            prefs.session.startup_urls.enforcement)]]" restamp>
+          <div class="list-item" id="addPage">
+            <a is="action-link" class="list-button" on-tap="onAddPageTap_">
+              $i18n{onStartupAddNewPage}
+            </a>
+          </div>
+          <div class="list-item" id="useCurrentPages">
+            <a is="action-link" class="list-button"
+                on-tap="onUseCurrentPagesTap_">
+              $i18n{onStartupUseCurrent}
+            </a>
+          </div>
+        </template>
+        <template is="dom-if" if="[[prefs.session.startup_urls.extensionId]]"
+            restamp>
+          <extension-controlled-indicator
+              extension-id="[[prefs.session.startup_urls.extensionId]]"
+              extension-name="[[prefs.session.startup_urls.controlledByName]]"
+              extension-can-be-disabled="[[
+                  prefs.session.startup_urls.extensionCanBeDisabled]]">
+          </extension-controlled-indicator>
+        </template>
+      </div>
+    </template>
     <template is="dom-if" if="[[showStartupUrlDialog_]]" restamp>
       <settings-startup-url-dialog model="[[startupUrlDialogModel_]]"
           on-close="destroyUrlDialog_">

chrome/browser/resources/settings/on_startup_page/startup_urls_page.js

@@ -13,7 +13,10 @@ Polymer({
   behaviors: [CrScrollableBehavior, WebUIListenerBehavior],
 
   properties: {
-    prefs: Object,
+    prefs: {
+      type: Object,
+      notify: true,
+    },
 
     /**
      * Pages to load upon browser startup.
@@ -29,6 +32,23 @@ Polymer({
 
     /** @private {Object}*/
     lastFocused_: Object,
+
+    /** @private {?NtpExtension} */
+    ntpExtension_: Object,
+
+    /**
+     * Enum values for the 'session.restore_on_startup' preference.
+     * @private {!Object<string, number>}
+     */
+    prefValues_: {
+      readOnly: true,
+      type: Object,
+      value: {
+        CONTINUE: 1,
+        OPEN_NEW_TAB: 5,
+        OPEN_SPECIFIC: 4,
+      },
+    },
   },
 
   /** @private {?settings.StartupUrlsPageBrowserProxy} */
@@ -42,6 +62,11 @@ Polymer({
 
   /** @override */
   attached: function() {
+    settings.OnStartupBrowserProxyImpl.getInstance().getNtpExtension().then(
+        function(ntpExtension) {
+          this.ntpExtension_ = ntpExtension;
+        }.bind(this));
+
     this.browserProxy_ = settings.StartupUrlsPageBrowserProxyImpl.getInstance();
     this.addWebUIListener('update-startup-pages', function(startupPages) {
       // If an "edit" URL dialog was open, close it, because the underlying page
@@ -96,4 +121,24 @@ Polymer({
     return this.get('prefs.session.startup_urls.enforcement') !=
         chrome.settingsPrivate.Enforcement.ENFORCED;
   },
+
+  /**
+   * @param {?NtpExtension} ntpExtension
+   * @param {number} restoreOnStartup Value of prefs.session.restore_on_startup.
+   * @return {boolean}
+   * @private
+   */
+  showIndicator_: function(ntpExtension, restoreOnStartup) {
+    return !!ntpExtension && restoreOnStartup == this.prefValues_.OPEN_NEW_TAB;
+  },
+
+  /**
+   * Determine whether to show the user defined startup pages.
+   * @param {number} restoreOnStartup Enum value from prefValues_.
+   * @return {boolean} Whether the open specific pages is selected.
+   * @private
+   */
+  showStartupUrls_: function(restoreOnStartup) {
+    return restoreOnStartup == this.prefValues_.OPEN_SPECIFIC;
+  },
 });

chrome/browser/resources/settings/route.js

@@ -138,6 +138,7 @@ cr.define('settings', function() {
   // </if>
 
   r.ON_STARTUP = r.BASIC.createSection('/onStartup', 'onStartup');
+  r.STARTUP_URLS = r.ON_STARTUP.createChild('/startupUrls');
 
   r.PEOPLE = r.BASIC.createSection('/people', 'people');
   r.SYNC = r.PEOPLE.createChild('/syncSetup');

chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

@@ -1186,6 +1186,9 @@ void AddChromeOSUserStrings(content::WebUIDataSource* html_source,
 void AddOnStartupStrings(content::WebUIDataSource* html_source) {
   LocalizedString localized_strings[] = {
       {"onStartup", IDS_SETTINGS_ON_STARTUP},
+      {"onStartupDescription", IDS_SETTINGS_ON_STARTUP_DESCRIPTION},
+      {"onStartupManage", IDS_SETTINGS_ON_STARTUP_MANAGE},
+      {"onStartupPages", IDS_SETTINGS_ON_STARTUP_PAGES},
       {"onStartupOpenNewTab", IDS_SETTINGS_ON_STARTUP_OPEN_NEW_TAB},
       {"onStartupContinue", IDS_SETTINGS_ON_STARTUP_CONTINUE},
       {"onStartupOpenSpecific", IDS_SETTINGS_ON_STARTUP_OPEN_SPECIFIC},

chrome/test/data/webui/settings/on_startup_browsertest.js

@@ -6,16 +6,6 @@
 
 GEN_INCLUDE(['settings_page_browsertest.js']);
 
-/**
-  * Radio button enum values for restore on startup.
-  * @enum
-  */
-var RestoreOnStartupEnum = {
-  CONTINUE: 1,
-  OPEN_NEW_TAB: 5,
-  OPEN_SPECIFIC: 4,
-};
-
 /**
  * Test Polymer On Startup Settings elements.
  * @constructor
@@ -39,45 +29,19 @@ OnStartupSettingsBrowserTest.prototype = {
 };
 
 TEST_F('OnStartupSettingsBrowserTest', 'uiTests', function() {
-  /**
-   * The prefs API that will get a fake implementation.
-   * @type {!SettingsPrivate}
-   */
-  var settingsPrefs;
   var self = this;
 
-  var restoreOnStartup = function() {
-    return self.getPageElement('#onStartupRadioGroup').querySelector(
-        '.iron-selected').label;
-  };
-
   suite('OnStartupHandler', function() {
     suiteSetup(function() {
       self.basicPage.set('pageVisibility.onStartup', true);
-      Polymer.dom.flush();
-
-      settingsPrefs = document.querySelector('settings-ui').$$(
-          'settings-prefs');
-      assertTrue(!!settingsPrefs);
-      return CrSettingsPrefs.initialized;
-    });
-
-    test('open-continue', function() {
-      settingsPrefs.set('prefs.session.restore_on_startup.value',
-          RestoreOnStartupEnum.CONTINUE);
-      assertEquals('Continue where you left off', restoreOnStartup());
-    });
-
-    test('open-ntp', function() {
-      settingsPrefs.set('prefs.session.restore_on_startup.value',
-          RestoreOnStartupEnum.OPEN_NEW_TAB);
-      assertEquals('Open the New Tab page', restoreOnStartup());
     });
 
-    test('open-specific', function() {
-      settingsPrefs.set('prefs.session.restore_on_startup.value',
-          RestoreOnStartupEnum.OPEN_SPECIFIC);
-      assertEquals('Open a specific page or set of pages', restoreOnStartup());
+    test('ManageStartupUrls', function() {
+      /* Test that the manage startup urls button is present on the basic page.
+       */
+      var manageButton =
+          self.getPageElement('#manage-startup-urls-subpage-trigger');
+      assertTrue(!!manageButton);
     });
   });
   mocha.run();

chrome/test/data/webui/settings/startup_urls_page_test.js

@@ -148,7 +148,7 @@ cr.define('settings_startup_urls_page', function() {
     });
 
     /**
-     * Tests that the appropritae browser proxy method is called when the action
+     * Tests that the appropriate browser proxy method is called when the action
      * button is tapped.
      * @param {string} proxyMethodName
      */
@@ -202,6 +202,16 @@ cr.define('settings_startup_urls_page', function() {
   });
 
   suite('StartupUrlsPage', function() {
+    /**
+     * Radio button enum values for restore on startup.
+     * @enum
+     */
+    var RestoreOnStartupEnum = {
+      CONTINUE: 1,
+      OPEN_NEW_TAB: 5,
+      OPEN_SPECIFIC: 4,
+    };
+
     /** @type {?SettingsStartupUrlsPageElement} */
     var page = null;
 
@@ -231,7 +241,39 @@ cr.define('settings_startup_urls_page', function() {
       return browserProxy.whenCalled('loadStartupPages');
     });
 
+    function restoreOnStartupLabel() {
+      return page.$$('#onStartupRadioGroup')
+          .querySelector('.iron-selected')
+          .label;
+    }
+
+    test('open-continue', function() {
+      page.set(
+          'prefs.session.restore_on_startup.value',
+          RestoreOnStartupEnum.CONTINUE);
+      assertEquals('Continue where you left off', restoreOnStartupLabel());
+    });
+
+    test('open-ntp', function() {
+      page.set(
+          'prefs.session.restore_on_startup.value',
+          RestoreOnStartupEnum.OPEN_NEW_TAB);
+      assertEquals('Open the New Tab page', restoreOnStartupLabel());
+    });
+
+    test('open-specific', function() {
+      page.set(
+          'prefs.session.restore_on_startup.value',
+          RestoreOnStartupEnum.OPEN_SPECIFIC);
+      assertEquals(
+          'Open a specific page or set of pages', restoreOnStartupLabel());
+    });
+
     test('UseCurrentPages', function() {
+      page.set(
+          'prefs.session.restore_on_startup.value',
+          RestoreOnStartupEnum.OPEN_SPECIFIC);
+      Polymer.dom.flush();
       var useCurrentPagesButton = page.$$('#useCurrentPages > a');
       assertTrue(!!useCurrentPagesButton);
       MockInteractions.tap(useCurrentPagesButton);
@@ -239,6 +281,10 @@ cr.define('settings_startup_urls_page', function() {
     });
 
     test('AddPage_OpensDialog', function() {
+      page.set(
+          'prefs.session.restore_on_startup.value',
+          RestoreOnStartupEnum.OPEN_SPECIFIC);
+      Polymer.dom.flush();
       var addPageButton = page.$$('#addPage > a');
       assertTrue(!!addPageButton);
       assertFalse(!!page.$$('settings-startup-url-dialog'));
@@ -284,7 +330,11 @@ cr.define('settings_startup_urls_page', function() {
       assertFalse(!!page.$$('settings-startup-url-dialog'));
     });
 
-    test('StarupPages_WhenExtensionControlled', function() {
+    test('StartupPages_WhenExtensionControlled', function() {
+      page.set(
+          'prefs.session.restore_on_startup.value',
+          RestoreOnStartupEnum.OPEN_SPECIFIC);
+      Polymer.dom.flush();
       assertFalse(!!page.get('prefs.session.startup_urls.controlledBy'));
       assertFalse(!!page.$$('extension-controlled-indicator'));
       assertTrue(!!page.$$('#addPage'));