[MD Extensions] Add support for URL navigation

chrome/browser/resources/md_extensions/manager.js

Index: chrome/browser/resources/md_extensions/manager.js
diff --git a/chrome/browser/resources/md_extensions/manager.js b/chrome/browser/resources/md_extensions/manager.js
index 390944de0a712420eb0fcd0c86c18e9861fa4122..e8520b9f1d190322c7a0bb57c97ca9a7d0a96841 100644
--- a/chrome/browser/resources/md_extensions/manager.js
+++ b/chrome/browser/resources/md_extensions/manager.js
@@ -2,18 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-/**
- * The different pages that can be shown at a time.
- * Note: This must remain in sync with the order in manager.html!
- * @enum {string}
- */
-var Page = {
-  ITEM_LIST: '0',
-  DETAIL_VIEW: '1',
-  KEYBOARD_SHORTCUTS: '2',
-  ERROR_PAGE: '3',
-};
-
 cr.define('extensions', function() {
   'use strict';
 
@@ -83,6 +71,18 @@ cr.define('extensions', function() {
        */
       detailViewItem_: Object,
 
+      /**
+       * The helper object to maintain page state.
+       * @private {!extensions.NavigationHelper}
+       */
+      navigationHelper_: Object,
+
+      /**
+       * The current page being shown.
+       * @private {!PageState}
+       */
+      currentPage_: Object,
+
       /** @type {!Array<!chrome.developerPrivate.ExtensionInfo>} */
       extensions: {
         type: Array,
@@ -114,6 +114,23 @@ cr.define('extensions', function() {
       this.listHelper_ = new ListHelper(this);
       this.sidebar.setListDelegate(this.listHelper_);
       this.readyPromiseResolver.resolve();
+      this.currentPage_ = {page: Page.LIST};
+      this.navigationHelper_ =
+          new extensions.NavigationHelper(function(newPage) {
+        this.changePage(newPage, true);
+      }.bind(this));
+      this.optionsDialog.addEventListener('close', function() {
+        // We update the page when the options dialog closes, but only if we're
+        // still on the details page. We could be on a different page if the
+        // user hit back while the options dialog was visible; in that case, the
+        // new page is already correct.
+        if (this.currentPage_.page == Page.DETAILS) {
+          // This will update the currentPage_ and the NavigationHelper; since
+          // the active page is already the details page, no main page
+          // transition occurs.
+          this.changePage({page: Page.DETAILS, id: this.currentPage_.id});
+        }
+      }.bind(this));
     },
 
     get keyboardShortcuts() {
@@ -141,9 +158,15 @@ cr.define('extensions', function() {
      * @param {!chrome.developerPrivate.ExtensionInfo} data
      */
     showItemDetails: function(data) {
-      this.$['items-list'].willShowItemSubpage(data.id);
-      this.detailViewItem_ = data;
-      this.changePage(Page.DETAIL_VIEW);
+      this.changePage({page: Page.DETAILS, id: data.id});
+    },
+
+    /**
+     * Initializes the page to reflect what's specified in the url so that if
+     * the user visits chrome://extensions/?id=..., we land on the proper page.
+     */
+    initPage: function() {
+      this.changePage(this.navigationHelper_.getCurrentPage(), true);
     },
 
     /**
@@ -154,6 +177,7 @@ cr.define('extensions', function() {
       this.filter = /** @type {string} */ (event.detail);
     },
 
+    /** @private */
     onMenuButtonTap_: function() {
       this.$.drawer.toggle();
     },
@@ -198,6 +222,15 @@ cr.define('extensions', function() {
     },
 
     /**
+     * @return {?chrome.developerPrivate.ExtensionInfo}
+     * @private
+     */
+    getData_: function(id) {
+      return this.extensions[this.getIndexInList_('extensions', id)] ||
+             this.apps[this.getIndexInList_('apps', id)];
+    },
+
+    /**
      * @return {boolean} Whether the list should be visible.
      * @private
      */
@@ -240,10 +273,10 @@ cr.define('extensions', function() {
       // that the DOM will have stale data, but there's no point in causing the
       // extra work.
       if (this.detailViewItem_ && this.detailViewItem_.id == item.id &&
-          this.$.pages.selected == Page.DETAIL_VIEW) {
+          this.$.pages.selected == Page.DETAILS) {
         this.detailViewItem_ = item;
       } else if (this.errorPageItem_ && this.errorPageItem_.id == item.id &&
-                 this.$.pages.selected == Page.ERROR_PAGE) {
+                 this.$.pages.selected == Page.ERRORS) {
         this.errorPageItem_ = item;
       }
     },
@@ -269,13 +302,13 @@ cr.define('extensions', function() {
      */
     getPage_: function(page) {
       switch (page) {
-        case Page.ITEM_LIST:
+        case Page.LIST:
           return this.$['items-list'];
-        case Page.DETAIL_VIEW:
+        case Page.DETAILS:
           return this.$['details-view'];
-        case Page.KEYBOARD_SHORTCUTS:
+        case Page.SHORTCUTS:
           return this.$['keyboard-shortcuts'];
-        case Page.ERROR_PAGE:
+        case Page.ERRORS:
           return this.$['error-page'];
       }
       assertNotReached();
@@ -283,33 +316,73 @@ cr.define('extensions', function() {
 
     /**
      * Changes the active page selection.
-     * @param {Page} toPage
+     * @param {PageState} newPage
+     * @param {boolean=} isSilent If true, does not notify the navigation helper
+     *     of the change.
      */
-    changePage: function(toPage) {
+    changePage: function(newPage, isSilent) {
+      if (this.currentPage_.page == newPage.page &&
+          this.currentPage_.subpage == newPage.subpage &&
+          this.currentPage_.id == newPage.id) {
+        return;
+      }
+
       this.$.drawer.closeDrawer();
+      if (this.optionsDialog.open)
+        this.optionsDialog.close();
+
       var fromPage = this.$.pages.selected;
-      if (fromPage == toPage)
-        return;
-      var entry;
-      var exit;
-      if (fromPage == Page.ITEM_LIST && (toPage == Page.DETAIL_VIEW ||
-                                         toPage == Page.ERROR_PAGE)) {
-        entry = [extensions.Animation.HERO];
-        // The item grid can be larger than the detail view that we're
-        // hero'ing into, so we want to also fade out to avoid any jarring.
-        exit = [extensions.Animation.HERO, extensions.Animation.FADE_OUT];
-      } else if (toPage == Page.ITEM_LIST) {
-        entry = [extensions.Animation.FADE_IN];
-        exit = [extensions.Animation.SCALE_DOWN];
-      } else {
-        assert(toPage == Page.DETAIL_VIEW ||
-               toPage == Page.KEYBOARD_SHORTCUTS);
-        entry = [extensions.Animation.FADE_IN];
-        exit = [extensions.Animation.FADE_OUT];
+      var toPage = newPage.page;
+      var data;
+      if (newPage.id) {
+        data = this.getData_(newPage.id);
+        assert(data, newPage.id);
       }
-      this.getPage_(fromPage).animationHelper.setExitAnimations(exit);
-      this.getPage_(toPage).animationHelper.setEntryAnimations(entry);
-      this.$.pages.selected = toPage;
+
+      if (toPage == Page.DETAILS) {
+        assert(newPage.id);

[michaelpg, 2017/05/01 23:41:25]

throughout this whole function: can you assert on data instead of newPage.id,
since data is what needs to be defined? that way the newPage.id detail doesn't
have to keep popping up. Just my opinion.

also you could do fancy things like:

  this.detailViewItem_ = assert(data);

etc. to condense the code a bit.

[Devlin, 2017/05/02 01:04:07]

Done.

+        this.detailViewItem_ = data;
+      } else if (toPage == Page.ERRORS) {
+        assert(newPage.id);
+        this.errorPageItem_ = data;
+      }
+
+      if (fromPage != toPage) {
+        var entry;

[michaelpg, 2017/05/01 23:41:25]

optional: extract this block into a standalone function

[Devlin, 2017/05/02 01:04:06]

I think that since it's only used here, relies on the same inputs, and has
either multiple outputs or mutates the state of this, it's better being inlined.
 So if you don't feel strongly, I'll keep it like that.

[michaelpg, 2017/05/02 20:58:09]

Acknowledged.

+        var exit;
+        if (fromPage == Page.LIST && (toPage == Page.DETAILS ||
+                                           toPage == Page.ERRORS)) {
+          assert(newPage.id);

[michaelpg, 2017/05/01 23:41:25]

duplicate assertion?

[Devlin, 2017/05/02 01:04:06]

Removed.

+          this.$['items-list'].willShowItemSubpage(data.id);
+          entry = [extensions.Animation.HERO];
+          // The item grid can be larger than the detail view that we're
+          // hero'ing into, so we want to also fade out to avoid any jarring.
+          exit = [extensions.Animation.HERO, extensions.Animation.FADE_OUT];
+        } else if (toPage == Page.LIST) {
+          entry = [extensions.Animation.FADE_IN];
+          exit = [extensions.Animation.SCALE_DOWN];
+        } else {
+          assert(toPage == Page.DETAILS ||
+                 toPage == Page.SHORTCUTS);
+          entry = [extensions.Animation.FADE_IN];
+          exit = [extensions.Animation.FADE_OUT];
+        }
+
+        this.getPage_(fromPage).animationHelper.setExitAnimations(exit);
+        this.getPage_(toPage).animationHelper.setEntryAnimations(entry);
+        this.$.pages.selected = toPage;
+      }
+
+      if (newPage.subpage) {
+        assert(newPage.subpage == Dialog.OPTIONS);
+        assert(newPage.id);
+        this.optionsDialog.show(data);
+      }
+
+      this.currentPage_ = newPage;
+
+      if (!isSilent)
+        this.navigationHelper_.updateHistory(newPage);
     },
 
     /**
@@ -327,24 +400,21 @@ cr.define('extensions', function() {
      * @private
      */
     onShouldShowItemErrors_: function(e) {
-      var data = e.detail.data;
-      this.$['items-list'].willShowItemSubpage(data.id);
-      this.errorPageItem_ = data;
-      this.changePage(Page.ERROR_PAGE);
+      this.changePage({page: Page.ERRORS, id: e.detail.data.id});
     },
 
     /** @private */
     onDetailsViewClose_: function() {
       // Note: we don't reset detailViewItem_ here because doing so just causes
       // extra work for the data-bound details view.
-      this.changePage(Page.ITEM_LIST);
+      this.changePage({page: Page.LIST});
     },
 
     /** @private */
     onErrorPageClose_: function() {
       // Note: we don't reset errorPageItem_ here because doing so just causes
       // extra work for the data-bound error page.
-      this.changePage(Page.ITEM_LIST);
+      this.changePage({page: Page.LIST});
     },
 
     /** @private */
@@ -376,12 +446,12 @@ cr.define('extensions', function() {
       }
 
       this.manager_.$/* hack */ ['items-list'].set('items', assert(items));
-      this.manager_.changePage(Page.ITEM_LIST);
+      this.manager_.changePage({page: Page.LIST});
     },
 
     /** @override */
     showKeyboardShortcuts: function() {
-      this.manager_.changePage(Page.KEYBOARD_SHORTCUTS);
+      this.manager_.changePage({page: Page.SHORTCUTS});
     },
   };