Created attachment 284451 [details] [Image] Overlapping segments The `has-inactive-segment` class should be removed from network bars with zero latency.
<rdar://problem/27516007>
Created attachment 284742 [details] [Patch] Proposed Fix
Comment on attachment 284742 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=284742&action=review r=me > Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js:307 > + let minimumSegmentDuration = graphDataSource.secondsPerPixel * WebInspector.TimelineRecordBar.MinimumWidthPixels; Nit: This block could use a comment like the ChangeLog. Maybe we should assert all the graphDataSource methods somewhere in here. I worry there may be some data source that is missing secondsPerPixel. A brief look around and all data sources now seem fine, but a new data source in the future might forget it. > Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js:308 > + if (barDuration < minimumSegmentDuration || barActiveStartTime - barStartTime < minimumSegmentDuration) { Is the second expression a stricter check then the first? Can we just do that check or is there some way the first is still needed?
Comment on attachment 284742 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=284742&action=review >> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js:307 >> + let minimumSegmentDuration = graphDataSource.secondsPerPixel * WebInspector.TimelineRecordBar.MinimumWidthPixels; > > Nit: This block could use a comment like the ChangeLog. > > Maybe we should assert all the graphDataSource methods somewhere in here. I worry there may be some data source that is missing secondsPerPixel. A brief look around and all data sources now seem fine, but a new data source in the future might forget it. Agreed, it could be overlooked in the future. >> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js:308 >> + if (barDuration < minimumSegmentDuration || barActiveStartTime - barStartTime < minimumSegmentDuration) { > > Is the second expression a stricter check then the first? Can we just do that check or is there some way the first is still needed? The second check catches cases where the bar has a small (or zero) latency that would artificially inflate the bar width by 1-3px.
Comment on attachment 284742 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=284742&action=review >>> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js:308 >>> + if (barDuration < minimumSegmentDuration || barActiveStartTime - barStartTime < minimumSegmentDuration) { >> >> Is the second expression a stricter check then the first? Can we just do that check or is there some way the first is still needed? > > The second check catches cases where the bar has a small (or zero) latency that would artificially inflate the bar width by 1-3px. Correction, the first check is not needed. Will remove.
Created attachment 284893 [details] [Patch] For Landing
Comment on attachment 284893 [details] [Patch] For Landing Clearing flags on attachment: 284893 Committed r203920: <http://trac.webkit.org/changeset/203920>
All reviewed patches have been landed. Closing bug.