WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
160147
Web Inspector: Inactive/active network bar segments overlap when latency is zero
https://bugs.webkit.org/show_bug.cgi?id=160147
Summary
Web Inspector: Inactive/active network bar segments overlap when latency is zero
Matt Baker
Reported
2016-07-24 12:10:54 PDT
Created
attachment 284451
[details]
[Image] Overlapping segments The `has-inactive-segment` class should be removed from network bars with zero latency.
Attachments
[Image] Overlapping segments
(114.33 KB, image/png)
2016-07-24 12:10 PDT
,
Matt Baker
no flags
Details
[Patch] Proposed Fix
(3.82 KB, patch)
2016-07-27 16:15 PDT
,
Matt Baker
no flags
Details
Formatted Diff
Diff
[Patch] For Landing
(4.26 KB, patch)
2016-07-29 13:15 PDT
,
Matt Baker
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2016-07-24 12:11:08 PDT
<
rdar://problem/27516007
>
Matt Baker
Comment 2
2016-07-27 16:15:11 PDT
Created
attachment 284742
[details]
[Patch] Proposed Fix
Joseph Pecoraro
Comment 3
2016-07-28 11:21:54 PDT
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?
Matt Baker
Comment 4
2016-07-28 13:16:23 PDT
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.
Matt Baker
Comment 5
2016-07-29 13:02:40 PDT
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.
Matt Baker
Comment 6
2016-07-29 13:15:06 PDT
Created
attachment 284893
[details]
[Patch] For Landing
WebKit Commit Bot
Comment 7
2016-07-29 13:44:57 PDT
Comment on
attachment 284893
[details]
[Patch] For Landing Clearing flags on attachment: 284893 Committed
r203920
: <
http://trac.webkit.org/changeset/203920
>
WebKit Commit Bot
Comment 8
2016-07-29 13:45:00 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug