[Devtools] New network timeline experiment Part 1

[Devtools] New network timeline experiment Part 1

Introduces skeleton for new canvas based network timeline.

This dies not show any data but presents a model for how it will
work.

Raw image: http://imgur.com/a/NJZuA

R=lushnikov,dgozman,alph
BUG=653738
Committed: https://crrev.com/dcad61be353f50886a0983ed50e7ad6ef192010d
Cr-Commit-Position: refs/heads/master@{#424060}

[allada, 2016-10-06 23:53:48]

Description was changed from

==========
[Devtools] New network timeline experirement Part 1

Introduces skeleton for new canvas based network timeline.

This dies not show any data but presents a model for how it will
work.

R=lushnikov,dgozman,alph
BUG=None
==========

to

==========
[Devtools] New network timeline experiment Part 1

Introduces skeleton for new canvas based network timeline.

This dies not show any data but presents a model for how it will
work.

R=lushnikov,dgozman,alph
BUG=None
==========

[allada, 2016-10-06 23:54:00]

Patchset #1 (id:1) has been deleted

[allada, 2016-10-06 23:54:11]

PTL

[allada, 2016-10-07 00:08:37]

Description was changed from

==========
[Devtools] New network timeline experiment Part 1

Introduces skeleton for new canvas based network timeline.

This dies not show any data but presents a model for how it will
work.

R=lushnikov,dgozman,alph
BUG=None
==========

to

==========
[Devtools] New network timeline experiment Part 1

Introduces skeleton for new canvas based network timeline.

This dies not show any data but presents a model for how it will
work.

R=lushnikov,dgozman,alph
BUG=653738
==========

[allada, 2016-10-07 00:11:46]

Patchset #1 (id:20001) has been deleted

[dgozman, 2016-10-07 21:39:57]

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

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/main/Main.js:94:
Runtime.experiments.register("networkCanvasTimeline", "Canvas based timeline in
Network panel", true);
We keep these sorted both by name and by title at the same time :-)

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

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:274:
show: function(element)
We are not supposed to override show(). It should be resolved in
constructor/wasShown instead.

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:295:
this._outerWidget = new WebInspector.VBox();
Not very descriptive name.

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:887:
this._networkTimelineView = new WebInspector.NetworkTimelineColumn(this,
this._dataGrid);
Can we avoid recreating it all the time?

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

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js:341:
(function()
Place this in constructor.

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js:350:
delete timelineColumn.sortConfig.entries;
This is not needed - you've just assigned to sortConfig.

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

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:416: var
lastSideBarSize = this._previousSideBarWidth || this._splitWidget.sidebarSize();
I don't get why we need this.

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

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkTimelineColumn.js:25:
this._requestData = [];
Let's annotate.

[allada, 2016-10-08 00:10:48]

Patchset #2 (id:20002) has been deleted

[allada, 2016-10-08 00:13:32]

PTaL

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

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/main/Main.js:94:
Runtime.experiments.register("networkCanvasTimeline", "Canvas based timeline in
Network panel", true);
On 2016/10/07 21:39:56, dgozman wrote:
> We keep these sorted both by name and by title at the same time :-)

Done.

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

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:274:
show: function(element)
On 2016/10/07 21:39:57, dgozman wrote:
> We are not supposed to override show(). It should be resolved in
> constructor/wasShown instead.

Done.

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:295:
this._outerWidget = new WebInspector.VBox();
On 2016/10/07 21:39:57, dgozman wrote:
> Not very descriptive name.

Done.

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:887:
this._networkTimelineView = new WebInspector.NetworkTimelineColumn(this,
this._dataGrid);
On 2016/10/07 21:39:57, dgozman wrote:
> Can we avoid recreating it all the time?

Done.

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

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js:341:
(function()
On 2016/10/07 21:39:57, dgozman wrote:
> Place this in constructor.

Done.

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js:350:
delete timelineColumn.sortConfig.entries;
On 2016/10/07 21:39:57, dgozman wrote:
> This is not needed - you've just assigned to sortConfig.

Done.

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

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:416: var
lastSideBarSize = this._previousSideBarWidth || this._splitWidget.sidebarSize();
On 2016/10/07 21:39:57, dgozman wrote:
> I don't get why we need this.

Done.

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

https://codereview.chromium.org/2400093002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkTimelineColumn.js:25:
this._requestData = [];
On 2016/10/07 21:39:57, dgozman wrote:
> Let's annotate.

Done.

[dgozman, 2016-10-08 00:36:41]

lgtm

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js
(right):

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:430: if
(Runtime.experiments.isEnabled("canvasNetworkTimeline"))
I think is not needed anymore.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:445:
this._networkLogView.switchViewMode(!this._networkItemView);
And this can be reverted as well.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/networkLogView.css
(right):

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/networkLogView.css:657:
.timelineHeader {
.network-timeline-header

[allada, 2016-10-08 01:09:02]

Done with dgozman's changes

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js
(right):

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:430: if
(Runtime.experiments.isEnabled("canvasNetworkTimeline"))
On 2016/10/08 00:36:41, dgozman wrote:
> I think is not needed anymore.

Done.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:445:
this._networkLogView.switchViewMode(!this._networkItemView);
On 2016/10/08 00:36:41, dgozman wrote:
> And this can be reverted as well.

Done.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/networkLogView.css
(right):

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/networkLogView.css:657:
.timelineHeader {
On 2016/10/08 00:36:41, dgozman wrote:
> .network-timeline-header

Done.

[alph, 2016-10-08 01:35:09]

lgtm % nits

Also would be nice to have a screenshot.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js
(right):

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:846:
this._timelineHeaderElement = this._timelineWidget.element.createChild("div",
"timelineHeader");
we use dash-style for classes.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:859: var
sortOrder = this._dataGrid.sortOrder() === sortOrders.Ascending  ?
sortOrders.Descending : sortOrders.Ascending;
s/g  /g /

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:1127:
this._timelineColumnSortIcon.classList.remove("sort-ascending");
nit: you can pass several classes in a single call to remove.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:1129: if
(this._dataGrid.sortColumnIdentifier() === "timeline") {
nit: we prefer early returns i.e. if (!...) return;

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js
(right):

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js:12:
if (Runtime.experiments.isEnabled("canvasNetworkTimeline")) {
early return

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/network/NetworkTimelineColumn.js
(right):

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkTimelineColumn.js:133:
if (rowTop + rowHeight <= scrollTop)
you can do better to find the first visible row. :-)

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkTimelineColumn.js:142:
context.save();
nit: move save/restore into _drawBar

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkTimelineColumn.js:150:
_timelineDuration: function()
annotate

[allada, 2016-10-08 02:06:34]

done.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js
(right):

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:846:
this._timelineHeaderElement = this._timelineWidget.element.createChild("div",
"timelineHeader");
On 2016/10/08 01:35:09, alph wrote:
> we use dash-style for classes.

Done.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:859: var
sortOrder = this._dataGrid.sortOrder() === sortOrders.Ascending  ?
sortOrders.Descending : sortOrders.Ascending;
On 2016/10/08 01:35:09, alph wrote:
> s/g  /g /

Done.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:1127:
this._timelineColumnSortIcon.classList.remove("sort-ascending");
On 2016/10/08 01:35:09, alph wrote:
> nit: you can pass several classes in a single call to remove.

Done.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:1129: if
(this._dataGrid.sortColumnIdentifier() === "timeline") {
On 2016/10/08 01:35:09, alph wrote:
> nit: we prefer early returns i.e. if (!...) return;

Done.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js
(right):

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js:12:
if (Runtime.experiments.isEnabled("canvasNetworkTimeline")) {
On 2016/10/08 01:35:09, alph wrote:
> early return

In this case I cannot because it must be done at the top and not retning in the
if. However this code is only during experiment.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/network/NetworkTimelineColumn.js
(right):

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkTimelineColumn.js:133:
if (rowTop + rowHeight <= scrollTop)
On 2016/10/08 01:35:09, alph wrote:
> you can do better to find the first visible row. :-)

Done.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkTimelineColumn.js:142:
context.save();
On 2016/10/08 01:35:09, alph wrote:
> nit: move save/restore into _drawBar

Done.

https://codereview.chromium.org/2400093002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkTimelineColumn.js:150:
_timelineDuration: function()
On 2016/10/08 01:35:09, alph wrote:
> annotate

Done.

[allada, 2016-10-08 02:06:57]

The CQ bit was checked by allada@chromium.org

[allada, 2016-10-08 02:06:57]

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/2400093002/#ps150001
(title: "changes")

[commit-bot: I haz the power, 2016-10-08 02:07:14]

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

[allada, 2016-10-08 02:08:14]

Description was changed from

==========
[Devtools] New network timeline experiment Part 1

Introduces skeleton for new canvas based network timeline.

This dies not show any data but presents a model for how it will
work.

R=lushnikov,dgozman,alph
BUG=653738
==========

to

==========
[Devtools] New network timeline experiment Part 1

Introduces skeleton for new canvas based network timeline.

This dies not show any data but presents a model for how it will
work.

Raw image: http://imgur.com/a/NJZuA

R=lushnikov,dgozman,alph
BUG=653738
==========

[commit-bot: I haz the power, 2016-10-08 03:40:57]

Description was changed from

==========
[Devtools] New network timeline experiment Part 1

Introduces skeleton for new canvas based network timeline.

This dies not show any data but presents a model for how it will
work.

Raw image: http://imgur.com/a/NJZuA

R=lushnikov,dgozman,alph
BUG=653738
==========

to

==========
[Devtools] New network timeline experiment Part 1

Introduces skeleton for new canvas based network timeline.

This dies not show any data but presents a model for how it will
work.

Raw image: http://imgur.com/a/NJZuA

R=lushnikov,dgozman,alph
BUG=653738
==========

[commit-bot: I haz the power, 2016-10-08 03:40:58]

Committed patchset #6 (id:150001)

[commit-bot: I haz the power, 2016-10-08 03:43:29]

Description was changed from

==========
[Devtools] New network timeline experiment Part 1

Introduces skeleton for new canvas based network timeline.

This dies not show any data but presents a model for how it will
work.

Raw image: http://imgur.com/a/NJZuA

R=lushnikov,dgozman,alph
BUG=653738
==========

to

==========
[Devtools] New network timeline experiment Part 1

Introduces skeleton for new canvas based network timeline.

This dies not show any data but presents a model for how it will
work.

Raw image: http://imgur.com/a/NJZuA

R=lushnikov,dgozman,alph
BUG=653738

Committed: https://crrev.com/dcad61be353f50886a0983ed50e7ad6ef192010d
Cr-Commit-Position: refs/heads/master@{#424060}
==========

[commit-bot: I haz the power, 2016-10-08 03:43:30]

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