Settings: load iframes anew when switching to a non-current one.

chrome/browser/resources/uber/uber.js

Index: chrome/browser/resources/uber/uber.js
diff --git a/chrome/browser/resources/uber/uber.js b/chrome/browser/resources/uber/uber.js
index 5888f6dbee74e4c231a90365fe880339e1407a96..66c28e6cf5b687a7a726f746fefd48e02f2ee3c5 100644
--- a/chrome/browser/resources/uber/uber.js
+++ b/chrome/browser/resources/uber/uber.js
@@ -105,12 +105,11 @@ cr.define('uber', function() {
       var params = resolvePageInfo();
       assert(params.id === e.state.pageId);
 
-      // If the page doesn't exist, create it. Otherwise, swap it in.
-      var frame = $(params.id).querySelector('iframe');
-      if (!frame)
+      // If the page isn't the current page, load it fresh. Even if the page is
+      // already loaded, it may have state not reflected in the URL, such as the
+      // history page's "Remove selected items" overlay. https://crbug.com/377386

[Dan Beam, 2014/05/27 18:01:16]

https://crbug -> http://crbug to fix 80 col (and so the link works)

[davidben, 2014/05/27 19:18:35]

Done. (https://crbug.com works now, actually.)

+      if (getRequiredElement(params.id) !== getSelectedIframe())
         showPage(params.id, HISTORY_STATE_OPTION.NONE, params.path);
-      else
-        selectPage(params.id);
 
       // Either way, send the state down to it.
       //
@@ -224,12 +223,11 @@ cr.define('uber', function() {
 
   /**
    * Changes the path past the page title (i.e. chrome://chrome/settings/(.*)).
-   * @param {string} pageId Should match an id of one of the iframe containers.
    * @param {Object} state The page's state object for the navigation.
    * @param {string} path The new /path/ to be set after the page name.
    * @param {number} historyOption The type of history modification to make.
    */
-  function changePathTo(pageId, state, path, historyOption) {
+  function changePathTo(state, path, historyOption) {
     assert(!path || path.substr(-1) != '/', 'invalid path given');
 
     var histFunc;
@@ -240,6 +238,7 @@ cr.define('uber', function() {
 
     assert(histFunc, 'invalid historyOption given ' + historyOption);
 
+    var pageId = getSelectedIframe().id;
     var args = [{pageId: pageId, pageState: state},
                 '',
                 '/' + pageId + '/' + (path || '')];
@@ -261,7 +260,7 @@ cr.define('uber', function() {
     // Only update the currently displayed path if this is the visible frame.
     var container = getIframeFromOrigin(origin).parentNode;
     if (container == getSelectedIframe())
-      changePathTo(container.id, state, path, historyOption);
+      changePathTo(state, path, historyOption);
   }
 
   /**
@@ -324,6 +323,7 @@ cr.define('uber', function() {
    */
   function showPage(pageId, historyOption, path) {
     var container = $(pageId);
+    var lastSelected = document.querySelector('.iframe-container.selected');

[Dan Beam, 2014/05/27 18:01:16]

lower to right before use

[davidben, 2014/05/27 19:18:35]

Done.

 
     // Lazy load of iframe contents.
     var sourceUrl = container.dataset.url + (path || '');
@@ -341,22 +341,6 @@ cr.define('uber', function() {
       frame.dataset.ready = false;
     }
 
-    // This must be called before selectPage so that the title change applies to
-    // the new history entry.
-    if (historyOption != HISTORY_STATE_OPTION.NONE)
-      changePathTo(pageId, {}, path, historyOption);
-
-    selectPage(pageId);
-  }
-
-  /**
-   * Switches to a subpage. The subpage must already exist.
-   * @param {string} pageId Should match an id of one of the iframe containers.
-   */
-  function selectPage(pageId) {
-    var container = getRequiredElement(pageId);
-    var lastSelected = document.querySelector('.iframe-container.selected');
-
     // If the last selected container is already showing, ignore the rest.
     if (lastSelected === container)
       return;
@@ -386,6 +370,9 @@ cr.define('uber', function() {
     var selectedFrame = getSelectedIframe().querySelector('iframe');
     uber.invokeMethodOnWindow(selectedFrame.contentWindow, 'frameSelected');
 
+    if (historyOption != HISTORY_STATE_OPTION.NONE)
+      changePathTo({}, path, historyOption);
+
     if (container.dataset.title)
       document.title = container.dataset.title;
     $('favicon').href = 'chrome://theme/' + container.dataset.favicon;