[DevTools] Introduce Common.List used as a backend for list controls

[DevTools] Introduce Common.List used as a backend for list controls

This will make it easier to have sorted lists and list-like select controls.

BUG=none

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

[dgozman, 2017-05-31 22:37:59]

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

[dgozman, 2017-05-31 22:38:15]

dgozman@chromium.org changed reviewers:
+ einbinder@chromium.org, pfeldman@chromium.org

[dgozman, 2017-05-31 22:38:16]

Could you please take a look?

[commit-bot: I haz the power, 2017-05-31 22:38:58]

Dry run: CQ is trying da patch.

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

[pfeldman, 2017-05-31 23:00:02]

lgtm

https://codereview.chromium.org/2916743002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/common/List.js (right):

https://codereview.chromium.org/2916743002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/common/List.js:8: Common.List =
class extends Common.Object {
UI.ListModel

[einbinder, 2017-05-31 23:23:08]

https://codereview.chromium.org/2916743002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/common/List.js (right):

https://codereview.chromium.org/2916743002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/common/List.js:50:
insertItemAtIndex(index, item) {
I don't think this is good to have if we want a sorted list.

https://codereview.chromium.org/2916743002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/ListControl.js (right):

https://codereview.chromium.org/2916743002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:68: this._source
= new Common.List();
Shouldn't this be passed in by the client?

[commit-bot: I haz the power, 2017-05-31 23:45:08]

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

[commit-bot: I haz the power, 2017-05-31 23:45:09]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[dgozman, 2017-06-01 01:03:02]

https://codereview.chromium.org/2916743002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/common/List.js (right):

https://codereview.chromium.org/2916743002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/common/List.js:8: Common.List =
class extends Common.Object {
On 2017/05/31 23:00:02, pfeldman wrote:
> UI.ListModel

Done.

https://codereview.chromium.org/2916743002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/common/List.js:50:
insertItemAtIndex(index, item) {
On 2017/05/31 23:23:08, einbinder wrote:
> I don't think this is good to have if we want a sorted list.

Yeah, we'll figure it out when the need arises. For now, removed TODO below.

https://codereview.chromium.org/2916743002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/ListControl.js (right):

https://codereview.chromium.org/2916743002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:68: this._source
= new Common.List();
On 2017/05/31 23:23:08, einbinder wrote:
> Shouldn't this be passed in by the client?

Well, it's convenient for most of the clients to not pass an empty list.

[dgozman, 2017-06-01 01:03:11]

Description was changed from

==========
[DevTools] Introduce Common.List used as a backend for list controls

This will make it easier to have sorted lists and list-like select controls.

BUG=none
==========

to

==========
[DevTools] Introduce Common.List used as a backend for list controls

This will make it easier to have sorted lists and list-like select controls.

BUG=none
==========

[dgozman, 2017-06-01 01:03:11]

dgozman@chromium.org changed reviewers:
+ caseq@chromium.org

[caseq, 2017-06-01 01:37:57]

lgtm

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector-unit/filtered-item-selection-dialog-filtering.js
(right):

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector-unit/filtered-item-selection-dialog-filtering.js:38:
for (var i = 0; i < items.length(); ++i)
let's make length a getter?

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/Tests.js (right):

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/Tests.js:852: for (var i = 0; i <
items.length(); ++i)
Make it iterable?

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ListControl.js (right):

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:96:
setModel(model) {
I hope this should not be necessary.

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:108: model() {
ditto.

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:122: for (var i =
0; i < data.removed.length; i++)
nit: for of?

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ListModel.js (right):

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListModel.js:122: var removed =
this._items.slice(from, to);
let's do it only in the else branch.

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListModel.js:124:
this._items.splice.bind(this._items, from, to - from).apply(null, items);
removed = this._items.splice(from, to - from, ...items)

[dgozman, 2017-06-01 20:31:49]

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector-unit/filtered-item-selection-dialog-filtering.js
(right):

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector-unit/filtered-item-selection-dialog-filtering.js:38:
for (var i = 0; i < items.length(); ++i)
On 2017/06/01 01:37:56, caseq wrote:
> let's make length a getter?

I'll follow up.

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/Tests.js (right):

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/Tests.js:852: for (var i = 0; i <
items.length(); ++i)
On 2017/06/01 01:37:56, caseq wrote:
> Make it iterable?

I'll follow up.

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ListControl.js (right):

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:96:
setModel(model) {
On 2017/06/01 01:37:57, caseq wrote:
> I hope this should not be necessary.

I don't see any harm in this.

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:108: model() {
On 2017/06/01 01:37:56, caseq wrote:
> ditto.

Removed.

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ListModel.js (right):

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListModel.js:122: var removed =
this._items.slice(from, to);
On 2017/06/01 01:37:57, caseq wrote:
> let's do it only in the else branch.

Done.

https://codereview.chromium.org/2916743002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListModel.js:124:
this._items.splice.bind(this._items, from, to - from).apply(null, items);
On 2017/06/01 01:37:57, caseq wrote:
> removed = this._items.splice(from, to - from, ...items)

Done.

[dgozman, 2017-06-01 20:31:58]

The CQ bit was checked by dgozman@chromium.org

[dgozman, 2017-06-01 20:31:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
pfeldman@chromium.org, caseq@chromium.org
Link to the patchset: https://codereview.chromium.org/2916743002/#ps60001
(title: "addressed comments")

[commit-bot: I haz the power, 2017-06-01 20:32:30]

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-06-01 23:18:56]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1496349118663830,
"parent_rev": "410dbeb8df3d4ab140390d7d1e603163885c8c9c", "commit_rev":
"a974ad98e45321dee1ebee0a9891451ee4b1294f"}

[commit-bot: I haz the power, 2017-06-01 23:19:08]

Description was changed from

==========
[DevTools] Introduce Common.List used as a backend for list controls

This will make it easier to have sorted lists and list-like select controls.

BUG=none
==========

to

==========
[DevTools] Introduce Common.List used as a backend for list controls

This will make it easier to have sorted lists and list-like select controls.

BUG=none

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

[commit-bot: I haz the power, 2017-06-01 23:19:09]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/a974ad98e45321dee1ebee0a9891...