[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 #8 from James Simonsen <simonjam at chromium.org>  2012-07-12 14:59:07 PST ---
(From update of attachment 151906)
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.

> Source/WebCore/page/Performance.idl:44
> +#if defined(ENABLE_PERFORMANCE_TIMELINE2) && ENABLE_PERFORMANCE_TIMELINE

Remove that 2.

> 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?

> 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.

> 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(...)

> Source/WebCore/page/PerformanceUserTiming.h:47
> +typedef int ExceptionCode;

You can delete this now. It should come from ExceptionCode.h.

> 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.

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