This adds an "aligned mode" to the timeline panel, which is differs in the following aspects: - asynchronous events are not connected as children to those events that caused them - all events are aligned by top-level event start time (synchronous children are offset from the parent by their relative start time) - the overview pane is replaced by vertical bars for each top-level event (or for the larges of sequential group of such events, when there's not enough space) The intended use case for this mode is to troubleshoot occasional hiccups/delays in a page that generate large amount of similar, short-termed events, e.g. to perform an animation.
Created attachment 127990 [details] Patch
Screenshot?
Comment on attachment 127990 [details] Patch Attachment 127990 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11560177 New failing tests: inspector/network-status-non-http.html http/tests/inspector/network/network-sidebar-width.html http/tests/inspector/network/network-initiator-from-console.html
Created attachment 128174 [details] Aligned mode, few events (columns in the overview are wide and rare)
Created attachment 128175 [details] Aligned mode, many events (overview columns are dense; selection window visible)
(In reply to comment #2) > Screenshot? Added. We did not decide on the best way to toggle it yet, and this is the primary reason it's behind the experimental switch. Currently, the mode is toggled by a button in the status bar, and when it's on, the Timeline/Memory toggle in the overview sidebar is hidden. We're considering to revamp the overview sidebar, so that the "normal" timeline mode, overview mode and performance graphs are controlled from there -- however, there are no particular ideas on how the control should look yet.
Comment on attachment 127990 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127990&action=review I don't think you should remove the Timeline/Memory buttons when your mode toggles. > Source/WebCore/inspector/front-end/TimelineGrid.js:118 > + dividerLabelBar._labelElement.textContent = calculator.formatTime(slice * i); why did this change? Calculator could be used for arranging rows by size or latency, etc. > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:40 > + this._alignedMode = false; this._startAtZero > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:76 > + this._alignedOverview = new WebInspector.TimelineAlignedModeOverview(this._presentationModel); lets create it lazily > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:138 > + setAlignedMode: function(enabled) setStartAtZero > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:144 > + this.element.addStyleClass("timeline-aligned-mode"); timeline-start-at-zero. > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:757 > + var calculator = new WebInspector.AlignedTimelineCalculator(); do you need to create calculator on each update? > Source/WebCore/inspector/front-end/TimelinePanel.js:687 > + if (!this._boundariesAreValid) why did this change? > Source/WebCore/inspector/front-end/TimelinePanel.js:713 > + var filter = this._alignMode ? new WebInspector.TimelineAlignedRecordFilter(this._presentationModel, this._rootRecord, this._showShortEvents) You don't really need to re-create filter each time? > Source/WebCore/inspector/front-end/TimelinePanel.js:-717 > - if (updateBoundaries) why did this change? > Source/WebCore/inspector/front-end/TimelinePanel.js:1718 > +WebInspector.TimelineRecordFilter = function(calculator, showShortEvents) Ok, we will hit 2K lines here soon. Time to extract some classes.
Created attachment 128191 [details] Patch
(In reply to comment #7) > (From update of attachment 127990 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=127990&action=review > > I don't think you should remove the Timeline/Memory buttons when your mode toggles. I think the primary use case for memory mode is to correlate changes in heap size against timeline events -- so it does not look like it has much meaning when all events are aligned; on the other hand, we gain some real estate for the overview bars this way. I guess we can change this once we manage to fit all mode controls to the overview sidebar. > > > Source/WebCore/inspector/front-end/TimelineGrid.js:118 > > + dividerLabelBar._labelElement.textContent = calculator.formatTime(slice * i); > > why did this change? Calculator could be used for arranging rows by size or latency, etc. It's just that we only used time in timeline. Ok, I'm bringing this back to formatValue(). > > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:40 > > + this._alignedMode = false; > > this._startAtZero Done. > > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:76 > > + this._alignedOverview = new WebInspector.TimelineAlignedModeOverview(this._presentationModel); > > lets create it lazily Done. > > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:138 > > + setAlignedMode: function(enabled) > > setStartAtZero Done. > > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:144 > > + this.element.addStyleClass("timeline-aligned-mode"); > > timeline-start-at-zero. Done. > > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:757 > > + var calculator = new WebInspector.AlignedTimelineCalculator(); > > do you need to create calculator on each update? Fixed. > > Source/WebCore/inspector/front-end/TimelinePanel.js:687 > > + if (!this._boundariesAreValid) > > why did this change? It's a drive-by change, I'd like to take boundaries update logic out of refreshRecords(). It's a trivial change, refreshRecords() is only called from here. > > Source/WebCore/inspector/front-end/TimelinePanel.js:713 > > + var filter = this._alignMode ? new WebInspector.TimelineAlignedRecordFilter(this._presentationModel, this._rootRecord, this._showShortEvents) > > You don't really need to re-create filter each time? I do, since it maintains some state that needs to be reset when we start enumerating records from 0 (one filter counts records, another holds a calculator). > > Source/WebCore/inspector/front-end/TimelinePanel.js:-717 > > - if (updateBoundaries) > > why did this change? See above. > > Source/WebCore/inspector/front-end/TimelinePanel.js:1718 > > +WebInspector.TimelineRecordFilter = function(calculator, showShortEvents) > > Ok, we will hit 2K lines here soon. Time to extract some classes. I'm doing this gradually by extracting some stuff into presentation model, filters etc. We can move them to a separate file later.
Committed r108502: <http://trac.webkit.org/changeset/108502>
WebKit policy is to always have classes in seperate files. We should do the same in the Inspector. It would encourage less spaghetti code and private function/property access.
(In reply to comment #11) > WebKit policy is to always have classes in seperate files. We should do the same in the Inspector. It would encourage less spaghetti code and private function/property access. +1, I'm going to push for < 1KLOC classes. Private classes are still fine to belong to the same unit as long as it does not grow > 1KLOC.