MD History: Remove list-container and list-behavior

MD History: Remove list-container and list-behavior

These were abstractions to help with switching between the main history
list and the grouped mode view. Now that grouped mode has been removed
(https://codereview.chromium.org/2684493002/), these elements are no
longer necessary, and their logic can be embedded directly into
<history-list>.

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

Review-Url: https://codereview.chromium.org/2684693004
Cr-Commit-Position: refs/heads/master@{#451941}
Committed: https://chromium.googlesource.com/chromium/src/+/e07cf60f84106241ccd5093e5ad3dc23f01e8b7d

[tsergeant, 2017-02-08 03:29:33]

Description was changed from

==========
MD History: Remove list-container and list-behavior

These were abstractions to help with switching between the main history
list and the grouped mode view. Now that grouped mode has been removed
(https://codereview.chromium.org/2684493002/), these elements are no
longer necessary, and their logic can be embedded directly into
<history-list>.

BUG=589357
==========

to

==========
MD History: Remove list-container and list-behavior

These were abstractions to help with switching between the main history
list and the grouped mode view. Now that grouped mode has been removed
(https://codereview.chromium.org/2684493002/), these elements are no
longer necessary, and their logic can be embedded directly into
<history-list>.

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

[tsergeant, 2017-02-13 04:16:46]

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

[commit-bot: I haz the power, 2017-02-13 04:17: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-02-13 04:18:54]

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

[commit-bot: I haz the power, 2017-02-13 04:18:55]

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-02-13 04:32:14]

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

[commit-bot: I haz the power, 2017-02-13 04:32:31]

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-02-13 04:34:38]

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

[commit-bot: I haz the power, 2017-02-13 04:34:40]

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-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[tsergeant, 2017-02-14 00:52:47]

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

[commit-bot: I haz the power, 2017-02-14 00:53:47]

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-02-14 02:15:11]

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

[commit-bot: I haz the power, 2017-02-14 02:15:12]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tsergeant, 2017-02-14 03:48:51]

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

[commit-bot: I haz the power, 2017-02-14 03:49:29]

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-02-14 05:19:56]

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

[commit-bot: I haz the power, 2017-02-14 05:20:00]

Dry run: This issue passed the CQ dry run.

[tsergeant, 2017-02-14 06:12:44]

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

[tsergeant, 2017-02-14 06:12:45]

📦✂️️📦

[calamity, 2017-02-21 02:40:06]

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.js (left):

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:245: else if (e.command.id ==
'delete-command')
nit: Did this change for a reason?

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:318: return;
Do we still need this for synced-device-manager? I'm pretty happy to remove this
and see if tests start flaking.

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/history_list.js (right):

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_list.js:141:
this.set('historyData_.' + index + '.selected', selected);
nit: Can be private.

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_list.js:188: deleteSelected:
function() {
nit: private-able.

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_list.js:209: removeItemsByIndex:
function(indices) {
nit: private-able.

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_list.js:271: loadMoreData_:
function() {
While you're here, you may as well make this and the last handler
'onThingHappened' style.

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_list.js:304: closeMenu_: function()
{
I don't think this is hooked to any events.

[tsergeant, 2017-02-21 23:12:38]

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

[commit-bot: I haz the power, 2017-02-21 23:14:50]

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

[tsergeant, 2017-02-21 23:24:13]

For what it's worth, deleting grouped has removed about 25K (10%) from the
unzipped app bundle.

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.js (left):

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:245: else if (e.command.id ==
'delete-command')
On 2017/02/21 02:40:06, calamity wrote:
> nit: Did this change for a reason?

Oh, just me messing up git.

Done.

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:318: return;
On 2017/02/21 02:40:06, calamity wrote:
> Do we still need this for synced-device-manager? I'm pretty happy to remove
this
> and see if tests start flaking.

I'm pretty sure that the new version of this function is correct and the tests
seem happy enough with it.

scrollTarget is now set through data binding. We still need the rAF ->
scrollHandler() to ensure the shadow updates correctly when switching between
pages.

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/history_list.js (right):

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_list.js:141:
this.set('historyData_.' + index + '.selected', selected);
On 2017/02/21 02:40:06, calamity wrote:
> nit: Can be private.

Done.

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_list.js:188: deleteSelected:
function() {
On 2017/02/21 02:40:06, calamity wrote:
> nit: private-able.

Done.

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_list.js:209: removeItemsByIndex:
function(indices) {
On 2017/02/21 02:40:06, calamity wrote:
> nit: private-able.

Done.

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_list.js:304: closeMenu_: function()
{
On 2017/02/21 02:40:06, calamity wrote:
> I don't think this is hooked to any events.

Moved up into the new Private methods section

[Dan Beam, 2017-02-21 23:26:55]

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.js (left):

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:245: else if (e.command.id ==
'delete-command')
On 2017/02/21 23:24:12, tsergeant wrote:
> On 2017/02/21 02:40:06, calamity wrote:
> > nit: Did this change for a reason?
> 
> Oh, just me messing up git.
> 
> Done.

fwiw: I liked this ^ version better.

[commit-bot: I haz the power, 2017-02-22 01:19:06]

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

[commit-bot: I haz the power, 2017-02-22 01:19:07]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  closure_compilation on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[tsergeant, 2017-02-22 01:53:53]

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

[tsergeant, 2017-02-22 01:54:28]

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/history_list.js (right):

https://codereview.chromium.org/2684693004/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_list.js:271: loadMoreData_:
function() {
On 2017/02/21 02:40:06, calamity wrote:
> While you're here, you may as well make this and the last handler
> 'onThingHappened' style.

Done, oops

[commit-bot: I haz the power, 2017-02-22 01:55:31]

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-02-22 03:58:01]

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

[commit-bot: I haz the power, 2017-02-22 03:58:02]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  closure_compilation on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[calamity, 2017-02-22 07:08:15]

lgtm

[calamity, 2017-02-22 07:08:22]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2017-02-22 07:09:15]

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-02-22 08:58:46]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1487747302241380,
"parent_rev": "602d6f696604aa263c17a78460ac59efbfbe256f", "commit_rev":
"e07cf60f84106241ccd5093e5ad3dc23f01e8b7d"}

[commit-bot: I haz the power, 2017-02-22 08:59:26]

Description was changed from

==========
MD History: Remove list-container and list-behavior

These were abstractions to help with switching between the main history
list and the grouped mode view. Now that grouped mode has been removed
(https://codereview.chromium.org/2684493002/), these elements are no
longer necessary, and their logic can be embedded directly into
<history-list>.

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

to

==========
MD History: Remove list-container and list-behavior

These were abstractions to help with switching between the main history
list and the grouped mode view. Now that grouped mode has been removed
(https://codereview.chromium.org/2684493002/), these elements are no
longer necessary, and their logic can be embedded directly into
<history-list>.

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

Review-Url: https://codereview.chromium.org/2684693004
Cr-Commit-Position: refs/heads/master@{#451941}
Committed:
https://chromium.googlesource.com/chromium/src/+/e07cf60f84106241ccd5093e5ad3...
==========

[commit-bot: I haz the power, 2017-02-22 08:59:27]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/e07cf60f84106241ccd5093e5ad3...