Settings: manage location.hash explicitly on a Page.

Settings: manage location.hash explicitly on a Page.

Maintain the current hash associated each Page's state and use that rather than
querying location.hash directly. This fixes several long-standing issues with
the settings search page and the back/forward list:

- Opening an overlay from a search page no longer keeps the fragment in the
  URL.
- Closing an overlay no longer loses the search in the URL.
- Navigating back from one search page to another actually works.
- Navigating back from the search page doesn't clear the search box.

Add tests for this behavior.

BUG=410204
Committed: https://crrev.com/3c4d33472f6cf253d5cfbc38f3620f2f4d282ad6
Cr-Commit-Position: refs/heads/master@{#294614}

[davidben, 2014-09-10 22:53:37]

davidben@chromium.org changed reviewers:
+ dbeam@chromium.org

[davidben, 2014-09-10 22:53:38]

Known issue, but probably not very important: possibly the page state associated
with each entry should contain all the visible pages and their associated
hashes; right now these two produce different results:

1. about:settings
2. Search for something
3. Open an overlay
4. Back + back to about:settings.
5. Drag down on forwards straight to the overlay.
6. Close the overlay.

vs.

same, but replace 5 with pressing forwards twice.

[Dan Beam, 2014-09-11 21:38:36]

https://codereview.chromium.org/555163007/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/search_page.js (right):

https://codereview.chromium.org/555163007/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/search_page.js:174: 
/** @override */

https://codereview.chromium.org/555163007/diff/40001/ui/webui/resources/js/cr...
File ui/webui/resources/js/cr/ui/page_manager/page.js (right):

https://codereview.chromium.org/555163007/diff/40001/ui/webui/resources/js/cr...
ui/webui/resources/js/cr/ui/page_manager/page.js:67: initializePage: function()
{},
/** Doc comment. */
didChangeHash: function() {},

and then just call didChangeHash() always instead of checking if it exists

https://codereview.chromium.org/555163007/diff/40001/ui/webui/resources/js/cr...
ui/webui/resources/js/cr/ui/page_manager/page.js:127: setHash: function(hash) {
nit:

  if (this.hash == hash)
    return;

https://codereview.chromium.org/555163007/diff/40001/ui/webui/resources/js/cr...
File ui/webui/resources/js/cr/ui/page_manager/page_manager.js (right):

https://codereview.chromium.org/555163007/diff/40001/ui/webui/resources/js/cr...
ui/webui/resources/js/cr/ui/page_manager/page_manager.js:500: overlay.hash =
hash;
why not .setHash()?

[davidben, 2014-09-12 00:32:35]

https://codereview.chromium.org/555163007/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/search_page.js (right):

https://codereview.chromium.org/555163007/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/search_page.js:174: 
On 2014/09/11 21:38:35, Dan Beam wrote:
> /** @override */

Done.

https://codereview.chromium.org/555163007/diff/40001/ui/webui/resources/js/cr...
File ui/webui/resources/js/cr/ui/page_manager/page.js (right):

https://codereview.chromium.org/555163007/diff/40001/ui/webui/resources/js/cr...
ui/webui/resources/js/cr/ui/page_manager/page.js:67: initializePage: function()
{},
On 2014/09/11 21:38:35, Dan Beam wrote:
> /** Doc comment. */
> didChangeHash: function() {},
> 
> and then just call didChangeHash() always instead of checking if it exists

Done.

https://codereview.chromium.org/555163007/diff/40001/ui/webui/resources/js/cr...
ui/webui/resources/js/cr/ui/page_manager/page.js:127: setHash: function(hash) {
On 2014/09/11 21:38:35, Dan Beam wrote:
> nit:
> 
>   if (this.hash == hash)
>     return;

Done.

https://codereview.chromium.org/555163007/diff/40001/ui/webui/resources/js/cr...
File ui/webui/resources/js/cr/ui/page_manager/page_manager.js (right):

https://codereview.chromium.org/555163007/diff/40001/ui/webui/resources/js/cr...
ui/webui/resources/js/cr/ui/page_manager/page_manager.js:500: overlay.hash =
hash;
On 2014/09/11 21:38:35, Dan Beam wrote:
> why not .setHash()?

setHash will notify PageManager and update history which isn't the direction we
want. It's the Page subclass's API rather than PageManager's API. PageManager's
API is all this stuff which sets hash/visible and calls
didShowPage/didChangeHash to notify the Page subclass.

(I.e. one is for pushing page state to history. The other is for pushing history
to page state.)

[Dan Beam, 2014-09-12 03:13:34]

lgtm

[davidben, 2014-09-12 14:19:09]

The CQ bit was checked by davidben@chromium.org

[commit-bot: I haz the power, 2014-09-12 14:21:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/555163007/60001

[commit-bot: I haz the power, 2014-09-12 15:40:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-12 15:40:26]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[davidben, 2014-09-12 17:16:20]

The CQ bit was checked by davidben@chromium.org

[commit-bot: I haz the power, 2014-09-12 17:17:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/555163007/60001

[commit-bot: I haz the power, 2014-09-12 18:11:20]

Committed patchset #4 (id:60001) as 7c4d271df006d037e665a1e1aad89c2b966c9eac

[commit-bot: I haz the power, 2014-09-12 18:30:07]

Patchset 4 (id:??) landed as
https://crrev.com/3c4d33472f6cf253d5cfbc38f3620f2f4d282ad6
Cr-Commit-Position: refs/heads/master@{#294614}