Currently, we update timeline overview upon any refresh of the lower pane. This really needs to be done only when we have new events, or displayed categories change.
Created attachment 133262 [details] Patch
Comment on attachment 133262 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133262&action=review > Source/WebCore/inspector/front-end/TimelinePanel.js:562 > + if (this._needOverviewUpdate) { What about onRecordsCleared? why would we end up here with this._needOverviewUpdate === false anyways?
Created attachment 134295 [details] Patch
Comment on attachment 134295 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134295&action=review A bunch of nits. > Source/WebCore/ChangeLog:6 > + - do not use formatted records and TimelinePresentationModel in Overview; It sounds like you are doing more than one thing in this change. Please consider splitting it into parts. > Source/WebCore/inspector/front-end/TimelineModel.js:88 > +WebInspector.TimelineModel.getStartTime = function(record) No get prefix in WebKit functions. > Source/WebCore/inspector/front-end/TimelineModel.js:90 > + return record.startTime / 1000; I think we should operate original units on the model level. > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:102 > + var categories = WebInspector.TimelinePresentationModel.getCategories(); categories() > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:155 > + this._heapGraph.setSize(this._overviewGrid.element.offsetWidth, 60); Please extract constant. > Source/WebCore/inspector/front-end/TimelinePresentationModel.js:60 > +WebInspector.TimelinePresentationModel.getRecordStyle = function(record) { recordStyle.
Created attachment 134306 [details] Patch
(In reply to comment #4) > (From update of attachment 134295 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=134295&action=review > > A bunch of nits. > > > Source/WebCore/ChangeLog:6 > > + - do not use formatted records and TimelinePresentationModel in Overview; > > It sounds like you are doing more than one thing in this change. Please consider splitting it into parts. Well, these changes are actually quite interconnected, separating them would appear artificial to me. Changed wording of the ChangeLog instead. > No get prefix in WebKit functions. Fixed. > > Source/WebCore/inspector/front-end/TimelineModel.js:90 > > + return record.startTime / 1000; > > I think we should operate original units on the model level. The model does operate original unit, we just provide this as a helper for higher-level code -- presentation model and overview should use same units, as they pass times back and forth. We could change PresentationModel to use original units as well, but I'd prefer this to be a separate change. > categories() Fixed. > > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:155 > > + this._heapGraph.setSize(this._overviewGrid.element.offsetWidth, 60); > > Please extract constant. This was a drive by fix, I got rid of the method and the constant in bug 82471. > > Source/WebCore/inspector/front-end/TimelinePresentationModel.js:60 > > +WebInspector.TimelinePresentationModel.getRecordStyle = function(record) { > > recordStyle. Fixed.
Created attachment 134318 [details] Patch
Comment on attachment 134318 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134318&action=review > Source/WebCore/inspector/front-end/TimelineModel.js:88 > +WebInspector.TimelineModel.startTime = function(record) startTimeInSeconds ?
Committed r112409: <http://trac.webkit.org/changeset/112409>