Diff
Modified: trunk/Source/WebCore/ChangeLog (275301 => 275302)
--- trunk/Source/WebCore/ChangeLog 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebCore/ChangeLog 2021-03-31 20:13:05 UTC (rev 275302)
@@ -1,3 +1,57 @@
+2021-03-30 Simon Fraser <simon.fra...@apple.com>
+
+ Assertions in DisplayUpdate when dragging WebView windows between screens with different refresh rates
+ https://bugs.webkit.org/show_bug.cgi?id=223984
+
+ Reviewed by Sam Weinig.
+
+ There are various reasons why rigorous assertions in DisplayUpdate::relevantForUpdateFrequency()
+ are ill advised.
+
+ When WebKitLegacy calls windowScreenDidChange() it does not pass in a nominal display
+ refresh rate, so we assume 60fps. If the screen actually has a non-60fps refresh rate, then
+ we can get mixed 60/50 state in various places; this change fixes one of them, but such bugs
+ have to not result in assertions or divide by zero crashes in
+ DisplayUpdate::relevantForUpdateFrequency().
+
+ The second reason is webkit.org/b/212120; we may start DisplayRefreshMonitors with DisplayID
+ 0 before we know what screen we're on, again risking a situation where the actual screen has
+ a non-60fps refresh rate.
+
+ To fix the case where no nominal refresh rate is passed through
+ Chrome::windowScreenDidChange(), allow a DisplayRefreshMonitor to supply a rate; often, a
+ DisplayRefreshMonitor can compute one from its knowledge of the display. This requires
+ DisplayRefreshMonitorManager is able to create a DisplayRefreshMonitor with no client, so
+ support that with a little refactoring.
+
+ Have some DisplayRefreshMonitor implementations supply their nominal display refresh rate
+ when known.
+
+ * page/Chrome.cpp:
+ (WebCore::Chrome::windowScreenDidChange): No need to early return here; Page does the same check.
+ * page/Page.cpp:
+ (WebCore::Page::windowScreenDidChange): Push the call to adjustRenderingUpdateFrequency() into
+ renderingUpdateScheduler().windowScreenDidChange() as suggested in a previous review.
+ * page/RenderingUpdateScheduler.cpp:
+ (WebCore::RenderingUpdateScheduler::windowScreenDidChange):
+ * platform/graphics/DisplayRefreshMonitor.h:
+ (WebCore::DisplayRefreshMonitor::displayNominalFramesPerSecond):
+ * platform/graphics/DisplayRefreshMonitorManager.cpp:
+ (WebCore::DisplayRefreshMonitorManager::ensureMonitorForDisplayID):
+ (WebCore::DisplayRefreshMonitorManager::nominalFramesPerSecondForDisplay):
+ (WebCore::DisplayRefreshMonitorManager::monitorForClient):
+ * platform/graphics/DisplayRefreshMonitorManager.h:
+ * platform/graphics/DisplayUpdate.cpp:
+ (WebCore::DisplayUpdate::relevantForUpdateFrequency const):
+ * platform/graphics/ios/DisplayRefreshMonitorIOS.h:
+ * platform/graphics/ios/DisplayRefreshMonitorIOS.mm:
+ (WebCore::DisplayRefreshMonitorIOS::displayNominalFramesPerSecond):
+ * platform/graphics/mac/LegacyDisplayRefreshMonitorMac.cpp:
+ (WebCore::LegacyDisplayRefreshMonitorMac::ensureDisplayLink):
+ (WebCore::LegacyDisplayRefreshMonitorMac::startNotificationMechanism):
+ (WebCore::LegacyDisplayRefreshMonitorMac::displayNominalFramesPerSecond):
+ * platform/graphics/mac/LegacyDisplayRefreshMonitorMac.h:
+
2021-03-31 Sihui Liu <sihui_...@apple.com>
Add logging in IndexedDB to help debug flaky quota tests
Modified: trunk/Source/WebCore/page/Chrome.cpp (275301 => 275302)
--- trunk/Source/WebCore/page/Chrome.cpp 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebCore/page/Chrome.cpp 2021-03-31 20:13:05 UTC (rev 275302)
@@ -546,11 +546,8 @@
return m_page.displayID();
}
-void Chrome::windowScreenDidChange(PlatformDisplayID displayID, Optional<unsigned> nominalFrameInterval)
+void Chrome::windowScreenDidChange(PlatformDisplayID displayID, Optional<FramesPerSecond> nominalFrameInterval)
{
- if (displayID == m_page.displayID() && nominalFrameInterval == m_page.displayNominalFramesPerSecond())
- return;
-
m_page.windowScreenDidChange(displayID, nominalFrameInterval);
}
Modified: trunk/Source/WebCore/page/Page.cpp (275301 => 275302)
--- trunk/Source/WebCore/page/Page.cpp 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebCore/page/Page.cpp 2021-03-31 20:13:05 UTC (rev 275302)
@@ -45,6 +45,7 @@
#include "DebugPageOverlays.h"
#include "DiagnosticLoggingClient.h"
#include "DiagnosticLoggingKeys.h"
+#include "DisplayRefreshMonitorManager.h"
#include "DocumentLoader.h"
#include "DocumentMarkerController.h"
#include "DocumentTimeline.h"
@@ -1181,6 +1182,11 @@
m_displayID = displayID;
m_displayNominalFramesPerSecond = nominalFramesPerSecond;
+ if (!m_displayNominalFramesPerSecond) {
+ // If the caller didn't give us a refresh rate, maybe the relevant DisplayRefreshMonitor can? This happens in WebKitLegacy
+ // because WebView doesn't have a convenient way to access the display refresh rate.
+ m_displayNominalFramesPerSecond = DisplayRefreshMonitorManager::sharedManager().nominalFramesPerSecondForDisplay(m_displayID, chrome().client().displayRefreshMonitorFactory());
+ }
for (Frame* frame = &mainFrame(); frame; frame = frame->tree().traverseNext()) {
if (frame->document())
@@ -1195,10 +1201,9 @@
#endif
if (m_scrollingCoordinator)
- m_scrollingCoordinator->windowScreenDidChange(displayID, nominalFramesPerSecond);
+ m_scrollingCoordinator->windowScreenDidChange(displayID, m_displayNominalFramesPerSecond);
renderingUpdateScheduler().windowScreenDidChange(displayID);
- renderingUpdateScheduler().adjustRenderingUpdateFrequency();
setNeedsRecalcStyleInAllFrames();
}
Modified: trunk/Source/WebCore/page/RenderingUpdateScheduler.cpp (275301 => 275302)
--- trunk/Source/WebCore/page/RenderingUpdateScheduler.cpp 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebCore/page/RenderingUpdateScheduler.cpp 2021-03-31 20:13:05 UTC (rev 275302)
@@ -115,6 +115,7 @@
void RenderingUpdateScheduler::windowScreenDidChange(PlatformDisplayID displayID)
{
+ adjustRenderingUpdateFrequency();
DisplayRefreshMonitorManager::sharedManager().windowScreenDidChange(displayID, *this);
}
Modified: trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitor.h (275301 => 275302)
--- trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitor.h 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitor.h 2021-03-31 20:13:05 UTC (rev 275302)
@@ -58,7 +58,9 @@
bool hasClients() const { return m_clients.size(); }
void addClient(DisplayRefreshMonitorClient&);
bool removeClient(DisplayRefreshMonitorClient&);
-
+
+ virtual Optional<FramesPerSecond> displayNominalFramesPerSecond() { return WTF::nullopt; }
+
PlatformDisplayID displayID() const { return m_displayID; }
static RefPtr<DisplayRefreshMonitor> createDefaultDisplayRefreshMonitor(PlatformDisplayID);
Modified: trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp (275301 => 275302)
--- trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp 2021-03-31 20:13:05 UTC (rev 275302)
@@ -41,23 +41,16 @@
return manager.get();
}
-DisplayRefreshMonitor* DisplayRefreshMonitorManager::monitorForClient(DisplayRefreshMonitorClient& client)
+DisplayRefreshMonitor* DisplayRefreshMonitorManager::ensureMonitorForDisplayID(PlatformDisplayID displayID, DisplayRefreshMonitorFactory* factory)
{
- if (!client.hasDisplayID())
- return nullptr;
-
- PlatformDisplayID clientDisplayID = client.displayID();
- if (auto* existingMonitor = monitorForDisplayID(clientDisplayID)) {
- existingMonitor->addClient(client);
+ if (auto* existingMonitor = monitorForDisplayID(displayID))
return existingMonitor;
- }
- auto monitor = DisplayRefreshMonitor::create(client.displayRefreshMonitorFactory(), clientDisplayID);
+ auto monitor = DisplayRefreshMonitor::create(factory, displayID);
if (!monitor)
return nullptr;
- LOG_WITH_STREAM(DisplayLink, stream << "DisplayRefreshMonitorManager::monitorForClient() - created monitor " << monitor.get() << " for display " << clientDisplayID);
- monitor->addClient(client);
+ LOG_WITH_STREAM(DisplayLink, stream << "DisplayRefreshMonitorManager::ensureMonitorForDisplayID() - created monitor " << monitor.get() << " for display " << displayID);
DisplayRefreshMonitor* result = monitor.get();
m_monitors.append({ WTFMove(monitor) });
return result;
@@ -108,6 +101,15 @@
scheduleAnimation(client);
}
+Optional<FramesPerSecond> DisplayRefreshMonitorManager::nominalFramesPerSecondForDisplay(PlatformDisplayID displayID, DisplayRefreshMonitorFactory* factory)
+{
+ auto* monitor = ensureMonitorForDisplayID(displayID, factory);
+ if (monitor)
+ monitor->displayNominalFramesPerSecond();
+
+ return WTF::nullopt;
+}
+
void DisplayRefreshMonitorManager::displayWasUpdated(PlatformDisplayID displayID, const DisplayUpdate& displayUpdate)
{
auto* monitor = monitorForDisplayID(displayID);
@@ -122,6 +124,18 @@
});
}
+DisplayRefreshMonitor* DisplayRefreshMonitorManager::monitorForClient(DisplayRefreshMonitorClient& client)
+{
+ if (!client.hasDisplayID())
+ return nullptr;
+
+ auto* monitor = ensureMonitorForDisplayID(client.displayID(), client.displayRefreshMonitorFactory());
+ if (monitor)
+ monitor->addClient(client);
+
+ return monitor;
+}
+
DisplayRefreshMonitor* DisplayRefreshMonitorManager::monitorForDisplayID(PlatformDisplayID displayID) const
{
auto index = findMonitorForDisplayID(displayID);
Modified: trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h (275301 => 275302)
--- trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h 2021-03-31 20:13:05 UTC (rev 275302)
@@ -47,6 +47,8 @@
void setPreferredFramesPerSecond(DisplayRefreshMonitorClient&, FramesPerSecond);
bool scheduleAnimation(DisplayRefreshMonitorClient&);
void windowScreenDidChange(PlatformDisplayID, DisplayRefreshMonitorClient&);
+
+ Optional<FramesPerSecond> nominalFramesPerSecondForDisplay(PlatformDisplayID, DisplayRefreshMonitorFactory*);
WEBCORE_EXPORT void displayWasUpdated(PlatformDisplayID, const DisplayUpdate&);
@@ -60,6 +62,8 @@
DisplayRefreshMonitor* monitorForDisplayID(PlatformDisplayID) const;
DisplayRefreshMonitor* monitorForClient(DisplayRefreshMonitorClient&);
+ DisplayRefreshMonitor* ensureMonitorForDisplayID(PlatformDisplayID, DisplayRefreshMonitorFactory*);
+
struct DisplayRefreshMonitorWrapper {
DisplayRefreshMonitorWrapper(DisplayRefreshMonitorWrapper&&) = default;
~DisplayRefreshMonitorWrapper()
Modified: trunk/Source/WebCore/platform/graphics/DisplayUpdate.cpp (275301 => 275302)
--- trunk/Source/WebCore/platform/graphics/DisplayUpdate.cpp 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebCore/platform/graphics/DisplayUpdate.cpp 2021-03-31 20:13:05 UTC (rev 275302)
@@ -34,9 +34,12 @@
bool DisplayUpdate::relevantForUpdateFrequency(FramesPerSecond preferredFramesPerSecond) const
{
- ASSERT(WTF::isIntegral(static_cast<float>(updatesPerSecond) / preferredFramesPerSecond));
+ // FIXME: Ideally this would be an assertion, but that may fire when windows move between screens with different refresh rates, because of webkit.org/b/212120.
+ if (!WTF::isIntegral(static_cast<float>(updatesPerSecond) / preferredFramesPerSecond || !preferredFramesPerSecond))
+ return true;
+
unsigned rateFactor = updatesPerSecond / preferredFramesPerSecond;
- return !(updateIndex % rateFactor);
+ return !rateFactor || !(updateIndex % rateFactor);
}
TextStream& operator<<(TextStream& ts, const DisplayUpdate& update)
Modified: trunk/Source/WebCore/platform/graphics/ios/DisplayRefreshMonitorIOS.h (275301 => 275302)
--- trunk/Source/WebCore/platform/graphics/ios/DisplayRefreshMonitorIOS.h 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebCore/platform/graphics/ios/DisplayRefreshMonitorIOS.h 2021-03-31 20:13:05 UTC (rev 275302)
@@ -51,6 +51,7 @@
void stop() final;
bool startNotificationMechanism() final;
void stopNotificationMechanism() final;
+ Optional<FramesPerSecond> displayNominalFramesPerSecond() final;
RetainPtr<WebDisplayLinkHandler> m_handler;
DisplayUpdate m_currentUpdate;
Modified: trunk/Source/WebCore/platform/graphics/ios/DisplayRefreshMonitorIOS.mm (275301 => 275302)
--- trunk/Source/WebCore/platform/graphics/ios/DisplayRefreshMonitorIOS.mm 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebCore/platform/graphics/ios/DisplayRefreshMonitorIOS.mm 2021-03-31 20:13:05 UTC (rev 275302)
@@ -149,6 +149,11 @@
m_displayLinkIsActive = false;
}
+Optional<FramesPerSecond> DisplayRefreshMonitorIOS::displayNominalFramesPerSecond()
+{
+ return DisplayLinkFramesPerSecond;
+}
+
} // namespace WebCore
#endif // PLATFORM(IOS_FAMILY)
Modified: trunk/Source/WebCore/platform/graphics/mac/LegacyDisplayRefreshMonitorMac.cpp (275301 => 275302)
--- trunk/Source/WebCore/platform/graphics/mac/LegacyDisplayRefreshMonitorMac.cpp 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebCore/platform/graphics/mac/LegacyDisplayRefreshMonitorMac.cpp 2021-03-31 20:13:05 UTC (rev 275302)
@@ -85,18 +85,27 @@
return round((double)refreshPeriod.timeScale / (double)refreshPeriod.timeValue);
}
+bool LegacyDisplayRefreshMonitorMac::ensureDisplayLink()
+{
+ if (m_displayLink)
+ return true;
+
+ auto error = CVDisplayLinkCreateWithCGDisplay(displayID(), &m_displayLink);
+ if (error)
+ return false;
+
+ error = CVDisplayLinkSetOutputCallback(m_displayLink, displayLinkCallback, this);
+ if (error)
+ return false;
+
+ return true;
+}
+
bool LegacyDisplayRefreshMonitorMac::startNotificationMechanism()
{
if (!m_displayLink) {
- auto error = CVDisplayLinkCreateWithCGDisplay(displayID(), &m_displayLink);
- if (error)
+ if (!ensureDisplayLink())
return false;
-
- error = CVDisplayLinkSetOutputCallback(m_displayLink, displayLinkCallback, this);
- if (error)
- return false;
-
- m_currentUpdate = { 0, nominalFramesPerSecondFromDisplayLink(m_displayLink) };
}
if (!m_displayLinkIsActive) {
@@ -107,7 +116,7 @@
return false;
m_displayLinkIsActive = true;
- m_currentUpdate.updateIndex = 0;
+ m_currentUpdate = { 0, nominalFramesPerSecondFromDisplayLink(m_displayLink) };
}
return true;
@@ -126,6 +135,14 @@
m_displayLinkIsActive = false;
}
+Optional<FramesPerSecond> LegacyDisplayRefreshMonitorMac::displayNominalFramesPerSecond()
+{
+ if (!ensureDisplayLink())
+ return WTF::nullopt;
+
+ return nominalFramesPerSecondFromDisplayLink(m_displayLink);
+}
+
} // namespace WebCore
#endif // PLATFORM(MAC)
Modified: trunk/Source/WebCore/platform/graphics/mac/LegacyDisplayRefreshMonitorMac.h (275301 => 275302)
--- trunk/Source/WebCore/platform/graphics/mac/LegacyDisplayRefreshMonitorMac.h 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebCore/platform/graphics/mac/LegacyDisplayRefreshMonitorMac.h 2021-03-31 20:13:05 UTC (rev 275302)
@@ -54,6 +54,9 @@
bool startNotificationMechanism() final;
void stopNotificationMechanism() final;
+ Optional<FramesPerSecond> displayNominalFramesPerSecond() final;
+
+ bool ensureDisplayLink();
static FramesPerSecond nominalFramesPerSecondFromDisplayLink(CVDisplayLinkRef);
Modified: trunk/Source/WebKit/ChangeLog (275301 => 275302)
--- trunk/Source/WebKit/ChangeLog 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebKit/ChangeLog 2021-03-31 20:13:05 UTC (rev 275302)
@@ -1,3 +1,17 @@
+2021-03-30 Simon Fraser <simon.fra...@apple.com>
+
+ Assertions in DisplayUpdate when dragging WebView windows between screens with different refresh rates
+ https://bugs.webkit.org/show_bug.cgi?id=223984
+
+ Reviewed by Sam Weinig.
+
+ RemoteLayerTreeDisplayRefreshMonitor knows its m_preferredFramesPerSecond so can
+ return that.
+
+ * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDisplayRefreshMonitor.h:
+ * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDisplayRefreshMonitor.mm:
+ (WebKit::RemoteLayerTreeDisplayRefreshMonitor::displayNominalFramesPerSecond):
+
2021-03-31 Sihui Liu <sihui_...@apple.com>
Add logging in IndexedDB to help debug flaky quota tests
Modified: trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDisplayRefreshMonitor.h (275301 => 275302)
--- trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDisplayRefreshMonitor.h 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDisplayRefreshMonitor.h 2021-03-31 20:13:05 UTC (rev 275302)
@@ -51,6 +51,7 @@
bool startNotificationMechanism() final { return true; }
void stopNotificationMechanism() final { }
+ Optional<WebCore::FramesPerSecond> displayNominalFramesPerSecond() final;
WeakPtr<RemoteLayerTreeDrawingArea> m_drawingArea;
Modified: trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDisplayRefreshMonitor.mm (275301 => 275302)
--- trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDisplayRefreshMonitor.mm 2021-03-31 19:34:51 UTC (rev 275301)
+++ trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDisplayRefreshMonitor.mm 2021-03-31 20:13:05 UTC (rev 275302)
@@ -91,4 +91,9 @@
m_drawingArea = makeWeakPtr(drawingArea);
}
+Optional<FramesPerSecond> RemoteLayerTreeDisplayRefreshMonitor::displayNominalFramesPerSecond()
+{
+ return m_preferredFramesPerSecond;
+}
+
} // namespace WebKit
