[webkit-reviews] review granted: [Bug 132794] REGRESSION (iOS WebKit2): requestAnimationFrame fires more than once between layer tree commits : [Attachment 231999] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri May 23 16:46:09 PDT 2014
Simon Fraser (smfr) <simon.fraser at apple.com> has granted Tim Horton
<thorton at apple.com>'s request for review:
Bug 132794: REGRESSION (iOS WebKit2): requestAnimationFrame fires more than
once between layer tree commits
https://bugs.webkit.org/show_bug.cgi?id=132794
Attachment 231999: patch
https://bugs.webkit.org/attachment.cgi?id=231999&action=review
------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=231999&action=review
> Source/WebCore/dom/ScriptedAnimationController.h:95
> + virtual DisplayRefreshMonitorFactory* displayRefreshMonitorFactory()
const override;
Do we really need a factory class, or can we just ask it to create a
DisplayRefreshMonitor?
> Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:42
> + if (factory)
> + return factory->createDisplayRefreshMonitor(displayID);
It's a bit weird for the create() method to just turn around and use the
factory passed in.
> Source/WebCore/platform/graphics/DisplayRefreshMonitorClient.cpp:2
> + * Copyright (C) 2010 Apple Inc. All rights reserved.
2014?
> Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp:63
> + if (!client->m_displayIDIsSet)
Weird to access internal state.
> Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp:71
> + if (!client->m_displayIDIsSet)
Ditto.
> Source/WebCore/platform/graphics/GraphicsLayerUpdater.cpp:86
> +DisplayRefreshMonitorFactory*
GraphicsLayerUpdater::displayRefreshMonitorFactory() const
> +{
> + return m_client ? m_client->displayRefreshMonitorFactory() : nullptr;
> +}
> +#endif
Maybe push the 82DisplayRefreshMonitor onto the GraphicsLayerUpdater?
> Source/WebCore/rendering/RenderLayerCompositor.cpp:3937
> +DisplayRefreshMonitorFactory*
RenderLayerCompositor::displayRefreshMonitorFactory() const
> +{
> + Frame& frame = m_renderView.frameView().frame();
> + Page* page = frame.page();
> + if (!page)
> + return nullptr;
> +
> + return page->chrome().client().displayRefreshMonitorFactory();
> +}
> +
Can this go away?
> Source/WebCore/rendering/RenderLayerCompositor.h:392
> + DisplayRefreshMonitorFactory* displayRefreshMonitorFactory() const;
Why does RLC have to be a factory?
More information about the webkit-reviews
mailing list