[Devtools] Added ability to add extension columns to network

[Devtools] Added ability to add extension columns to network

This patch introduces the ability to add extension columns and it bound
the networkGroupingRequests experiment to it (only shown when experiment
is on right now).

See: http://imgur.com/a/WOUqw

R=caseq
BUG=701973
Review-Url: https://codereview.chromium.org/2756583002
Cr-Commit-Position: refs/heads/master@{#457910}
Committed: https://chromium.googlesource.com/chromium/src/+/222cced63f41cf7214483734f4c0b775f7659a97

[allada, 2017-03-15 21:41:55]

Description was changed from

==========
[Devtools] Added ability to add extension columns to network

This patch introduces the ability to add extension columns and it bound
the networkGroupingRequests experiment to it (only shown when experiment
is on right now).

R=caseq
BUG=701973
==========

to

==========
[Devtools] Added ability to add extension columns to network

This patch introduces the ability to add extension columns and it bound
the networkGroupingRequests experiment to it (only shown when experiment
is on right now).

See: http://imgur.com/a/WOUqw

R=caseq
BUG=701973
==========

[allada, 2017-03-15 21:42:02]

PTL

https://codereview.chromium.org/2756583002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js
(right):

https://codereview.chromium.org/2756583002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:999:
if (this.columnExtensions.has(columnIdentifier))
This is the createCell for group nodes which we currently never want to put
extension data into their cell since currently the only extension is for the
"product" which is the "name" of the cell. Otherwise it'd be verbose.

[allada, 2017-03-15 21:49:59]

Patchset #2 (id:20001) has been deleted

[caseq, 2017-03-17 00:16:29]

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

https://codereview.chromium.org/2756583002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:45:
this.columnExtensions = new Map();
Let's make it private, this is just what you need considering derived classes
are in the same file.

https://codereview.chromium.org/2756583002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:485:
static ExtensionColumnComparator(extensionsMap, extensionId, a, b) {
style: functions start with lowercase.

https://codereview.chromium.org/2756583002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:497:
return aValue > bValue ? 1 : -1;
Let's rather make comparison a part of extension interface.

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

https://codereview.chromium.org/2756583002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:692: *
@param {boolean=} scheduleUpdate
deferUpdate?

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

https://codereview.chromium.org/2756583002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js:369:
Object.assign(/** @type {!Object} */ ({}),
Network.NetworkLogViewColumns._defaultColumnConfig, {
I think this cast should not be required.

[allada, 2017-03-17 01:16:09]

Done.

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

https://codereview.chromium.org/2756583002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:45:
this.columnExtensions = new Map();
On 2017/03/17 00:16:29, caseq wrote:
> Let's make it private, this is just what you need considering derived classes
> are in the same file.

Done.

https://codereview.chromium.org/2756583002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:485:
static ExtensionColumnComparator(extensionsMap, extensionId, a, b) {
On 2017/03/17 00:16:29, caseq wrote:
> style: functions start with lowercase.

I am keeping it consistent with the other comparator names.

https://codereview.chromium.org/2756583002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:497:
return aValue > bValue ? 1 : -1;
On 2017/03/17 00:16:29, caseq wrote:
> Let's rather make comparison a part of extension interface.

Done.

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

https://codereview.chromium.org/2756583002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:692: *
@param {boolean=} scheduleUpdate
On 2017/03/17 00:16:29, caseq wrote:
> deferUpdate?

Done.

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

https://codereview.chromium.org/2756583002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js:369:
Object.assign(/** @type {!Object} */ ({}),
Network.NetworkLogViewColumns._defaultColumnConfig, {
On 2017/03/17 00:16:29, caseq wrote:
> I think this cast should not be required.

Done.

[caseq, 2017-03-17 17:53:20]

lgtm % comments

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

https://codereview.chromium.org/2756583002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js:871:
Network.NetworkColumnExtensionInterface.prototype = {
requestComparator should be part of this interface too, right?

https://codereview.chromium.org/2756583002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js:876:
lookupColumnValue(request) {}
Let's drop "lookup", perhaps just columnValue()?

[allada, 2017-03-17 19:01:54]

The CQ bit was checked by allada@chromium.org

[allada, 2017-03-17 19:01:55]

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

[allada, 2017-03-17 19:02:02]

Done.

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

https://codereview.chromium.org/2756583002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js:871:
Network.NetworkColumnExtensionInterface.prototype = {
On 2017/03/17 17:53:20, caseq wrote:
> requestComparator should be part of this interface too, right?

I added it to the wrong interface. Fixed.

https://codereview.chromium.org/2756583002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogViewColumns.js:876:
lookupColumnValue(request) {}
On 2017/03/17 17:53:20, caseq wrote:
> Let's drop "lookup", perhaps just columnValue()?

I'd prefer to keep lookup since it means that the cost is potentially expensive
(which when it's released it will be relatively expensive, because it will split
the domain up, hash each part, then do a hash table lookup on each part).

[commit-bot: I haz the power, 2017-03-17 19:02:56]

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-03-17 22:19:41]

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

[commit-bot: I haz the power, 2017-03-17 22:19:42]

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_...)

[allada, 2017-03-17 22:45:44]

The CQ bit was checked by allada@chromium.org

[commit-bot: I haz the power, 2017-03-17 22:46:18]

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-03-18 00:26:38]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1489790744102080,
"parent_rev": "1e85d5d4d0841f935a43b3b3dd60b517139a5b39", "commit_rev":
"222cced63f41cf7214483734f4c0b775f7659a97"}

[commit-bot: I haz the power, 2017-03-18 00:28:21]

Description was changed from

==========
[Devtools] Added ability to add extension columns to network

This patch introduces the ability to add extension columns and it bound
the networkGroupingRequests experiment to it (only shown when experiment
is on right now).

See: http://imgur.com/a/WOUqw

R=caseq
BUG=701973
==========

to

==========
[Devtools] Added ability to add extension columns to network

This patch introduces the ability to add extension columns and it bound
the networkGroupingRequests experiment to it (only shown when experiment
is on right now).

See: http://imgur.com/a/WOUqw

R=caseq
BUG=701973

Review-Url: https://codereview.chromium.org/2756583002
Cr-Commit-Position: refs/heads/master@{#457910}
Committed:
https://chromium.googlesource.com/chromium/src/+/222cced63f41cf7214483734f4c0...
==========

[commit-bot: I haz the power, 2017-03-18 00:28:22]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/222cced63f41cf7214483734f4c0...