DevTools: [network] simplify type column

DevTools: [network] simplify type column



BUG=460661
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=190824

[paulirish, 2015-02-21 19:06:37]

paulirish@chromium.org changed reviewers:
+ eustas@chromium.org

[eustas, 2015-02-24 05:54:04]

https://codereview.chromium.org/947583003/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/network/NetworkDataGridNode.js (right):

https://codereview.chromium.org/947583003/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/network/NetworkDataGridNode.js:298: var resourceName =
this._request._resourceType.name();
I'd prefer call .name() only in last "else" case and compare resourceType with
WebInspector.resourceTypes items

https://codereview.chromium.org/947583003/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/network/NetworkDataGridNode.js:300: function
stripTopLevel(mimeType, top)
Probably this should be inlined to avoid repetitive construction of RegExp
object.

https://codereview.chromium.org/947583003/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/network/NetworkDataGridNode.js:313: 
Nuke new-line

[pfeldman, 2015-02-24 10:14:54]

pfeldman@chromium.org changed reviewers:
+ pfeldman@chromium.org

[pfeldman, 2015-02-24 10:14:55]

https://codereview.chromium.org/947583003/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/network/NetworkDataGridNode.js (right):

https://codereview.chromium.org/947583003/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/network/NetworkDataGridNode.js:300: function
stripTopLevel(mimeType, top)
code style: always annotate all functions.

https://codereview.chromium.org/947583003/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/network/NetworkDataGridNode.js:302: var regex = top ?
new RegExp("^" + top + "\/") : /^(application|image)\//;
Creating regex from external data is almost always a bad idea. Hardcode regex
instead.

https://codereview.chromium.org/947583003/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/network/NetworkDataGridNode.js:303: return
mimeType.replace(regex,'');
code style: double quotes

https://codereview.chromium.org/947583003/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/network/NetworkDataGridNode.js:306: if (resourceName
== "other") {
code style: no need for {} around single line statements.

[paulirish, 2015-02-25 09:08:41]

New patch up with a revised approach. 

More straightforward logic and no more helper function or dynamic regex.

PTAL

[pfeldman, 2015-02-25 09:25:00]

lgtm

[eustas, 2015-02-25 09:41:00]

lgtm

https://codereview.chromium.org/947583003/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/network/NetworkDataGridNode.js (right):

https://codereview.chromium.org/947583003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/network/NetworkDataGridNode.js:298: var resourceName =
this._request.resourceType().name();
I'd rather compare with type instead of name:

var resourceType = this._request.resourceType();
var simpleType = resourceType.name();
if (resourceType == WebInspector.resourceTypes.Other
    || resourceType == WebInspector.resourceTypes.Image)

https://codereview.chromium.org/947583003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/network/NetworkDataGridNode.js:302: || resourceName ==
WebInspector.resourceTypes.Image.name() )
nit: whitespace before closing bracket

[paulirish, 2015-02-25 09:47:43]

https://codereview.chromium.org/947583003/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/network/NetworkDataGridNode.js (right):

https://codereview.chromium.org/947583003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/network/NetworkDataGridNode.js:298: var resourceName =
this._request.resourceType().name();
On 2015/02/25 09:41:00, eustas wrote:
> I'd rather compare with type instead of name:
 
good call. done.

[paulirish, 2015-02-25 09:48:05]

The CQ bit was checked by paulirish@chromium.org

[paulirish, 2015-02-25 09:48:05]

The patchset sent to the CQ was uploaded after l-g-t-m from eustas@chromium.org,
pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/947583003/#ps80001 (title:
"compare resourceType")

[commit-bot: I haz the power, 2015-02-25 09:48:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/947583003/80001

[commit-bot: I haz the power, 2015-02-25 11:58:40]

Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=190824