This adds an instrumentation interfaces and some visualization for frame-related events: - beginning and end of frame rendering (end is currently ignored) - layer compositing events When frame marks are available, these are displayed in "start at zero" overview mode as vertical lines on main timeline grid, provided it is zoomed to 1 second per screen or closer. Also, start at zero aggregates events by frames.
Created attachment 129844 [details] Patch
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment on attachment 129844 [details] Patch Attachment 129844 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11776932 New failing tests: inspector/timeline/timeline-enum-stability.html
Comment on attachment 129844 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129844&action=review > Source/WebKit/chromium/ChangeLog:20 > + * public/WebWidget.h: > + (WebWidget): > + (WebKit::WebWidget::willBeginUpdate): > + (WebKit::WebWidget::didEndUpdate): > + * src/WebDevToolsAgentImpl.cpp: > + (WebKit::WebDevToolsAgentImpl::willComposeLayers): > + (WebKit): > + (WebKit::WebDevToolsAgentImpl::didComposeLayers): > + (WebKit::WebDevToolsAgentImpl::willBeginFrameUpdate): > + (WebKit::WebDevToolsAgentImpl::didEndFrameUpdate): Please file bugs for the other WebKit ports to hook this up.
Created attachment 129892 [details] Patch
Comment on attachment 129892 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129892&action=review > Source/WebCore/inspector/InspectorController.cpp:214 > +void InspectorController::willBeginFrameUpdate() We use will*/did* notation for non-atomic actions, we use did* notation for atomic actions. Should either be didBeginFrame() / didEndFrame() or willProcessFrame / didProcessFrame. > Source/WebCore/inspector/InspectorController.h:83 > + void willBeginFrameUpdate(); I would move it into the instrumentation. > Source/WebCore/inspector/InspectorController.h:85 > + void willBeginCompositing(); What does Begin / End compositing mean? Can it contain nested paints? If yes, when and why? Also, seems unrelated to the change. > Source/WebCore/inspector/InspectorTimelineAgent.cpp:64 > +static const char LayersCompositing[] = "LayersCompositing"; This will make test fail. > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:792 > + var lastFrameTime = records[0] && records[0].startTime; records.length && ... ? > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:828 > + for (; currentFrame < lastIndex && currentRecord < records.length; ++currentRecord) { Could you extract method? > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:-856 > - var rightIndex = rightOffset + barWidth >= this.element.clientWidth ? null : Math.ceil((rightOffset - offset0 - 2) / barWidth * this._recordsPerBar); What is 2 ? > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:909 > + endTime: rightOffset + 3 > windowSpan ? null : this._barTimes[lastBar].endTime What is 3? > Source/WebKit/chromium/public/WebWidget.h:83 > + virtual void willBeginUpdate() { } 1) "update" looks to generic, WebWidget may be updated in various ways 2) We should emphasize the instrumenting nature of these methods. debugWillBeginUpdate instrumentBeginFrame() > Source/WebKit/chromium/public/WebWidget.h:84 > + virtual void didEndUpdate() { } nuke this? > Source/WebKit/chromium/src/WebDevToolsAgentImpl.cpp:234 > +void WebDevToolsAgentImpl::willBeginFrameUpdate() revert? > Source/WebKit/chromium/src/WebDevToolsAgentImpl.h:72 > + virtual void willBeginFrameUpdate(); revert? > Source/WebKit/chromium/src/WebDevToolsAgentPrivate.h:46 > + virtual void willBeginFrameUpdate() = 0; revert? > Source/WebKit/chromium/src/WebViewImpl.cpp:1271 > + if (m_devToolsAgent) InspectorInstrumentation::didBeginFrame(m_page.get()) > Source/WebKit/chromium/src/WebViewImpl.cpp:1421 > + if (m_devToolsAgent) Separate change please.
Created attachment 129932 [details] Patch
Comment on attachment 129932 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129932&action=review > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:283 > + return this._windowEndTime || this._presentationModel.maximumRecordTime(); !!this._windowEndTime ? > Source/WebCore/inspector/front-end/TimelineOverviewPane.js:809 > + var scale = (this.element.clientHeight - 4) / boundarySpan; Please use const paddingTop = 4;
I don't think we should wait for Darin's approval for the WebView::instrumentBeginFrame() and it can be landed safely. Darin, if you have any concerns about the changes to the API, please let us know and we will fix it.
Committed r109724: <http://trac.webkit.org/changeset/109724>