# STEPS TO REPRODUCE: 1. inspect any page with some activity 2. enable the CPU/memory timeline 3. start a timeline recording 4. scroll/interact with the page to capture activity 5. stop the timeline recording 6. try zooming in/our or scrolling left/right => CPU "columns" may appear/disappear if the record is mostly offscreen => the memory graph may suddenly update when the next record becomes visible (e.g. the slope of the graph changes)
<rdar://problem/46886315>
Created attachment 368617 [details] Patch
Comment on attachment 368617 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368617&action=review > Source/WebInspectorUI/UserInterface/Models/Timeline.js:122 > + return this._records.slice(constrain(lowerIndex - 1), constrain(upperIndex + 1)); It sounds like records overlapping time range now include one record before and after both ends? That is not what I'd expect in some cases. Sometimes we use this to get an exact count of the records in the range. If we are including the surrounding records that are not overlapping then that is bad. > Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:1270 > - let networkRecords = networkTimeline ? networkTimeline.recordsInTimeRange(startTime, endTime) : []; > + let networkRecords = networkTimeline ? networkTimeline.recordsOverlappingTimeRange(startTime, endTime) : []; Does this now include records outside the range? We use this for a count.
Created attachment 369026 [details] Patch
Created attachment 369033 [details] Patch
Comment on attachment 369033 [details] Patch Attachment 369033 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12096164 New failing tests: security/contentSecurityPolicy/video-with-file-url-allowed-by-media-src-star.html security/contentSecurityPolicy/video-with-file-url-allowed-by-media-src-star-with-AllowContentSecurityPolicySourceStarToMatchAnyProtocol-enabled.html
Created attachment 369053 [details] Archive of layout-test-results from ews214 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews214 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment on attachment 369033 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=369033&action=review r=me > Source/WebInspectorUI/UserInterface/Models/CPUTimeline.js:37 > + super.addRecord(record); > + > + if (lastRecord) > + record.adjustStartTimeToLastRecord(lastRecord); I don't think I like this approach of modifying the CPUTimelineRecord like this. That said, it seems like an elegant solution. This means we are treating records not as a Sample time but as a Range from the last timestamp to the sample time. This changes the meaning of the model class slightly from my mental model. I suppose it is fine because CPUTimelineRecord has `timestamp` for the sample time and `start/end` may be used for other purposes. Either way, I would expect the modification of the record to happen before `super.addRecord`, thus before any observers would have seen the pre-modified `startTime`. > Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:450 > - // Don't include the record before the graph start if the graph start is within a gap. > - let includeRecordBeforeStart = !discontinuities.length || discontinuities[0].startTime > graphStartTime; > - let visibleRecords = this.representedObject.recordsInTimeRange(graphStartTime, visibleEndTime, includeRecordBeforeStart); > + let visibleRecords = this.representedObject.recordsInTimeRange(graphStartTime, visibleEndTime, { > + includeRecordBeforeStart: !discontinuities.length || discontinuities[0].startTime > graphStartTime, > + }); Maybe this could include AfterEnd, and we would see the graph slope up/down to the next entry that is just beyond the selection range. I don't think we do that properly right now. > Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:456 > + Oops. > Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js:787 > + if (startTime < this.selectionStartTime || endTime > this.selectionStartTime + this.selectionDuration || firstRecord instanceof WI.CPUTimelineRecord) { Wrap the `endTime > (...)` in parens to improve readability.
Created attachment 370184 [details] Patch
Created attachment 370186 [details] Patch
Comment on attachment 370186 [details] Patch Clearing flags on attachment: 370186 Committed r245498: <https://trac.webkit.org/changeset/245498>
All reviewed patches have been landed. Closing bug.