Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1897463002:
[DevTools] Add sorting to all tabs in network (Closed)
|
Created:
4 years, 8 months ago by allada Modified:
4 years, 8 months ago CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, sergeyv+blink_chromium.org, pfeldman, kozyatinskiy+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[DevTools] Add sorting to all tabs in network
All tabs should now be sortable in network tab log
view.
BUG=602190
R=lushnikov
Committed: https://crrev.com/b0597edee984b9852f1181f04a4702403897a1ca
Cr-Commit-Position: refs/heads/master@{#387941}
Patch Set 1 #Patch Set 2 : #
Total comments: 4
Patch Set 3 : #
Total comments: 8
Patch Set 4 : #
Total comments: 3
Patch Set 5 : #
Messages
Total messages: 17 (6 generated)
PTL
caseq@chromium.org changed reviewers: + caseq@chromium.org
https://codereview.chromium.org/1897463002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js (right): https://codereview.chromium.org/1897463002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:734: WebInspector.NetworkDataGridNode.ResponseHeaderStringComparator = function(propertyName, revert, a, b) do we ever pass revert as true? https://codereview.chromium.org/1897463002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:742: return a._request.indentityCompare(b._request); this could be written as return a.Value.localeCompare(b.value) || a._request.indentityCompare(b._request);
PTL https://codereview.chromium.org/1897463002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js (right): https://codereview.chromium.org/1897463002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:734: WebInspector.NetworkDataGridNode.ResponseHeaderStringComparator = function(propertyName, revert, a, b) On 2016/04/15 20:43:00, caseq wrote: > do we ever pass revert as true? Looks like the only case is for "latency" and "duration" around: https://goo.gl/xQvFOP I kept the call handlers the same as RequestPropertyComparator https://codereview.chromium.org/1897463002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:742: return a._request.indentityCompare(b._request); On 2016/04/15 20:43:00, caseq wrote: > this could be written as > return a.Value.localeCompare(b.value) || > a._request.indentityCompare(b._request); Done.
https://codereview.chromium.org/1897463002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js (right): https://codereview.chromium.org/1897463002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:736: return (aValue.localeCompare(bValue) * (revert ? -1 : 1)) || a._request.indentityCompare(b._request); let's nuke revert, it's not used by callers and only adds complexity here. https://codereview.chromium.org/1897463002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:746: WebInspector.NetworkDataGridNode.ResponseHeaderNumberComparator = function(propertyName, revert, a, b) ditto https://codereview.chromium.org/1897463002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:762: WebInspector.NetworkDataGridNode.ResponseHeaderDateComparator = function(propertyName, revert, a, b) ditto. https://codereview.chromium.org/1897463002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js (right): https://codereview.chromium.org/1897463002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:643: this._sortingFunctions.duration = WebInspector.NetworkDataGridNode.RequestPropertyComparator.bind(null, "duration", false); if you're doing this, we could as well nuke revert in RequestPropertyComparator
PTL https://codereview.chromium.org/1897463002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js (right): https://codereview.chromium.org/1897463002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:736: return (aValue.localeCompare(bValue) * (revert ? -1 : 1)) || a._request.indentityCompare(b._request); On 2016/04/15 21:30:00, caseq wrote: > let's nuke revert, it's not used by callers and only adds complexity here. Done. https://codereview.chromium.org/1897463002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:746: WebInspector.NetworkDataGridNode.ResponseHeaderNumberComparator = function(propertyName, revert, a, b) On 2016/04/15 21:30:00, caseq wrote: > ditto Done. https://codereview.chromium.org/1897463002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:762: WebInspector.NetworkDataGridNode.ResponseHeaderDateComparator = function(propertyName, revert, a, b) On 2016/04/15 21:30:00, caseq wrote: > ditto. Done. https://codereview.chromium.org/1897463002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js (right): https://codereview.chromium.org/1897463002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js:643: this._sortingFunctions.duration = WebInspector.NetworkDataGridNode.RequestPropertyComparator.bind(null, "duration", false); On 2016/04/15 21:30:00, caseq wrote: > if you're doing this, we could as well nuke revert in RequestPropertyComparator Done.
lgtm https://codereview.chromium.org/1897463002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js (right): https://codereview.chromium.org/1897463002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:721: return (aValue > bValue) ? 1 : -1; drop parenthesis or just return aValue - bValue https://codereview.chromium.org/1897463002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:749: return (aValue > bValue) ? 1 : -1; ditto https://codereview.chromium.org/1897463002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:766: return (aValue > bValue) ? 1 : -1; ditto.
The CQ bit was checked by allada@google.com
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/1897463002/#ps80001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1897463002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1897463002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by allada@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1897463002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1897463002/80001
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== [DevTools] Add sorting to all tabs in network All tabs should now be sortable in network tab log view. BUG=602190 R=lushnikov ========== to ========== [DevTools] Add sorting to all tabs in network All tabs should now be sortable in network tab log view. BUG=602190 R=lushnikov Committed: https://crrev.com/b0597edee984b9852f1181f04a4702403897a1ca Cr-Commit-Position: refs/heads/master@{#387941} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/b0597edee984b9852f1181f04a4702403897a1ca Cr-Commit-Position: refs/heads/master@{#387941} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
