.
Created attachment 201896 [details] Fix
Created attachment 201898 [details] Fix GTK build
Comment on attachment 201898 [details] Fix GTK build View in context: https://bugs.webkit.org/attachment.cgi?id=201898&action=review > Source/WebCore/dom/ScriptedAnimationController.cpp:60 > + , m_throttled(false) Typically we try to name boolean data members and such so they finish a phrase “controller <xxx>”. Thus we’d call this m_isThrottled. > Source/WebCore/dom/ScriptedAnimationController.cpp:91 > + m_throttled = isThrottled; > + if (m_animationTimer.isActive()) { This seems not quite right to me. If we were not throttled before and are still not throttled, it seems strange to stop and restart the timer. Maybe harmless, but strange nonetheless. We should have an early return when m_throttled is already == isThrottled. > Source/WebCore/dom/ScriptedAnimationController.cpp:197 > +#if USE(REQUEST_ANIMATION_FRAME_TIMER) && USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR) > + double animationInterval = m_throttled ? MinimumThrottledAnimationInterval : MinimumAnimationInterval; > +#else > + double animationInterval = MinimumAnimationInterval; > +#endif I’d write it like this: double animationInterval = MinimumAnimationInterval; #if USE(REQUEST_ANIMATION_FRAME_TIMER) && USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR) if (m_isThrottled) animationInterval = MinimumThrottledAnimationInterval; #endif I think easier to read than the repeated code and #else in the code you posted. > Source/WebCore/dom/ScriptedAnimationController.h:93 > bool m_useTimer; Seems like it should be named m_isUsingTimer or something like that.
Fixed in r150159.
Missed Darin's review comments – fixed in r150162.