RESOLVED FIXED 80350
[Web Timing] Add a vendor-prefixed Performance Timeline API
https://bugs.webkit.org/show_bug.cgi?id=80350
Summary [Web Timing] Add a vendor-prefixed Performance Timeline API
James Simonsen
Reported 2012-03-05 16:47:49 PST
[Web Timing] Add a vendor-prefixed Performance Timeline API
Attachments
Patch (35.58 KB, patch)
2012-03-05 17:00 PST, James Simonsen
no flags
Patch (35.69 KB, patch)
2012-03-05 17:45 PST, James Simonsen
no flags
Patch (36.22 KB, patch)
2012-03-06 16:59 PST, James Simonsen
no flags
Patch (36.21 KB, patch)
2012-03-15 18:27 PDT, James Simonsen
no flags
Patch (35.19 KB, patch)
2012-03-21 20:00 PDT, James Simonsen
no flags
Patch for landing (35.17 KB, patch)
2012-04-25 16:02 PDT, James Simonsen
no flags
James Simonsen
Comment 1 2012-03-05 17:00:06 PST
Build Bot
Comment 2 2012-03-05 17:35:23 PST
James Simonsen
Comment 3 2012-03-05 17:45:54 PST
Early Warning System Bot
Comment 4 2012-03-05 19:54:09 PST
James Simonsen
Comment 5 2012-03-06 16:59:14 PST
Hajime Morrita
Comment 6 2012-03-15 04:42:43 PDT
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.
James Simonsen
Comment 7 2012-03-15 18:27:09 PDT
James Simonsen
Comment 8 2012-03-15 18:28:28 PDT
(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.
James Simonsen
Comment 9 2012-03-15 18:29:28 PDT
Thanks for taking a look. Tony, did you want to review this too?
Tony Gentilcore
Comment 10 2012-03-21 03:40:29 PDT
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?
James Simonsen
Comment 11 2012-03-21 20:00:05 PDT
James Simonsen
Comment 12 2012-03-21 20:52:29 PDT
(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.
Tony Gentilcore
Comment 13 2012-04-25 14:58:28 PDT
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)?
James Simonsen
Comment 14 2012-04-25 16:01:41 PDT
(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.
James Simonsen
Comment 15 2012-04-25 16:02:43 PDT
Created attachment 138887 [details] Patch for landing
WebKit Review Bot
Comment 16 2012-04-25 18:16:59 PDT
Comment on attachment 138887 [details] Patch for landing Clearing flags on attachment: 138887 Committed r115274: <http://trac.webkit.org/changeset/115274>
WebKit Review Bot
Comment 17 2012-04-25 18:17:06 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.