Settings: Fix Clear Browsing Data dialog scrolling to Privacy page

chrome/browser/resources/settings/route.js

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 cr.define('settings', function() {
   /**
    * Class for navigable routes. May only be instantiated within this file.
    * @constructor
    * @param {string} path
    * @private
@@ skipping 56 matching lines @@
           return true;
       }
       return false;
     },
 
     /**
      * Returns true if this route is a subpage of a section.
      * @return {boolean}
      */
     isSubpage: function() {
-      return !!this.parent && this.parent.section == this.section;
+      return !!this.parent && !!this.section &&

[michaelpg, 2016/08/17 00:21:31]

lol, bet that was fun to debug :(

[tommycli, 2016/08/17 16:48:49]

Acknowledged.

+          this.parent.section == this.section;
     },
   };
 
   // Abbreviated variable for easier definitions.
   var r = Route;
 
   // Root pages.
   r.BASIC = new Route('/');
   r.ADVANCED = new Route('/advanced');
   r.ABOUT = new Route('/help');
@@ skipping 29 matching lines @@
   r.POINTERS = r.DEVICE.createChild('/pointer-overlay');
   r.KEYBOARD = r.DEVICE.createChild('/keyboard-overlay');
   r.DISPLAY = r.DEVICE.createChild('/display');
   r.NOTES = r.DEVICE.createChild('/note');
 </if>
 
   r.PRIVACY = r.ADVANCED.createSection('/privacy', 'privacy');
   r.CERTIFICATES = r.PRIVACY.createChild('/certificates');
 
   // CLEAR_BROWSER_DATA is the only navigable dialog route. It's the only child
-  // of a section that's not a subpage. Don't add any more routes like these.
+  // of a root page that's not a section. Don't add any more routes like these.
   // If more navigable dialogs are needed, add explicit support in Route.
-  r.CLEAR_BROWSER_DATA = r.PRIVACY.createChild('/clearBrowserData');
-  r.CLEAR_BROWSER_DATA.isSubpage = function() { return false; };
+  r.CLEAR_BROWSER_DATA = r.ADVANCED.createChild('/clearBrowserData');
 
   r.SITE_SETTINGS = r.PRIVACY.createChild('/siteSettings');
   r.SITE_SETTINGS_ALL = r.SITE_SETTINGS.createChild('all');
   r.SITE_SETTINGS_ALL_DETAILS = r.SITE_SETTINGS_ALL.createChild('details');
 
   r.SITE_SETTINGS_HANDLERS = r.SITE_SETTINGS.createChild('handlers');
 
   // TODO(tommycli): Find a way to refactor these repetitive category routes.
   r.SITE_SETTINGS_AUTOMATIC_DOWNLOADS =
       r.SITE_SETTINGS.createChild('automaticDownloads');
@@ skipping 192 matching lines @@
       if (queryString)
         url += '?' + queryString;
     }
 
     // History serializes the state, so we don't push the actual route object.
     var previousRoutePath = currentRoute_.path;
     setCurrentRoute(route, params);
     window.history.pushState(previousRoutePath, '', url);
   };
 
+  /**
+   * Navigates to the previous route, but will never exit Settings. If there is
+   * no previous route in the history, navigates to the immediate parent.
+   * @private

[michaelpg, 2016/08/17 00:21:31]

no private

[tommycli, 2016/08/17 16:48:49]

Done.

+   */
+  var navigateToPreviousRoute = function() {
+    var previousRoute =
+        window.history.state &&
+        assert(getRouteForPath(/** @type {string} */ (window.history.state)));
+
+    if (previousRoute && previousRoute.contains(settings.getCurrentRoute()))

[michaelpg, 2016/08/17 00:21:31]

why does it matter if it contains the current route? e.g. if I navigate from one
subpage to another at the same level, shouldn't navigateToPreviousRoute go back
to the previous subpage?

[tommycli, 2016/08/17 16:48:49]

Done.

+      window.history.back();
+    else
+      navigateTo(assert(settings.getCurrentRoute().parent));

[michaelpg, 2016/08/17 00:21:31]

so navigateToPreviousRoute throws if called on a root page?

oh, I see you took this behavior from settings_subpage.js.
"navigateToPreviousRoute" sounds more general, like it wouldn't make the
assertions onTapBack_ did about the route.

[tommycli, 2016/08/17 16:48:49]

Done.

+  };
+
   window.addEventListener('popstate', function(event) {
     // On pop state, do not push the state onto the window.history again.
     setCurrentRoute(getRouteForPath(window.location.pathname) || Route.BASIC,
                     new URLSearchParams(window.location.search));
   });
 
   return {
     Route: Route,
     RouteObserverBehavior: RouteObserverBehavior,
     getRouteForPath: getRouteForPath,
     initializeRouteFromUrl: initializeRouteFromUrl,
     getCurrentRoute: getCurrentRoute,
     getQueryParameters: getQueryParameters,
     navigateTo: navigateTo,
+    navigateToPreviousRoute: navigateToPreviousRoute,
   };
 });