Summary: | [Web Timing] Add a vendor-prefixed Performance Timeline API | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | James Simonsen <simonjam> | ||||||||||||||
Component: | New Bugs | Assignee: | James Simonsen <simonjam> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | abarth, dwright, ojan, rakuco, sullivan, tonyg, vestbo, webkit.review.bot | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 84881, 61152 | ||||||||||||||||
Attachments: |
|
Description
James Simonsen
2012-03-05 16:47:49 PST
Created attachment 130236 [details]
Patch
Comment on attachment 130236 [details] Patch Attachment 130236 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/11833072 Created attachment 130250 [details]
Patch
Comment on attachment 130250 [details] Patch Attachment 130250 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11833128 Created attachment 130478 [details]
Patch
Comment on attachment 130478 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130478&action=review Basically looks good. > Source/WebCore/page/Performance.cpp:76 > +PassRefPtr<PerformanceEntryList> Performance::webkitGetEntriesByType(const String& entryType) Could you omit the name of unused parameters? Some compiler configuration complains it. > Source/WebCore/page/Performance.cpp:82 > +PassRefPtr<PerformanceEntryList> Performance::webkitGetEntriesByName(const String& name, const String& entryType) Ditto for param name. Created attachment 132171 [details]
Patch
(In reply to comment #6) > (From update of attachment 130478 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=130478&action=review > > Basically looks good. > > > Source/WebCore/page/Performance.cpp:76 > > +PassRefPtr<PerformanceEntryList> Performance::webkitGetEntriesByType(const String& entryType) > > Could you omit the name of unused parameters? > Some compiler configuration complains it. Done. > > Source/WebCore/page/Performance.cpp:82 > > +PassRefPtr<PerformanceEntryList> Performance::webkitGetEntriesByName(const String& name, const String& entryType) > > Ditto for param name. Done. Thanks for taking a look. Tony, did you want to review this too? Comment on attachment 132171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132171&action=review The patch looks correct to me. Is the webkit-dev thread resolved now? > Source/WebCore/page/PerformanceEntryList.cpp:34 > +#if ENABLE(WEB_TIMING) Should we consider separating out the flags for NAVIGATION_TIMING and PERFORMANCE_TIMELINE? Created attachment 133173 [details]
Patch
(In reply to comment #10) > (From update of attachment 132171 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132171&action=review > > The patch looks correct to me. Is the webkit-dev thread resolved now? I've pinged it. It's somewhat separate from Resource Timing, but I mentioned it anyway. > > > Source/WebCore/page/PerformanceEntryList.cpp:34 > > +#if ENABLE(WEB_TIMING) > > Should we consider separating out the flags for NAVIGATION_TIMING and PERFORMANCE_TIMELINE? I've got 3 flags now: 1. WEB_TIMING: Enables window.performance and Navigation Timing. 2. PERFORMANCE_TIMELINE: Requires WEB_TIMING. Adds Performance Timeline. Inferred that Navigation Timing is included in the timeline, because WEB_TIMING is true. 3. RESOURCE_TIMING: Requires PERFORMANCE_TIMELINE. Adds Resource Timing to the timeline. Comment on attachment 133173 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133173&action=review Looks really good. With the webkit-dev thread satisfied and the spec at last call, I agree it is time to get this moving. > Source/WebCore/ChangeLog:14 > + Test: fast/dom/Window/window-properties-performance.html This doesn't really test it yet since it is disabled by default. Perhaps the log could be more clear about that and the fact that this is off-by-default. > Source/WebCore/page/PerformanceEntry.h:57 > + double m_duration; Is it worth making this completely immutable (all data members const)? (In reply to comment #13) > > Source/WebCore/page/PerformanceEntry.h:57 > > + double m_duration; > > Is it worth making this completely immutable (all data members const)? Yeah, sounds good. Created attachment 138887 [details]
Patch for landing
Comment on attachment 138887 [details] Patch for landing Clearing flags on attachment: 138887 Committed r115274: <http://trac.webkit.org/changeset/115274> All reviewed patches have been landed. Closing bug. |