MD Settings: methods to get root pages and sections

chrome/browser/resources/settings/settings_main/settings_main.js

Index: chrome/browser/resources/settings/settings_main/settings_main.js
diff --git a/chrome/browser/resources/settings/settings_main/settings_main.js b/chrome/browser/resources/settings/settings_main/settings_main.js
index 676429b994c7533a5426cb7ced6b98673c6cadf0..b3376ebecebc698a90b62f26fb1292fcb7fac330 100644
--- a/chrome/browser/resources/settings/settings_main/settings_main.js
+++ b/chrome/browser/resources/settings/settings_main/settings_main.js
@@ -165,11 +165,8 @@ Polymer({
     if (route.subpage.length != 0 || this.showPages_.about)
       return 0;
 
-    var query = 'settings-section[section="' + route.section + '"]';
-    var topSection = this.$$('settings-basic-page').$$(query);
-    if (!topSection && this.showPages_.advanced)
-      topSection = this.$$('settings-advanced-page').$$(query);
-
+    var page = this.getPage_(route);
+    var topSection = page && page.getSection(route.section);
     if (!topSection || !topSection.offsetParent)
       return 0;
 
@@ -186,6 +183,28 @@ Polymer({
   },
 
   /**
+   * Returns the root page (if it exists) for a route.
+   * @param {!settings.Route} route
+   * @return {(?SettingsAboutPageElement|?SettingsAdvancedPageElement|
+   *           ?SettingsBasicPageElement)}
+   */
+  getPage_: function(route) {

[dschuyler, 2016/08/10 17:16:17]

I'd like to hear more about why this function helps.

It seems to query the about page with each call
(which is unlikely to have the element in question).

Is it useful to separate the data types of the return
values?

It also seems like a lot of extra code to read.

[michaelpg, 2016/08/15 18:08:11]

No, it only queries the page for the current route -- so if the current route is
or descends from ABOUT, it will return the about page.
Separate, no; have, yes. The type is useful because MainPageBehavior has
functions like getSection which are useful to settings-main (rather than
re-implementing them here). We have to specify all the types because Closure
isn't smart enough to accept a behavior as a type.
We could try duplicating all of the public member docs from MainPageBehavior
onto an @interface, and claim to Closure that each of our main pages @implements
that interface, but that would be more duplication.

+    if (settings.Route.ABOUT.contains(route)) {
+      return /** @type {?SettingsAboutPageElement} */(
+          this.$$('settings-about-page'));
+    }
+    if (settings.Route.ADVANCED.contains(route)) {
+      return /** @type {?SettingsAdvancedPageElement} */(
+          this.$$('settings-advanced-page'));
+    }
+    if (settings.Route.BASIC.contains(route)) {
+      return /** @type {?SettingsBasicPageElement} */(
+          this.$$('settings-basic-page'));
+    }
+    assertNotReached();
+  },
+
+  /**
    * Navigates to the default search page (if necessary).
    * @private
    */
@@ -207,11 +226,11 @@ Polymer({
     return new Promise(function(resolve, reject) {
       setTimeout(function() {
         var whenSearchDone = settings.getSearchManager().search(
-            query, assert(this.$$('settings-basic-page')));
+            query, assert(this.getPage_(settings.Route.BASIC)));
         assert(
             whenSearchDone ===
                 settings.getSearchManager().search(
-                    query, assert(this.$$('settings-advanced-page'))));
+                    query, assert(this.getPage_(settings.Route.ADVANCED))));
 
         whenSearchDone.then(function(request) {
           resolve();