[Webkit-unassigned] [Bug 90963] [User Timing] Implementation of User Timing W/O Getter Method
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Oct 16 08:18:21 PDT 2012
https://bugs.webkit.org/show_bug.cgi?id=90963
--- Comment #23 from pdeng6 <pan.deng at intel.com> 2012-10-16 08:19:09 PST ---
(In reply to comment #21)
> (From update of attachment 156330 [details])
> 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.
rebase done.
>
> > 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.
yep, done.
>
> > 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.
yep, done.
>
> > Source/WebCore/page/PerformanceUserTiming.cpp:26
> > +
>
> nit: there's an extra line break here.
done
>
> > Source/WebCore/page/PerformanceUserTiming.cpp:37
> > +namespace WebCore {
>
> Please add a line break above namespace
done
>
> > Source/WebCore/page/PerformanceUserTiming.cpp:44
> > + static const struct RestrictiveField {
>
> This should be named RestrictedField.
thanks, done
>
> > Source/WebCore/page/PerformanceUserTiming.cpp:91
> > + }
>
> Indentation is off
done
>
> > 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.
>
done.
> > 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.
>
yep, done
> > 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/ ,/, /
done
>
> > 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
done
>
> > 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/ ,/, /
done
>
> > 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.');
>
done
> 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/ ,/, /
done
>
> > LayoutTests/http/tests/w3c/webperf/resources/webperftestharnessextension.js:12
> > + '',
>
> indent only 4 spaces
thanks, done
--
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