[Webkit-unassigned] [Bug 90963] [User Timing] Implementation of User Timing W/O Getter Method

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=90963





--- Comment #11 from pdeng6 <pan.deng at intel.com>  2012-07-12 18:32:46 PST ---
(In reply to comment #8)
> (From update of attachment 151906 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=151906&action=review
> 
> Thanks for the changes. Just a couple more small changes and this should be ready to commit.
> 
> > Source/WebCore/page/Performance.cpp:195
> > +        m_timing = PerformanceTiming::create(m_frame);
> 
> I don't think this is necessary any more. When you call m_performance->timing() in PerformanceUserTiming, it'll be constructed automatically.

Yep, done.
> 
> > Source/WebCore/page/Performance.idl:44
> > +#if defined(ENABLE_PERFORMANCE_TIMELINE2) && ENABLE_PERFORMANCE_TIMELINE
> 
> Remove that 2.
> 
thanks for catch this, removed.
> > Source/WebCore/page/PerformanceUserTiming.cpp:44
> > +    static const struct restrictive_field {
> 
> I think structs are supposed to be CamelCase.
> 
> > Source/WebCore/page/PerformanceUserTiming.cpp:46
> > +      functionPointer navFunctionPointer;
> 
> In general, abbreviations are discouraged in WebKit. Maybe use navigationTimingFunction?
yes, replaced.
> 
> > Source/WebCore/page/PerformanceUserTiming.cpp:71
> > +    for (const struct restrictive_field* restriction = default_restrictions;
> 
> You should only do this if m_restrictiveKeyMap is empty. We only need to do it once now.
> 
Done
> > Source/WebCore/page/PerformanceUserTiming.cpp:140
> > +        return insertPerformanceEntry(m_measuresMap, PerformanceMeasure::create(measureName, startTime, endTime - startTime));
> 
> How about we just call insertPerformanceEntry once at the end of the function? So the code would look something like:
> 
> if (startMark.isNull()) {
>   endTime = ...
> } else if (endMark.isNull()) {
>   startTime = ...
>   endTime = ...
> } else {
>   startTime = ...
>   endTime = ...
> }
> insertPerformanceEntry(...)
> 
yep, looks better.

> > Source/WebCore/page/PerformanceUserTiming.h:47
> > +typedef int ExceptionCode;
> 
> You can delete this now. It should come from ExceptionCode.h.
> 
removed
> > Source/WebCore/page/PerformanceUserTiming.h:48
> > +typedef unsigned long long (PerformanceTiming::*functionPointer)() const;
> 
> Maybe call this NavigationTimingFunction? I think types are supposed to start upper case.
Yep, thanks.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.