[Webkit-unassigned] [Bug 90963] [User Timing] Implementation of User Timing W/O Getter Method
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Oct 15 17:23:06 PDT 2012
https://bugs.webkit.org/show_bug.cgi?id=90963
Tony Gentilcore <tonyg at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #156330|review? |review-
Flag| |
--- Comment #21 from Tony Gentilcore <tonyg at chromium.org> 2012-10-15 17:23:52 PST ---
(From update of attachment 156330)
View in context: https://bugs.webkit.org/attachment.cgi?id=156330&action=review
Just a few stylistic nits. This will need a rebase and then everything else looks good to me.
> Source/WebCore/page/Performance.cpp:194
> +
You can remove the extra line break here and on line 202 to be consistent with the above methods.
> Source/WebCore/page/Performance.h:114
> + mutable RefPtr<UserTiming> m_userTiming;
This doesn't need to be mutable as the methods that touch it aren't const.
> Source/WebCore/page/PerformanceUserTiming.cpp:26
> +
nit: there's an extra line break here.
> Source/WebCore/page/PerformanceUserTiming.cpp:37
> +namespace WebCore {
Please add a line break above namespace
> Source/WebCore/page/PerformanceUserTiming.cpp:44
> + static const struct RestrictiveField {
This should be named RestrictedField.
> Source/WebCore/page/PerformanceUserTiming.cpp:91
> + }
Indentation is off
> Source/WebCore/page/PerformanceUserTiming.h:62
> + static HashMap<String, NavigationTimingFunction> m_restrictiveKeyMap;
This should be named m_restrictedKeyMap, also I recommend moving it up a line and separating it by a line break from the ctor and the first non-static method.
> LayoutTests/ChangeLog:10
> + * http/tests/w3c/webperf/proposal/user-timing/test_user_timing_clearMarks-expected.txt: Added.
The directory name of these tests should be submission instead of proposal.
> LayoutTests/http/tests/w3c/webperf/proposal/user-timing/test_user_timing_clearMarks.html:28
> + var non_retained_entries = context.getEntriesByName(mark_names[i] ,'mark');
s/ ,/, /
> LayoutTests/http/tests/w3c/webperf/proposal/user-timing/test_user_timing_clearMarks.html:30
> + test_equals(non_retained_entries.length, 0,'Marks that we cleared should not exist anymore.');
Nit: space after comma
> LayoutTests/http/tests/w3c/webperf/proposal/user-timing/test_user_timing_clearMarks.html:45
> + var non_retained_entries = context.getEntriesByName(mark_names[i] ,'mark');
s/ ,/, /
> LayoutTests/http/tests/w3c/webperf/proposal/user-timing/test_user_timing_clearMarks.html:47
> + test_equals(non_retained_entries.length, 0,'Marks that we cleared should not exist anymore.');
Nit: space after comma
> LayoutTests/http/tests/w3c/webperf/proposal/user-timing/test_user_timing_clearMeasures.html:29
> + var non_retained_entries = context.getEntriesByName(measures[i][0] ,'measure');
s/ ,/, /
> LayoutTests/http/tests/w3c/webperf/resources/webperftestharnessextension.js:12
> + '',
indent only 4 spaces
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list