[Devtools] Moved flatten children to children nodes instead of datagrid

[Devtools] Moved flatten children to children nodes instead of datagrid

This patch moves the flattenNodeList() and renames it flattenChildren() to
be more appropriate. It is also moves it to the nodes instead of the
DataGrid.

R=dgozman
BUG=None

Committed: https://crrev.com/9d0774bc242a8126f10c9be7922549336162ed88
Committed: https://crrev.com/6f9b09a3d7267a54ec7d5a2fa4c98d838c1add06
Cr-Original-Commit-Position: refs/heads/master@{#434901}
Cr-Commit-Position: refs/heads/master@{#438286}

[allada, 2016-11-28 23:46:31]

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

[allada, 2016-11-28 23:46:42]

PTL

[commit-bot: I haz the power, 2016-11-28 23:47:25]

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

[allada, 2016-11-29 00:38:35]

Description was changed from

==========
[Devtools] Moved flaten children to children nodes instead of datagrid

This patch moves the flatenNodeList() and renames it flatenChildren() to
be more appropriate. It is also moves it to the nodes instead of the
DataGrid.

R=dgozman
BUG=None
==========

to

==========
[Devtools] Moved flatten children to children nodes instead of datagrid

This patch moves the flattenNodeList() and renames it flattenChildren() to
be more appropriate. It is also moves it to the nodes instead of the
DataGrid.

R=dgozman
BUG=None
==========

[dgozman, 2016-11-29 00:42:35]

Let's add a couple of test cases to viewport-datagrid.html

lgtm

[allada, 2016-11-29 01:27:42]

One last look please.

[allada, 2016-11-29 01:27:50]

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

[commit-bot: I haz the power, 2016-11-29 01:28:54]

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

[allada, 2016-11-29 01:53:03]

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

[allada, 2016-11-29 01:53:09]

Patchset #4 (id:60001) has been deleted

[commit-bot: I haz the power, 2016-11-29 01:53:09]

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

[allada, 2016-11-29 01:53:30]

Patchset #4 (id:60001) has been deleted

[allada, 2016-11-29 02:04:01]

Patchset #4 (id:80001) has been deleted

[allada, 2016-11-29 02:04:05]

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

[commit-bot: I haz the power, 2016-11-29 02:04:22]

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

[dgozman, 2016-11-29 02:04:31]

lgtm

[allada, 2016-11-29 02:40:17]

The CQ bit was unchecked by allada@chromium.org

[allada, 2016-11-29 02:40:23]

The CQ bit was checked by allada@chromium.org

[commit-bot: I haz the power, 2016-11-29 02:40:44]

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-11-29 05:17:25]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1480387223264710,
"parent_rev": "212e46a15c602b4a271b0498d24cbbe9835caa0a", "commit_rev":
"26887b001b8e9560f157f64ae723931a4b0d78f0"}

[commit-bot: I haz the power, 2016-11-29 05:17:47]

Description was changed from

==========
[Devtools] Moved flatten children to children nodes instead of datagrid

This patch moves the flattenNodeList() and renames it flattenChildren() to
be more appropriate. It is also moves it to the nodes instead of the
DataGrid.

R=dgozman
BUG=None
==========

to

==========
[Devtools] Moved flatten children to children nodes instead of datagrid

This patch moves the flattenNodeList() and renames it flattenChildren() to
be more appropriate. It is also moves it to the nodes instead of the
DataGrid.

R=dgozman
BUG=None
==========

[commit-bot: I haz the power, 2016-11-29 05:17:48]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-11-29 05:19:57]

Description was changed from

==========
[Devtools] Moved flatten children to children nodes instead of datagrid

This patch moves the flattenNodeList() and renames it flattenChildren() to
be more appropriate. It is also moves it to the nodes instead of the
DataGrid.

R=dgozman
BUG=None
==========

to

==========
[Devtools] Moved flatten children to children nodes instead of datagrid

This patch moves the flattenNodeList() and renames it flattenChildren() to
be more appropriate. It is also moves it to the nodes instead of the
DataGrid.

R=dgozman
BUG=None

Committed: https://crrev.com/9d0774bc242a8126f10c9be7922549336162ed88
Cr-Commit-Position: refs/heads/master@{#434901}
==========

[commit-bot: I haz the power, 2016-11-29 05:19:58]

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

[alph, 2016-12-06 19:10:57]

A revert of this CL (patchset #4 id:100001) has been created in
https://codereview.chromium.org/2554753004/ by alph@chromium.org.

The reason for reverting is: Broke timeline tree view.
BUG=671487.

[allada, 2016-12-07 23:30:30]

Description was changed from

==========
[Devtools] Moved flatten children to children nodes instead of datagrid

This patch moves the flattenNodeList() and renames it flattenChildren() to
be more appropriate. It is also moves it to the nodes instead of the
DataGrid.

R=dgozman
BUG=None

Committed: https://crrev.com/9d0774bc242a8126f10c9be7922549336162ed88
Cr-Commit-Position: refs/heads/master@{#434901}
==========

to

==========
[Devtools] Moved flatten children to children nodes instead of datagrid

This patch moves the flattenNodeList() and renames it flattenChildren() to
be more appropriate. It is also moves it to the nodes instead of the
DataGrid.

R=dgozman
BUG=None

Committed: https://crrev.com/9d0774bc242a8126f10c9be7922549336162ed88
Cr-Commit-Position: refs/heads/master@{#434901}
==========

[allada, 2016-12-07 23:30:59]

allada@chromium.org changed reviewers:
+ alph@chromium.org

[allada, 2016-12-07 23:31:00]

PTaL. I fixed the issue in timeline.

[alph, 2016-12-08 00:54:57]

On 2016/12/07 23:31:00, Blaise wrote:
> PTaL. I fixed the issue in timeline.

Could you please also merge the test unflake patch here. I had to revert it as
well.

[allada, 2016-12-08 01:28:51]

On 2016/12/08 00:54:57, alph wrote:
> On 2016/12/07 23:31:00, Blaise wrote:
> > PTaL. I fixed the issue in timeline.
> 
> Could you please also merge the test unflake patch here. I had to revert it as
> well.

done

[alph, 2016-12-09 03:07:10]

Sorry, I didn't get why you need this functionality on each node?

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/inspector/network/network-columns-sorted.html
(right):

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/network/network-columns-sorted.html:24:
if (++resourceCount >= totalResourceCount) {
we usually prefer early return

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/network/network-columns-sorted.html:25:
debugger;
what does it do here?

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/inspector/components/viewport-datagrid.html
(right):

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/components/viewport-datagrid.html:49:
for (var node of nodes) {
drop {}

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/ui_lazy/ViewportDataGrid.js
(right):

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/ui_lazy/ViewportDataGrid.js:304:
flattenChildren() {
maybe flatChildren. Otherwise it looks like it modifies the node by flattening
its children.

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/ui_lazy/ViewportDataGrid.js:313: if
(!node) {
Accessing OOB element causes V8 to deopt.

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/ui_lazy/ViewportDataGrid.js:318:
node.setDepth(depth);
hmm. what happens to depths if flattenChildren is called not on the root node?

[allada, 2016-12-13 00:07:46]

PTaL

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/inspector/network/network-columns-sorted.html
(right):

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/network/network-columns-sorted.html:24:
if (++resourceCount >= totalResourceCount) {
On 2016/12/09 03:07:09, alph wrote:
> we usually prefer early return

Done.

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/network/network-columns-sorted.html:25:
debugger;
On 2016/12/09 03:07:09, alph wrote:
> what does it do here?

Done.

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/inspector/components/viewport-datagrid.html
(right):

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/components/viewport-datagrid.html:49:
for (var node of nodes) {
On 2016/12/09 03:07:09, alph wrote:
> drop {}

Done.

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/ui_lazy/ViewportDataGrid.js
(right):

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/ui_lazy/ViewportDataGrid.js:304:
flattenChildren() {
On 2016/12/09 03:07:10, alph wrote:
> maybe flatChildren. Otherwise it looks like it modifies the node by flattening
> its children.

Done.

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/ui_lazy/ViewportDataGrid.js:313: if
(!node) {
On 2016/12/09 03:07:09, alph wrote:
> Accessing OOB element causes V8 to deopt.

I moved this code, but yes I should have fixed it.

https://codereview.chromium.org/2540543002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/ui_lazy/ViewportDataGrid.js:318:
node.setDepth(depth);
On 2016/12/09 03:07:10, alph wrote:
> hmm. what happens to depths if flattenChildren is called not on the root node?

turns out this was unused code.

[alph, 2016-12-13 01:38:23]

Great. Thank you!

What about my question regarding why do you need it at all? Where are flat
children of a non-root node going to be used?

[alph, 2016-12-13 01:38:26]

Great. Thank you!

What about my question regarding why do you need it at all? Where are flat
children of a non-root node going to be used?

[allada, 2016-12-13 01:47:25]

On 2016/12/13 01:38:26, alph wrote:
> Great. Thank you!
> 
> What about my question regarding why do you need it at all? Where are flat
> children of a non-root node going to be used?

It's for a follow up. I am trying to restructure a bit of the code in prep for
network
request grouping. The next CL is/was to begin the experiment. Flatten node was
to make
it easier to transport the requests that are grouped from the NetworkLogView to
the
NetworkWaterfall, since waterfall will soon allow multiple requests per row.

[alph, 2016-12-13 02:52:36]

Thank you. lgtm

[allada, 2016-12-13 18:45:29]

The CQ bit was checked by allada@chromium.org

[allada, 2016-12-13 18:45:29]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2540543002/#ps200001
(title: "changes")

[commit-bot: I haz the power, 2016-12-13 18:46:08]

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-13 18:49:17]

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

[commit-bot: I haz the power, 2016-12-13 18:49:18]

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

[allada, 2016-12-13 18:57:01]

The CQ bit was checked by allada@chromium.org

[allada, 2016-12-13 18:57:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, alph@chromium.org
Link to the patchset: https://codereview.chromium.org/2540543002/#ps220001
(title: "Merge branch 'master' into FLATEN_CHILDREN")

[commit-bot: I haz the power, 2016-12-13 18:57:33]

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-13 21:40:09]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1481655421293340,
"parent_rev": "27c2db1f78cd920987da9a962aab4868dc7e985f", "commit_rev":
"53ed489f4e87e4145d8503200c39b9c2376528d0"}

[commit-bot: I haz the power, 2016-12-13 21:40:39]

Description was changed from

==========
[Devtools] Moved flatten children to children nodes instead of datagrid

This patch moves the flattenNodeList() and renames it flattenChildren() to
be more appropriate. It is also moves it to the nodes instead of the
DataGrid.

R=dgozman
BUG=None

Committed: https://crrev.com/9d0774bc242a8126f10c9be7922549336162ed88
Cr-Commit-Position: refs/heads/master@{#434901}
==========

to

==========
[Devtools] Moved flatten children to children nodes instead of datagrid

This patch moves the flattenNodeList() and renames it flattenChildren() to
be more appropriate. It is also moves it to the nodes instead of the
DataGrid.

R=dgozman
BUG=None

Committed: https://crrev.com/9d0774bc242a8126f10c9be7922549336162ed88
Cr-Commit-Position: refs/heads/master@{#434901}
Review-Url: https://codereview.chromium.org/2540543002
==========

[commit-bot: I haz the power, 2016-12-13 21:40:41]

Committed patchset #10 (id:220001)

[commit-bot: I haz the power, 2016-12-13 21:43:40]

Description was changed from

==========
[Devtools] Moved flatten children to children nodes instead of datagrid

This patch moves the flattenNodeList() and renames it flattenChildren() to
be more appropriate. It is also moves it to the nodes instead of the
DataGrid.

R=dgozman
BUG=None

Committed: https://crrev.com/9d0774bc242a8126f10c9be7922549336162ed88
Cr-Commit-Position: refs/heads/master@{#434901}
Review-Url: https://codereview.chromium.org/2540543002
==========

to

==========
[Devtools] Moved flatten children to children nodes instead of datagrid

This patch moves the flattenNodeList() and renames it flattenChildren() to
be more appropriate. It is also moves it to the nodes instead of the
DataGrid.

R=dgozman
BUG=None

Committed: https://crrev.com/9d0774bc242a8126f10c9be7922549336162ed88
Committed: https://crrev.com/6f9b09a3d7267a54ec7d5a2fa4c98d838c1add06
Cr-Original-Commit-Position: refs/heads/master@{#434901}
Cr-Commit-Position: refs/heads/master@{#438286}
==========

[commit-bot: I haz the power, 2016-12-13 21:43:42]

Patchset 10 (id:??) landed as
https://crrev.com/6f9b09a3d7267a54ec7d5a2fa4c98d838c1add06
Cr-Commit-Position: refs/heads/master@{#438286}