[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