[Devtools] Added frame grouping to network panel

[Devtools] Added frame grouping to network panel

Adds an option to group by frame in network panel group experiment.

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

R=pfeldman,caseq
BUG=664704
Review-Url: https://codereview.chromium.org/2788793002
Cr-Commit-Position: refs/heads/master@{#461886}
Committed: https://chromium.googlesource.com/chromium/src/+/2f8a6b3d77598929c50e1605885de8c8361c7c6e

[allada, 2017-03-30 23:41:33]

Patchset #1 (id:1) has been deleted

[allada, 2017-03-30 23:41:38]

Patchset #1 (id:20001) has been deleted

[allada, 2017-03-30 23:41:42]

Patchset #1 (id:40001) has been deleted

[allada, 2017-03-30 23:41:55]

PTL

[pfeldman, 2017-03-30 23:45:51]

https://codereview.chromium.org/2788793002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/data_grid/ViewportDataGrid.js
(right):

https://codereview.chromium.org/2788793002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/data_grid/ViewportDataGrid.js:349: 
roll back

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

https://codereview.chromium.org/2788793002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:1822:
Network.NetworkGroupLookupInterface.LookupKeyNameTuple;
use an objects with key and value fields instead.

https://codereview.chromium.org/2788793002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js
(right):

https://codereview.chromium.org/2788793002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:14:
lookupNameForRequest(request) {
Split this interface into two methods:

lookupGroup(request) : Object
groupName(object) : string

for efficiency

https://codereview.chromium.org/2788793002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:70:
var frameParsedURL = new Common.ParsedURL(frame.url);
Otherwise you parse too much.

https://codereview.chromium.org/2788793002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:77:
for (var stack of creationStackTrace) {
Traverse async.

[allada, 2017-03-31 00:17:42]

PTaL

https://codereview.chromium.org/2788793002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js
(right):

https://codereview.chromium.org/2788793002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:14:
lookupNameForRequest(request) {
On 2017/03/30 23:45:51, pfeldman wrote:
> Split this interface into two methods:
> 
> lookupGroup(request) : Object
> groupName(object) : string
> 
> for efficiency

Done.

https://codereview.chromium.org/2788793002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:70:
var frameParsedURL = new Common.ParsedURL(frame.url);
On 2017/03/30 23:45:50, pfeldman wrote:
> Otherwise you parse too much.

Done.

https://codereview.chromium.org/2788793002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:77:
for (var stack of creationStackTrace) {
On 2017/03/30 23:45:50, pfeldman wrote:
> Traverse async.

Done.

[pfeldman, 2017-03-31 00:55:06]

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

https://codereview.chromium.org/2788793002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:1813: *
@return {?T}
It can't be template, it should be an Object or "*" since it is interface (has
to be polymorphic).

https://codereview.chromium.org/2788793002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:1815:
groupKeyForRequest(request) {},
Also, just say "groupForRequest"

https://codereview.chromium.org/2788793002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js
(right):

https://codereview.chromium.org/2788793002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:27:
return key;
And here you would need to cast it, which is fine.

https://codereview.chromium.org/2788793002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:57:
NetworkGroupLookup.NetworkProductFrameGroupLookup = class {
It seems like closure gave up altogether compiling it. It should not have
compiled...

https://codereview.chromium.org/2788793002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:89:
name = ProductRegistry.nameForUrl(new Common.ParsedURL(stack.url));
This is going to be expensive at some point. 1000 requests * 20 stack frames
each, we are now parsing 20K URLs on the page load -> stress. I'd suggest that
you introduce a local cache inside this lookup class instance that would at
least not do it over and over. I.e. url -> name map.

[allada, 2017-04-03 17:40:52]

PTaL

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

https://codereview.chromium.org/2788793002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:1813: *
@return {?T}
On 2017/03/31 00:55:06, pfeldman wrote:
> It can't be template, it should be an Object or "*" since it is interface (has
> to be polymorphic).

Done.

https://codereview.chromium.org/2788793002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:1815:
groupKeyForRequest(request) {},
On 2017/03/31 00:55:06, pfeldman wrote:
> Also, just say "groupForRequest"

Done.

https://codereview.chromium.org/2788793002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js
(right):

https://codereview.chromium.org/2788793002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:27:
return key;
On 2017/03/31 00:55:06, pfeldman wrote:
> And here you would need to cast it, which is fine.

Done.

https://codereview.chromium.org/2788793002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:57:
NetworkGroupLookup.NetworkProductFrameGroupLookup = class {
On 2017/03/31 00:55:06, pfeldman wrote:
> It seems like closure gave up altogether compiling it. It should not have
> compiled...

Done.

https://codereview.chromium.org/2788793002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:89:
name = ProductRegistry.nameForUrl(new Common.ParsedURL(stack.url));
On 2017/03/31 00:55:06, pfeldman wrote:
> This is going to be expensive at some point. 1000 requests * 20 stack frames
> each, we are now parsing 20K URLs on the page load -> stress. I'd suggest that
> you introduce a local cache inside this lookup class instance that would at
> least not do it over and over. I.e. url -> name map.

Done.

[pfeldman, 2017-04-04 20:11:59]

lgtm

https://codereview.chromium.org/2788793002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js
(right):

https://codereview.chromium.org/2788793002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:1820:
groupNameForKey(key) {}
nit: groupName(key)

https://codereview.chromium.org/2788793002/diff/140001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js
(right):

https://codereview.chromium.org/2788793002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:79:
frame = /** @type {!SDK.ResourceTreeFrame} */ (frame);
nit: var frame = /** */ (key); - for better readability and easy migration to
let in the future.

https://codereview.chromium.org/2788793002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:98:
_lookupFrameStacktraceName(frame) {
I've just seen this code in the context's version, which makes me thing that
there is a place for the frame namer. You could place it into the
ProductRegistry for now and then we'll see where it belongs. You can do it when
the second instance (console) emerges.

[allada, 2017-04-04 21:25:56]

https://codereview.chromium.org/2788793002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js
(right):

https://codereview.chromium.org/2788793002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:1820:
groupNameForKey(key) {}
On 2017/04/04 20:11:59, pfeldman wrote:
> nit: groupName(key)

Done.

https://codereview.chromium.org/2788793002/diff/140001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js
(right):

https://codereview.chromium.org/2788793002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:79:
frame = /** @type {!SDK.ResourceTreeFrame} */ (frame);
On 2017/04/04 20:11:59, pfeldman wrote:
> nit: var frame = /** */ (key); - for better readability and easy migration to
> let in the future.

Done.

https://codereview.chromium.org/2788793002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/network_group_lookup/NetworkProductGroupLookup.js:98:
_lookupFrameStacktraceName(frame) {
On 2017/04/04 20:11:59, pfeldman wrote:
> I've just seen this code in the context's version, which makes me thing that
> there is a place for the frame namer. You could place it into the
> ProductRegistry for now and then we'll see where it belongs. You can do it
when
> the second instance (console) emerges.

Acknowledged.

[allada, 2017-04-04 21:26:04]

The CQ bit was checked by allada@chromium.org

[allada, 2017-04-04 21:26:05]

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

[allada, 2017-04-04 21:26:15]

Patchset #3 (id:100001) has been deleted

[allada, 2017-04-04 21:26:20]

Patchset #3 (id:120001) has been deleted

[commit-bot: I haz the power, 2017-04-04 21:26:57]

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-04-04 23:15:07]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1491341164952550,
"parent_rev": "d2819bd5ebfb32f0866d92ad6e62a45499e05517", "commit_rev":
"2f8a6b3d77598929c50e1605885de8c8361c7c6e"}

[commit-bot: I haz the power, 2017-04-04 23:15:57]

Description was changed from

==========
[Devtools] Added frame grouping to network panel

Adds an option to group by frame in network panel group experiment.

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

R=pfeldman,caseq
BUG=664704
==========

to

==========
[Devtools] Added frame grouping to network panel

Adds an option to group by frame in network panel group experiment.

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

R=pfeldman,caseq
BUG=664704

Review-Url: https://codereview.chromium.org/2788793002
Cr-Commit-Position: refs/heads/master@{#461886}
Committed:
https://chromium.googlesource.com/chromium/src/+/2f8a6b3d77598929c50e1605885d...
==========

[commit-bot: I haz the power, 2017-04-04 23:15:59]

Committed patchset #4 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/2f8a6b3d77598929c50e1605885d...