https://bugs.webkit.org/show_bug.cgi?id=142776
<rdar://problem/
18921338>
Reviewed by Alexey Proskuryakov.
Test: fast/animation/request-animation-frame-unparented-iframe-crash.html
In some cases (like the new test), we can end up trying to start
requestAnimationFrame on a Document that has no Page. Most paths null-checked
the Page and did the right thing, but one failed to do so. In addition,
the current fallback (when Page is null) can result in us constructing
the wrong kind of DisplayRefreshMonitor, which could lead to trouble
down the road when it's reused. Instead, just completely avoid making a
DisplayRefreshMonitor in the null-page case.
* dom/ScriptedAnimationController.cpp:
(WebCore::ScriptedAnimationController::createDisplayRefreshMonitor):
If the page is null, bail.
* dom/ScriptedAnimationController.h:
* platform/graphics/DisplayRefreshMonitor.cpp:
(WebCore::DisplayRefreshMonitor::create):
Use Optional<> to make it easy to distinguish between ChromeClient
being unreachable (because we don't have a Page for some reason) and
ChromeClient declaring that it doesn't want to override the type of
DisplayRefreshMonitor that is created.
If ChromeClient was unreachable for some reason, we'll get back an engaged
nullptr and return it (instead of creating a DisplayRefreshMonitor based
on the platform). This avoids creating the wrong type of DisplayRefreshMonitor
in the rare case where we can't reach the ChromeClient (e.g. a freshly unparented
IFrame).
If instead the client returns a disengaged Nullopt, we'll interpret that as
"construct the default type", which falls back on the platform #ifdefs to
decide what to make.
* platform/graphics/DisplayRefreshMonitorManager.cpp:
(WebCore::DisplayRefreshMonitorManager::ensureMonitorForClient):
(WebCore::DisplayRefreshMonitorManager::scheduleAnimation):
Silently handle the case where we failed to make a DisplayRefreshMonitor.
* platform/graphics/DisplayRefreshMonitor.h:
* platform/graphics/DisplayRefreshMonitorClient.h:
* platform/graphics/GraphicsLayerUpdater.cpp:
(WebCore::GraphicsLayerUpdater::createDisplayRefreshMonitor):
* platform/graphics/GraphicsLayerUpdater.h:
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::createDisplayRefreshMonitor):
* rendering/RenderLayerCompositor.h:
Adjust to the new signature of createDisplayRefreshMonitor, and return
an engaged (nullptr) Optional if we can't get to ChromeClient for any reason.
* page/ChromeClient.h:
Return Nullopt (indicating a lack of override) by default.
* WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::createDisplayRefreshMonitor):
* WebProcess/WebCoreSupport/WebChromeClient.h:
Adjust to the new signature.
* fast/animation/request-animation-frame-unparented-iframe-crash-expected.txt: Added.
* fast/animation/request-animation-frame-unparented-iframe-crash.html: Added.
Add a test that ensures that calling requestAnimationFrame on a recently-unparented
frame doesn't crash.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@181656
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2015-03-17 Timothy Horton <timothy_horton@apple.com>
+
+ Reproducible null deref under ScriptedAnimationController::createDisplayRefreshMonitor
+ https://bugs.webkit.org/show_bug.cgi?id=142776
+ <rdar://problem/18921338>
+
+ Reviewed by Alexey Proskuryakov.
+
+ * fast/animation/request-animation-frame-unparented-iframe-crash-expected.txt: Added.
+ * fast/animation/request-animation-frame-unparented-iframe-crash.html: Added.
+ Add a test that ensures that calling requestAnimationFrame on a recently-unparented
+ frame doesn't crash.
+
2015-03-17 Dean Jackson <dino@apple.com>
Implement Scroll Container Animation Triggers
--- /dev/null
+This test passes if it does not crash.
--- /dev/null
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
+
+window.onload = function () {
+ var frame = document.getElementById("frame");
+ var frameWindow = frame.contentWindow;
+ var rAF = frameWindow.requestAnimationFrame;
+
+ frame.parentElement.removeChild(frame);
+ rAF.call(frameWindow, function () { });
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+};
+</script>
+<body>
+<iframe id="frame"></iframe>
+This test passes if it does not crash.
+</body>
+2015-03-17 Timothy Horton <timothy_horton@apple.com>
+
+ Reproducible null deref under ScriptedAnimationController::createDisplayRefreshMonitor
+ https://bugs.webkit.org/show_bug.cgi?id=142776
+ <rdar://problem/18921338>
+
+ Reviewed by Alexey Proskuryakov.
+
+ Test: fast/animation/request-animation-frame-unparented-iframe-crash.html
+
+ In some cases (like the new test), we can end up trying to start
+ requestAnimationFrame on a Document that has no Page. Most paths null-checked
+ the Page and did the right thing, but one failed to do so. In addition,
+ the current fallback (when Page is null) can result in us constructing
+ the wrong kind of DisplayRefreshMonitor, which could lead to trouble
+ down the road when it's reused. Instead, just completely avoid making a
+ DisplayRefreshMonitor in the null-page case.
+
+ * dom/ScriptedAnimationController.cpp:
+ (WebCore::ScriptedAnimationController::createDisplayRefreshMonitor):
+ If the page is null, bail.
+
+ * dom/ScriptedAnimationController.h:
+ * platform/graphics/DisplayRefreshMonitor.cpp:
+ (WebCore::DisplayRefreshMonitor::create):
+ Use Optional<> to make it easy to distinguish between ChromeClient
+ being unreachable (because we don't have a Page for some reason) and
+ ChromeClient declaring that it doesn't want to override the type of
+ DisplayRefreshMonitor that is created.
+
+ If ChromeClient was unreachable for some reason, we'll get back an engaged
+ nullptr and return it (instead of creating a DisplayRefreshMonitor based
+ on the platform). This avoids creating the wrong type of DisplayRefreshMonitor
+ in the rare case where we can't reach the ChromeClient (e.g. a freshly unparented
+ IFrame).
+
+ If instead the client returns a disengaged Nullopt, we'll interpret that as
+ "construct the default type", which falls back on the platform #ifdefs to
+ decide what to make.
+
+ * platform/graphics/DisplayRefreshMonitorManager.cpp:
+ (WebCore::DisplayRefreshMonitorManager::ensureMonitorForClient):
+ (WebCore::DisplayRefreshMonitorManager::scheduleAnimation):
+ Silently handle the case where we failed to make a DisplayRefreshMonitor.
+
+ * platform/graphics/DisplayRefreshMonitor.h:
+ * platform/graphics/DisplayRefreshMonitorClient.h:
+ * platform/graphics/GraphicsLayerUpdater.cpp:
+ (WebCore::GraphicsLayerUpdater::createDisplayRefreshMonitor):
+ * platform/graphics/GraphicsLayerUpdater.h:
+ * rendering/RenderLayerCompositor.cpp:
+ (WebCore::RenderLayerCompositor::createDisplayRefreshMonitor):
+ * rendering/RenderLayerCompositor.h:
+ Adjust to the new signature of createDisplayRefreshMonitor, and return
+ an engaged (nullptr) Optional if we can't get to ChromeClient for any reason.
+
+ * page/ChromeClient.h:
+ Return Nullopt (indicating a lack of override) by default.
+
2015-03-17 Dean Jackson <dino@apple.com>
Implement Scroll Container Animation Triggers
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
-PassRefPtr<DisplayRefreshMonitor> ScriptedAnimationController::createDisplayRefreshMonitor(PlatformDisplayID displayID) const
+Optional<RefPtr<DisplayRefreshMonitor>> ScriptedAnimationController::createDisplayRefreshMonitor(PlatformDisplayID displayID) const
{
- return m_document->page()->chrome().client().createDisplayRefreshMonitor(displayID);
+ if (!m_document->page())
+ return Optional<RefPtr<DisplayRefreshMonitor>>(nullptr);
+ return Optional<RefPtr<DisplayRefreshMonitor>>(m_document->page()->chrome().client().createDisplayRefreshMonitor(displayID));
}
#endif
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
// Override for DisplayRefreshMonitorClient
virtual void displayRefreshFired(double timestamp) override;
- virtual PassRefPtr<DisplayRefreshMonitor> createDisplayRefreshMonitor(PlatformDisplayID) const override;
+ virtual Optional<RefPtr<DisplayRefreshMonitor>> createDisplayRefreshMonitor(PlatformDisplayID) const override;
bool m_isUsingTimer;
bool m_isThrottled;
virtual GraphicsLayerFactory* graphicsLayerFactory() const { return nullptr; }
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
- virtual PassRefPtr<DisplayRefreshMonitor> createDisplayRefreshMonitor(PlatformDisplayID) const { return nullptr; }
+ virtual Optional<RefPtr<DisplayRefreshMonitor>> createDisplayRefreshMonitor(PlatformDisplayID) const { return Nullopt; }
#endif
// Pass 0 as the GraphicsLayer to detatch the root layer.
namespace WebCore {
-PassRefPtr<DisplayRefreshMonitor> DisplayRefreshMonitor::create(DisplayRefreshMonitorClient* client)
+RefPtr<DisplayRefreshMonitor> DisplayRefreshMonitor::create(DisplayRefreshMonitorClient* client)
{
PlatformDisplayID displayID = client->displayID();
- if (RefPtr<DisplayRefreshMonitor> monitor = client->createDisplayRefreshMonitor(displayID))
- return monitor.release();
+ if (Optional<RefPtr<DisplayRefreshMonitor>> monitor = client->createDisplayRefreshMonitor(displayID))
+ return monitor.value();
+
+ // If ChromeClient returned Nullopt, we'll go ahead and make one of the default type.
#if PLATFORM(MAC)
return DisplayRefreshMonitorMac::create(displayID);
class DisplayRefreshMonitor : public RefCounted<DisplayRefreshMonitor> {
public:
- static PassRefPtr<DisplayRefreshMonitor> create(DisplayRefreshMonitorClient*);
+ static RefPtr<DisplayRefreshMonitor> create(DisplayRefreshMonitorClient*);
WEBCORE_EXPORT virtual ~DisplayRefreshMonitor();
// Return true if callback request was scheduled, false if it couldn't be
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
#include "PlatformScreen.h"
+#include <wtf/Optional.h>
namespace WebCore {
// Always called on the main thread.
virtual void displayRefreshFired(double timestamp) = 0;
- virtual PassRefPtr<DisplayRefreshMonitor> createDisplayRefreshMonitor(PlatformDisplayID) const = 0;
+ // Returning nullopt indicates that WebCore should create whatever DisplayRefreshMonitor it deems
+ // most appropriate for the current platform. Returning nullptr indicates that we should not try to
+ // create a DisplayRefreshMonitor at all (and should instead fall back to using a timer).
+ virtual Optional<RefPtr<DisplayRefreshMonitor>> createDisplayRefreshMonitor(PlatformDisplayID) const = 0;
PlatformDisplayID displayID() const { return m_displayID; }
bool hasDisplayID() const { return m_displayIDIsSet; }
return manager.get();
}
-DisplayRefreshMonitor* DisplayRefreshMonitorManager::ensureMonitorForClient(DisplayRefreshMonitorClient* client)
+DisplayRefreshMonitor* DisplayRefreshMonitorManager::createMonitorForClient(DisplayRefreshMonitorClient* client)
{
PlatformDisplayID clientDisplayID = client->displayID();
for (const RefPtr<DisplayRefreshMonitor>& monitor : m_monitors) {
}
RefPtr<DisplayRefreshMonitor> monitor = DisplayRefreshMonitor::create(client);
+ if (!monitor)
+ return nullptr;
monitor->addClient(client);
DisplayRefreshMonitor* result = monitor.get();
m_monitors.append(monitor.release());
if (!client->hasDisplayID())
return;
- ensureMonitorForClient(client);
+ createMonitorForClient(client);
}
void DisplayRefreshMonitorManager::unregisterClient(DisplayRefreshMonitorClient* client)
if (!client->hasDisplayID())
return false;
- DisplayRefreshMonitor* monitor = ensureMonitorForClient(client);
+ DisplayRefreshMonitor* monitor = createMonitorForClient(client);
+ if (!monitor)
+ return false;
client->setIsScheduled(true);
return monitor->requestRefreshCallback();
DisplayRefreshMonitorManager() { }
virtual ~DisplayRefreshMonitorManager();
- DisplayRefreshMonitor* ensureMonitorForClient(DisplayRefreshMonitorClient*);
+ DisplayRefreshMonitor* createMonitorForClient(DisplayRefreshMonitorClient*);
Vector<RefPtr<DisplayRefreshMonitor>> m_monitors;
};
#endif
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
-PassRefPtr<DisplayRefreshMonitor> GraphicsLayerUpdater::createDisplayRefreshMonitor(PlatformDisplayID displayID) const
+Optional<RefPtr<DisplayRefreshMonitor>> GraphicsLayerUpdater::createDisplayRefreshMonitor(PlatformDisplayID displayID) const
{
- return m_client ? m_client->createDisplayRefreshMonitor(displayID) : nullptr;
+ if (!m_client)
+ return Optional<RefPtr<DisplayRefreshMonitor>>(nullptr);
+ return m_client->createDisplayRefreshMonitor(displayID);
}
#endif
virtual ~GraphicsLayerUpdaterClient() { }
virtual void flushLayersSoon(GraphicsLayerUpdater*) = 0;
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
- virtual PassRefPtr<DisplayRefreshMonitor> createDisplayRefreshMonitor(PlatformDisplayID) const = 0;
+ virtual Optional<RefPtr<DisplayRefreshMonitor>> createDisplayRefreshMonitor(PlatformDisplayID) const = 0;
#endif
};
void screenDidChange(PlatformDisplayID);
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
- virtual PassRefPtr<DisplayRefreshMonitor> createDisplayRefreshMonitor(PlatformDisplayID) const override;
+ virtual Optional<RefPtr<DisplayRefreshMonitor>> createDisplayRefreshMonitor(PlatformDisplayID) const override;
#endif
private:
}
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
-PassRefPtr<DisplayRefreshMonitor> RenderLayerCompositor::createDisplayRefreshMonitor(PlatformDisplayID displayID) const
+Optional<RefPtr<DisplayRefreshMonitor>> RenderLayerCompositor::createDisplayRefreshMonitor(PlatformDisplayID displayID) const
{
Frame& frame = m_renderView.frameView().frame();
Page* page = frame.page();
if (!page)
- return nullptr;
+ return Optional<RefPtr<DisplayRefreshMonitor>>(nullptr);
- return page->chrome().client().createDisplayRefreshMonitor(displayID);
+ return Optional<RefPtr<DisplayRefreshMonitor>>(page->chrome().client().createDisplayRefreshMonitor(displayID));
}
#endif
ScrollingCoordinator* scrollingCoordinator() const;
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
- PassRefPtr<DisplayRefreshMonitor> createDisplayRefreshMonitor(PlatformDisplayID) const override;
+ Optional<RefPtr<DisplayRefreshMonitor>> createDisplayRefreshMonitor(PlatformDisplayID) const override;
#endif
bool requiresCompositingForAnimation(RenderLayerModelObject&) const;
+2015-03-17 Timothy Horton <timothy_horton@apple.com>
+
+ Reproducible null deref under ScriptedAnimationController::createDisplayRefreshMonitor
+ https://bugs.webkit.org/show_bug.cgi?id=142776
+ <rdar://problem/18921338>
+
+ Reviewed by Alexey Proskuryakov.
+
+ * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+ (WebKit::WebChromeClient::createDisplayRefreshMonitor):
+ * WebProcess/WebCoreSupport/WebChromeClient.h:
+ Adjust to the new signature.
+
2015-03-17 Antti Koivisto <antti@apple.com>
Disk cache should support Vary: Cookie
}
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
-PassRefPtr<WebCore::DisplayRefreshMonitor> WebChromeClient::createDisplayRefreshMonitor(PlatformDisplayID displayID) const
+Optional<RefPtr<WebCore::DisplayRefreshMonitor>> WebChromeClient::createDisplayRefreshMonitor(PlatformDisplayID displayID) const
{
- return m_page->drawingArea()->createDisplayRefreshMonitor(displayID);
+ return Optional<RefPtr<WebCore::DisplayRefreshMonitor>>(m_page->drawingArea()->createDisplayRefreshMonitor(displayID));
}
#endif
virtual bool adjustLayerFlushThrottling(WebCore::LayerFlushThrottleState::Flags) override;
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
- virtual PassRefPtr<WebCore::DisplayRefreshMonitor> createDisplayRefreshMonitor(PlatformDisplayID) const override;
+ virtual Optional<RefPtr<WebCore::DisplayRefreshMonitor>> createDisplayRefreshMonitor(PlatformDisplayID) const override;
#endif
virtual CompositingTriggerFlags allowedCompositingTriggers() const override