[webkit-reviews] review granted: [Bug 111244] Animations fail to start on http://www.google.com/insidesearch/howsearchworks/thestory/ : [Attachment 200004] Patch

[bugzilla-daemon at webkit.org]

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has granted Zalan Bujtas
<zalan at apple.com>'s request for review:
Bug 111244: Animations fail to start on
http://www.google.com/insidesearch/howsearchworks/thestory/
https://bugs.webkit.org/show_bug.cgi?id=111244

Attachment 200004: Patch
https://bugs.webkit.org/attachment.cgi?id=200004&action=review

------- Additional Comments from David Kilzer (:ddkilzer) <ddkilzer at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=200004&action=review


r=me, but consider the feedback.

> Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:183
> +ENABLE_WEB_TIMING_MINIMAL = ENABLE_WEB_TIMING_MINIMAL;
>  ENABLE_WEB_TIMING = ;

Nit:  I would put ENABLE_WEB_TIMING_MINIMAL after ENABLE_WEB_TIMING (to match
FEATURE_DEFINES below).  If you change this, please remember to make the same
change to all FeatureDefines.xcconfig files.

> Source/WTF/wtf/FeatureDefines.h:825
> +#if !defined(ENABLE_WEB_TIMING_MINIMAL)
> +#define ENABLE_WEB_TIMING_MINIMAL 0
> +#endif
> +
>  #if !defined(ENABLE_WEB_TIMING)
>  #define ENABLE_WEB_TIMING 0
>  #endif

Should we add an #error if both WEB_TIMING and WEB_TIMING_MINIMAL are enabled?

Nit:  I would reverse the order of these as well.