Options: maintain history entries on the parent frame.

chrome/browser/ui/webui/options/options_browsertest.js

Index: chrome/browser/ui/webui/options/options_browsertest.js
diff --git a/chrome/browser/ui/webui/options/options_browsertest.js b/chrome/browser/ui/webui/options/options_browsertest.js
index 9b15733a2977adea03516ed26826197652cfad82..9a9b414d197cd4cb73cb651f090ca8edf09db285 100644
--- a/chrome/browser/ui/webui/options/options_browsertest.js
+++ b/chrome/browser/ui/webui/options/options_browsertest.js
@@ -401,8 +401,8 @@ OptionsWebUIExtendedTest.prototype = {
     // TODO(pamg): Add a test fixture that loads chrome://settings and uses
     // UI elements to access sub-pages, so we can test the titles and
     // search-page URLs.
-    var fullExpectedUrl = 'chrome://settings-frame/' +
-                          (expectedUrl ? expectedUrl : lastExpected);
+    expectedUrl = (expectedUrl == undefined) ? lastExpected : expectedUrl;

[Dan Beam, 2014/05/19 23:06:49]

can L395-404 be:

  expectedUrl = expectedUrl || expectedPages.slice(-1)[0];

?

[davidben, 2014/05/19 23:32:28]

Problem is the empty string is falsey.

[Dan Beam, 2014/05/19 23:49:10]

yeah, ok, can you use `=== undefined` or `typeof expectedUrl == 'undefined'`
here instead?

[davidben, 2014/05/20 00:12:55]

Done.

+    var fullExpectedUrl = 'chrome://settings-frame/' + expectedUrl;
     assertEquals(fullExpectedUrl, window.location.href);

[Dan Beam, 2014/05/19 23:06:49]

i think these should be expect*() rather than assert*()

[davidben, 2014/05/19 23:32:28]

...that would explain why they failed by hanging rather aborting. Done.

 
     // Collect open pages.
@@ -483,7 +483,7 @@ TEST_F('OptionsWebUIExtendedTest', 'DISABLED_ShowSearchPageNoQuery',
 
 // Show a page without updating history.
 TEST_F('OptionsWebUIExtendedTest', 'ShowPageNoHistory', function() {
-  this.verifyOpenPages_(['settings']);
+  this.verifyOpenPages_(['settings'], '');
   // There are only two main pages, 'settings' and 'search'. It's not possible
   // to show the search page using OptionsPage.showPageByName, because it
   // reverts to the settings page if it has no search text set. So we show the
@@ -494,12 +494,12 @@ TEST_F('OptionsWebUIExtendedTest', 'ShowPageNoHistory', function() {
   // the search results. Furthermore, the URL hasn't been updated in the parent
   // page, because we've loaded the chrome-settings frame instead of the whole
   // settings page, so the cross-origin call to set the URL fails.
-  this.verifyOpenPages_(['settings', 'search'], 'settings#query');

[Dan Beam, 2014/05/19 23:06:49]

i'm generally confused, why s/settings//g ?

[davidben, 2014/05/19 23:32:28]

The settings OptionsPage is kind of weird in that, although page.name is
'settings', it appears in the top-level URL as empty. I.e., the URL is
chrome://settings/ and not chrome://settings/settings. This used to happen as a
special case in updateHistoryState_ where the inner path had the extra
"settings", but the outer path didn't; note how chrome://settings-frame/
rewrites to chrome://settings-frame/settings today, but chrome://settings
doesn't rewrite to chrome://settings/settings. This is the page ==
this.getDefaultPage() check.

Since the inner/outer syncing is now done within uber.pushState, that
special-case is gone. Probably not a bad plan anyway since this means that these
tests actually assert on URLs that (assuming uber_utils.js works correctly) are
analogous to what end up in the address bar.

(settings -> search is because that's another discrepancy between the two
frames. search is the version the outer frame sees, so the inner frame now gets
that too.)

+  this.verifyOpenPages_(['settings', 'search'], 'search#query');
   var self = this;
-  this.verifyHistory_(['settings', 'settings#query'], function() {
+  this.verifyHistory_(['', 'search#query'], function() {
     OptionsPage.showPageByName('settings', false);
-    self.verifyOpenPages_(['settings'], 'settings#query');
-    self.verifyHistory_(['settings', 'settings#query'], testDone);
+    self.verifyOpenPages_(['settings'], 'search#query');
+    self.verifyHistory_(['', 'search#query'], testDone);
   });
 });
 
@@ -507,10 +507,10 @@ TEST_F('OptionsWebUIExtendedTest', 'ShowPageWithHistory', function() {
   // See comments for ShowPageNoHistory.
   $('search-field').onsearch({currentTarget: {value: 'query'}});
   var self = this;
-  this.verifyHistory_(['settings', 'settings#query'], function() {
+  this.verifyHistory_(['', 'search#query'], function() {
     OptionsPage.showPageByName('settings', true);
-    self.verifyOpenPages_(['settings'], 'settings#query');
-    self.verifyHistory_(['settings', 'settings#query', 'settings#query'],
+    self.verifyOpenPages_(['settings'], '#query');
+    self.verifyHistory_(['', 'search#query', '#query'],
                         testDone);
   });
 });
@@ -519,10 +519,10 @@ TEST_F('OptionsWebUIExtendedTest', 'ShowPageReplaceHistory', function() {
   // See comments for ShowPageNoHistory.
   $('search-field').onsearch({currentTarget: {value: 'query'}});
   var self = this;
-  this.verifyHistory_(['settings', 'settings#query'], function() {
+  this.verifyHistory_(['', 'search#query'], function() {
     OptionsPage.showPageByName('settings', true, {'replaceState': true});
-    self.verifyOpenPages_(['settings'], 'settings#query');
-    self.verifyHistory_(['settings', 'settings#query'], testDone);
+    self.verifyOpenPages_(['settings'], '#query');
+    self.verifyHistory_(['', '#query'], testDone);
   });
 });
 
@@ -531,10 +531,10 @@ TEST_F('OptionsWebUIExtendedTest', 'NavigateToPage', function() {
   // See comments for ShowPageNoHistory.
   $('search-field').onsearch({currentTarget: {value: 'query'}});
   var self = this;
-  this.verifyHistory_(['settings', 'settings#query'], function() {
+  this.verifyHistory_(['', 'search#query'], function() {
     OptionsPage.navigateToPage('settings');
-    self.verifyOpenPages_(['settings'], 'settings#query');
-    self.verifyHistory_(['settings', 'settings#query', 'settings#query'],
+    self.verifyOpenPages_(['settings'], '#query');
+    self.verifyHistory_(['', 'search#query', '#query'],
                         testDone);
   });
 });
@@ -544,16 +544,16 @@ TEST_F('OptionsWebUIExtendedTest', 'NavigateToPage', function() {
 TEST_F('OptionsWebUIExtendedTest', 'ShowOverlayNoHistory', function() {
   // Open a layer-1 overlay, not updating history.
   OptionsPage.showPageByName('languages', false);
-  this.verifyOpenPages_(['settings', 'languages'], 'settings');
+  this.verifyOpenPages_(['settings', 'languages'], '');
 
   var self = this;
-  this.verifyHistory_(['settings'], function() {
+  this.verifyHistory_([''], function() {
     // Open a layer-2 overlay for which the layer-1 is a parent, not updating
     // history.
     OptionsPage.showPageByName('addLanguage', false);
     self.verifyOpenPages_(['settings', 'languages', 'addLanguage'],
-                          'settings');
-    self.verifyHistory_(['settings'], testDone);
+                          '');
+    self.verifyHistory_([''], testDone);
   });
 });
 
@@ -563,11 +563,11 @@ TEST_F('OptionsWebUIExtendedTest', 'ShowOverlayWithHistory', function() {
   this.verifyOpenPages_(['settings', 'languages']);
 
   var self = this;
-  this.verifyHistory_(['settings', 'languages'], function() {
+  this.verifyHistory_(['', 'languages'], function() {
     // Open a layer-2 overlay, updating history.
     OptionsPage.showPageByName('addLanguage', true);
     self.verifyOpenPages_(['settings', 'languages', 'addLanguage']);
-    self.verifyHistory_(['settings', 'languages', 'addLanguage'], testDone);
+    self.verifyHistory_(['', 'languages', 'addLanguage'], testDone);
   });
 });
 
@@ -575,11 +575,11 @@ TEST_F('OptionsWebUIExtendedTest', 'ShowOverlayReplaceHistory', function() {
   // Open a layer-1 overlay, updating history.
   OptionsPage.showPageByName('languages', true);
   var self = this;
-  this.verifyHistory_(['settings', 'languages'], function() {
+  this.verifyHistory_(['', 'languages'], function() {
     // Open a layer-2 overlay, replacing history.
     OptionsPage.showPageByName('addLanguage', true, {'replaceState': true});
     self.verifyOpenPages_(['settings', 'languages', 'addLanguage']);
-    self.verifyHistory_(['settings', 'addLanguage'], testDone);
+    self.verifyHistory_(['', 'addLanguage'], testDone);
   });
 });
 
@@ -590,7 +590,7 @@ TEST_F('OptionsWebUIExtendedTest', 'ShowOverlayFurtherAbove', function() {
   OptionsPage.showPageByName('addLanguage', true);
   this.verifyOpenPages_(['settings', 'languages', 'addLanguage']);
   var self = this;
-  this.verifyHistory_(['settings', 'addLanguage'], testDone);
+  this.verifyHistory_(['', 'addLanguage'], testDone);
 });
 
 // Directly show a layer-2 overlay for which the layer-1 overlay is not a
@@ -601,11 +601,11 @@ TEST_F('OptionsWebUIExtendedTest', 'ShowUnrelatedOverlay', function() {
   this.verifyOpenPages_(['settings', 'languages']);
 
   var self = this;
-  this.verifyHistory_(['settings', 'languages'], function() {
+  this.verifyHistory_(['', 'languages'], function() {
     // Open an unrelated layer-2 overlay.
     OptionsPage.showPageByName('cookies', true);
     self.verifyOpenPages_(['settings', 'content', 'cookies']);
-    self.verifyHistory_(['settings', 'languages', 'cookies'], testDone);
+    self.verifyHistory_(['', 'languages', 'cookies'], testDone);
   });
 });
 
@@ -618,18 +618,18 @@ TEST_F('OptionsWebUIExtendedTest', 'CloseOverlay', function() {
   this.verifyOpenPages_(['settings', 'languages', 'addLanguage']);
 
   var self = this;
-  this.verifyHistory_(['settings', 'languages', 'addLanguage'], function() {
+  this.verifyHistory_(['', 'languages', 'addLanguage'], function() {
     // Close the layer-2 overlay.
     OptionsPage.closeOverlay();
     self.verifyOpenPages_(['settings', 'languages']);
     self.verifyHistory_(
-        ['settings', 'languages', 'addLanguage', 'languages'],
+        ['', 'languages', 'addLanguage', 'languages'],
         function() {
       // Close the layer-1 overlay.
       OptionsPage.closeOverlay();
-      self.verifyOpenPages_(['settings']);
+      self.verifyOpenPages_(['settings'], '');
       self.verifyHistory_(
-          ['settings', 'languages', 'addLanguage', 'languages', 'settings'],
+          ['', 'languages', 'addLanguage', 'languages', ''],
           testDone);
     });
   });
@@ -657,25 +657,25 @@ TEST_F('OptionsWebUIExtendedTest', 'OverlayTabNavigation', function() {
 
   // Go back twice, then forward twice.
   self.verifyOpenPages_(['settings', 'languages', 'addLanguage']);
-  self.verifyHistory_(['settings', 'languages', 'addLanguage'], function() {
+  self.verifyHistory_(['', 'languages', 'addLanguage'], function() {
     window.history.back();
     waitForPopstate(function() {
       self.verifyOpenPages_(['settings', 'languages']);
-      self.verifyHistory_(['settings', 'languages'], function() {
+      self.verifyHistory_(['', 'languages'], function() {
         window.history.back();
         waitForPopstate(function() {
-          self.verifyOpenPages_(['settings']);
-          self.verifyHistory_(['settings'], function() {
+          self.verifyOpenPages_(['settings'], '');
+          self.verifyHistory_([''], function() {
             window.history.forward();
             waitForPopstate(function() {
               self.verifyOpenPages_(['settings', 'languages']);
-              self.verifyHistory_(['settings', 'languages'], function() {
+              self.verifyHistory_(['', 'languages'], function() {
                 window.history.forward();
                 waitForPopstate(function() {
                   self.verifyOpenPages_(
                       ['settings', 'languages', 'addLanguage']);
                   self.verifyHistory_(
-                      ['settings', 'languages', 'addLanguage'], testDone);
+                      ['', 'languages', 'addLanguage'], testDone);
                 });
               });
             });
@@ -695,19 +695,19 @@ TEST_F('OptionsWebUIExtendedTest', 'OverlayBackToChild', function() {
   var self = this;
 
   self.verifyOpenPages_(['settings', 'languages', 'addLanguage']);
-  self.verifyHistory_(['settings', 'languages', 'addLanguage'], function() {
+  self.verifyHistory_(['', 'languages', 'addLanguage'], function() {
     // Close the top overlay, then go back to it.
     OptionsPage.closeOverlay();
     self.verifyOpenPages_(['settings', 'languages']);
     self.verifyHistory_(
-        ['settings', 'languages', 'addLanguage', 'languages'],
+        ['', 'languages', 'addLanguage', 'languages'],
         function() {
       // Going back to the 'addLanguage' page should not close 'languages'.
       self.prohibitChangesToOverlay_(options.LanguageOptions.getInstance());
       window.history.back();
       waitForPopstate(function() {
         self.verifyOpenPages_(['settings', 'languages', 'addLanguage']);
-        self.verifyHistory_(['settings', 'languages', 'addLanguage'],
+        self.verifyHistory_(['', 'languages', 'addLanguage'],
                             testDone);
       });
     });
@@ -721,7 +721,7 @@ TEST_F('OptionsWebUIExtendedTest', 'OverlayBackToUnrelated', function() {
   OptionsPage.showPageByName('cookies', true);
   var self = this;
   self.verifyOpenPages_(['settings', 'content', 'cookies']);
-  self.verifyHistory_(['settings', 'languages', 'cookies'], function() {
+  self.verifyHistory_(['', 'languages', 'cookies'], function() {
     window.history.back();
     waitForPopstate(function() {
       self.verifyOpenPages_(['settings', 'languages']);