MD History: Move queryState to be managed by history-router

MD History: Move querying logic to new <history-query-manager> element

This moves most of the logic related to querying the history backend
into a new element. This makes it simpler to manage updates to the query,
and is the first step in allowing updates to grouped queries to be
reflected to the page URL.

BUG=675841
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2590093002
Cr-Commit-Position: refs/heads/master@{#445305}
Committed: https://chromium.googlesource.com/chromium/src/+/91b05c8bdccd7ffc19047ab04e8a5790976eaebc

[tsergeant, 2016-12-20 03:09:50]

Description was changed from

==========
MD History: Move queryState to be managed by history-router

This moves most of the logic related to querying the history backend
into the <history-router> element. This makes it simpler to manage
the relationship between the query and the URL and will allow us to add
the more complex grouped history state into the URL.

BUG=675841
==========

to

==========
MD History: Move queryState to be managed by history-router

This moves most of the logic related to querying the history backend
into the <history-router> element. This makes it simpler to manage
the relationship between the query and the URL and will allow us to add
the more complex grouped history state into the URL.

BUG=675841
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tsergeant, 2016-12-29 04:47:05]

Patchset #1 (id:1) has been deleted

[tsergeant, 2016-12-29 04:47:11]

Patchset #1 (id:20001) has been deleted

[tsergeant, 2016-12-29 04:47:17]

Patchset #1 (id:40001) has been deleted

[tsergeant, 2016-12-29 04:47:31]

The CQ bit was checked by tsergeant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-29 04:47:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-29 05:45:33]

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

[commit-bot: I haz the power, 2016-12-29 05:45:34]

Dry run: This issue passed the CQ dry run.

[tsergeant, 2017-01-04 04:52:10]

Patchset #2 (id:80001) has been deleted

[tsergeant, 2017-01-04 04:53:55]

tsergeant@chromium.org changed reviewers:
+ calamity@chromium.org

[tsergeant, 2017-01-04 04:53:55]

Alright, here's part 1. What do you think?

General plan is for 3 CLs:

1. Move queryState into router (this CL)
2. Use one-way binding and events for changes to queryState
3. Add grouped query parameters to route

[tsergeant, 2017-01-12 00:47:22]

Patchset #2 (id:100001) has been deleted

[tsergeant, 2017-01-12 00:47:30]

The CQ bit was checked by tsergeant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 00:48:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tsergeant, 2017-01-12 01:01:40]

The CQ bit was checked by tsergeant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 01:02:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tsergeant, 2017-01-12 01:03:25]

Description was changed from

==========
MD History: Move queryState to be managed by history-router

This moves most of the logic related to querying the history backend
into the <history-router> element. This makes it simpler to manage
the relationship between the query and the URL and will allow us to add
the more complex grouped history state into the URL.

BUG=675841
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD History: Move querying logic to new <history-query-manager> element

This moves most of the logic related to querying the history backend
into a new element. This makes it simpler to manage updates to the query,
and is the first step in allowing updates to grouped queries to be
reflected to the page URL.

BUG=675841
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tsergeant, 2017-01-12 01:04:28]

As discussed, I've pulled the logic out into a new element. I agree that this is
easier to read. Remaining steps in the plan are still the same, just that code
has moved around.

[commit-bot: I haz the power, 2017-01-12 02:57:51]

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

[commit-bot: I haz the power, 2017-01-12 02:57:52]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tsergeant, 2017-01-12 07:13:44]

The CQ bit was checked by tsergeant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 07:14:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 09:05:21]

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

[commit-bot: I haz the power, 2017-01-12 09:05:22]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tsergeant, 2017-01-16 23:18:49]

The CQ bit was checked by tsergeant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-16 23:18:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-17 00:13:57]

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

[commit-bot: I haz the power, 2017-01-17 00:13:57]

Dry run: This issue passed the CQ dry run.

[calamity, 2017-01-17 06:22:10]

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
File chrome/browser/resources/md_history/app.html (right):

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/md_history/app.html:99:
query-result="{{queryResult_}}">
This can be 1-way bound.

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
File chrome/browser/resources/md_history/query_manager.js (right):

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/md_history/query_manager.js:20: },
A private computed property, perhaps?

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/md_history/query_manager.js:74: lastVisitTime,
maxResults
nit: This is pretty hard to read. What happens if you put a comma at the end?

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
File chrome/browser/resources/md_history/router.js (right):

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/md_history/router.js:20: // TODO(tsergeant: Move this
initialization into query-manager.
Just wondering, why's this hard to do this patch?

[tsergeant, 2017-01-18 03:24:55]

The CQ bit was checked by tsergeant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 03:25:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tsergeant, 2017-01-18 03:27:22]

The CQ bit was checked by tsergeant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 03:27:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tsergeant, 2017-01-18 03:28:48]

Patchset #6 (id:200001) has been deleted

[commit-bot: I haz the power, 2017-01-18 03:29:12]

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

[commit-bot: I haz the power, 2017-01-18 03:29:13]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[tsergeant, 2017-01-18 03:34:01]

The CQ bit was checked by tsergeant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 03:34:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tsergeant, 2017-01-18 03:36:01]

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
File chrome/browser/resources/md_history/app.html (right):

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/md_history/app.html:99:
query-result="{{queryResult_}}">
On 2017/01/17 06:22:10, calamity wrote:
> This can be 1-way bound.

Done.

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
File chrome/browser/resources/md_history/query_manager.js (right):

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/md_history/query_manager.js:20: },
On 2017/01/17 06:22:10, calamity wrote:
> A private computed property, perhaps?

Done.

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/md_history/query_manager.js:74: lastVisitTime,
maxResults
On 2017/01/17 06:22:10, calamity wrote:
> nit: This is pretty hard to read. What happens if you put a comma at the end?

~~magic~~

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
File chrome/browser/resources/md_history/router.js (right):

https://codereview.chromium.org/2590093002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/md_history/router.js:20: // TODO(tsergeant: Move this
initialization into query-manager.
On 2017/01/17 06:22:10, calamity wrote:
> Just wondering, why's this hard to do this patch?

It's an initialization order thing.

If queryState was initialized in query-manager (which initialises first), when
router initializes it will see the top-down command to set searchTerm to ''
before it sees the bottom-up command to set searchTerm to whatever is in the
URL. This means that query strings in the URL get ignored.

I'm honestly not 100% sure why this isn't a problem in the next patch. I'll dig
into it a bit more and let you know.

[commit-bot: I haz the power, 2017-01-18 04:30:11]

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

[commit-bot: I haz the power, 2017-01-18 04:30:12]

Dry run: This issue passed the CQ dry run.

[calamity, 2017-01-18 06:21:13]

lgtm.

[tsergeant, 2017-01-22 23:24:03]

The CQ bit was checked by tsergeant@chromium.org

[tsergeant, 2017-01-22 23:24:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
calamity@chromium.org
Link to the patchset: https://codereview.chromium.org/2590093002/#ps260001
(title: "Rebase")

[commit-bot: I haz the power, 2017-01-22 23:24:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-23 00:27:07]

CQ is committing da patch.

Bot data: {"patchset_id": 260001, "attempt_start_ts": 1485127443252180,
"parent_rev": "f34fbb504afba2fa86f1dd7bbfd90e0134cbe639", "commit_rev":
"91b05c8bdccd7ffc19047ab04e8a5790976eaebc"}

[commit-bot: I haz the power, 2017-01-23 00:27:43]

Description was changed from

==========
MD History: Move querying logic to new <history-query-manager> element

This moves most of the logic related to querying the history backend
into a new element. This makes it simpler to manage updates to the query,
and is the first step in allowing updates to grouped queries to be
reflected to the page URL.

BUG=675841
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD History: Move querying logic to new <history-query-manager> element

This moves most of the logic related to querying the history backend
into a new element. This makes it simpler to manage updates to the query,
and is the first step in allowing updates to grouped queries to be
reflected to the page URL.

BUG=675841
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2590093002
Cr-Commit-Position: refs/heads/master@{#445305}
Committed:
https://chromium.googlesource.com/chromium/src/+/91b05c8bdccd7ffc19047ab04e8a...
==========

[commit-bot: I haz the power, 2017-01-23 00:27:44]

Committed patchset #8 (id:260001) as
https://chromium.googlesource.com/chromium/src/+/91b05c8bdccd7ffc19047ab04e8a...