Title: [279768] trunk/Source/WebCore
Revision
279768
Author
cdu...@apple.com
Date
2021-07-08 17:12:30 -0700 (Thu, 08 Jul 2021)

Log Message

Make sure SVG SMIL animations do not run in processes without any pages
https://bugs.webkit.org/show_bug.cgi?id=227720
<rdar://79625708>

Reviewed by Simon Fraser.

Make sure SVG SMIL animations do not run in processes without any pages.

We have seen traces / logging indicating that SVG SMIL animations may run in cached WebProcesses, thus
using CPU unnecessarily. Cached WebProcesses have no WebPage so this patch adds assertions to catch cases
where those animations run when there is no Page (and early returns in release to avoid doing unnecessary
work).

* page/Page.cpp:
(WebCore::Page::nonUtilityPageCount):
(WebCore::m_httpsUpgradeEnabled):
(WebCore::Page::~Page):
(WebCore::Page::isOnlyNonUtilityPage const):
* page/Page.h:
* svg/animation/SMILTimeContainer.cpp:
(WebCore::SMILTimeContainer::begin):
(WebCore::SMILTimeContainer::resume):
(WebCore::SMILTimeContainer::setElapsed):
(WebCore::SMILTimeContainer::startTimer):
(WebCore::SMILTimeContainer::updateAnimations):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (279767 => 279768)


--- trunk/Source/WebCore/ChangeLog	2021-07-09 00:11:20 UTC (rev 279767)
+++ trunk/Source/WebCore/ChangeLog	2021-07-09 00:12:30 UTC (rev 279768)
@@ -1,5 +1,33 @@
 2021-07-08  Chris Dumez  <cdu...@apple.com>
 
+        Make sure SVG SMIL animations do not run in processes without any pages
+        https://bugs.webkit.org/show_bug.cgi?id=227720
+        <rdar://79625708>
+
+        Reviewed by Simon Fraser.
+
+        Make sure SVG SMIL animations do not run in processes without any pages.
+
+        We have seen traces / logging indicating that SVG SMIL animations may run in cached WebProcesses, thus
+        using CPU unnecessarily. Cached WebProcesses have no WebPage so this patch adds assertions to catch cases
+        where those animations run when there is no Page (and early returns in release to avoid doing unnecessary
+        work).
+
+        * page/Page.cpp:
+        (WebCore::Page::nonUtilityPageCount):
+        (WebCore::m_httpsUpgradeEnabled):
+        (WebCore::Page::~Page):
+        (WebCore::Page::isOnlyNonUtilityPage const):
+        * page/Page.h:
+        * svg/animation/SMILTimeContainer.cpp:
+        (WebCore::SMILTimeContainer::begin):
+        (WebCore::SMILTimeContainer::resume):
+        (WebCore::SMILTimeContainer::setElapsed):
+        (WebCore::SMILTimeContainer::startTimer):
+        (WebCore::SMILTimeContainer::updateAnimations):
+
+2021-07-08  Chris Dumez  <cdu...@apple.com>
+
         [IndexedDB] KeyPath validity checks should happen on the cloned value, not the original one
         https://bugs.webkit.org/show_bug.cgi?id=227813
 

Modified: trunk/Source/WebCore/page/Page.cpp (279767 => 279768)


--- trunk/Source/WebCore/page/Page.cpp	2021-07-09 00:11:20 UTC (rev 279767)
+++ trunk/Source/WebCore/page/Page.cpp	2021-07-09 00:12:30 UTC (rev 279768)
@@ -189,7 +189,7 @@
     return set;
 }
 
-static unsigned nonUtilityPageCount { 0 };
+static unsigned gNonUtilityPageCount { 0 };
 
 static inline bool isUtilityPageChromeClient(ChromeClient& chromeClient)
 {
@@ -196,6 +196,11 @@
     return chromeClient.isEmptyChromeClient() || chromeClient.isSVGImageChromeClient();
 }
 
+unsigned Page::nonUtilityPageCount()
+{
+    return gNonUtilityPageCount;
+}
+
 DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, pageCounter, ("Page"));
 
 void Page::forEachPage(const WTF::Function<void(Page&)>& function)
@@ -334,8 +339,8 @@
     allPages().add(this);
 
     if (!isUtilityPage()) {
-        ++nonUtilityPageCount;
-        MemoryPressureHandler::setPageCount(nonUtilityPageCount);
+        ++gNonUtilityPageCount;
+        MemoryPressureHandler::setPageCount(gNonUtilityPageCount);
     }
 
 #ifndef NDEBUG
@@ -375,8 +380,8 @@
     setGroupName(String());
     allPages().remove(this);
     if (!isUtilityPage()) {
-        --nonUtilityPageCount;
-        MemoryPressureHandler::setPageCount(nonUtilityPageCount);
+        --gNonUtilityPageCount;
+        MemoryPressureHandler::setPageCount(gNonUtilityPageCount);
     }
     
     m_settings->pageDestroyed();
@@ -1294,7 +1299,7 @@
 
 bool Page::isOnlyNonUtilityPage() const
 {
-    return !isUtilityPage() && nonUtilityPageCount == 1;
+    return !isUtilityPage() && gNonUtilityPageCount == 1;
 }
 
 void Page::setLowPowerModeEnabledOverrideForTesting(std::optional<bool> isEnabled)

Modified: trunk/Source/WebCore/page/Page.h (279767 => 279768)


--- trunk/Source/WebCore/page/Page.h	2021-07-09 00:11:20 UTC (rev 279767)
+++ trunk/Source/WebCore/page/Page.h	2021-07-09 00:12:30 UTC (rev 279768)
@@ -282,6 +282,7 @@
     PageGroup& group();
 
     WEBCORE_EXPORT static void forEachPage(const WTF::Function<void(Page&)>&);
+    static unsigned nonUtilityPageCount();
 
     unsigned subframeCount() const;
 

Modified: trunk/Source/WebCore/svg/animation/SMILTimeContainer.cpp (279767 => 279768)


--- trunk/Source/WebCore/svg/animation/SMILTimeContainer.cpp	2021-07-09 00:11:20 UTC (rev 279767)
+++ trunk/Source/WebCore/svg/animation/SMILTimeContainer.cpp	2021-07-09 00:12:30 UTC (rev 279768)
@@ -114,6 +114,11 @@
 void SMILTimeContainer::begin()
 {
     ASSERT(!m_beginTime);
+
+    ASSERT(Page::nonUtilityPageCount());
+    if (!Page::nonUtilityPageCount())
+        return;
+
     MonotonicTime now = MonotonicTime::now();
 
     // If 'm_presetStartTime' is set, the timeline was modified via setElapsed() before the document began.
@@ -142,6 +147,9 @@
 void SMILTimeContainer::resume()
 {
     ASSERT(isPaused());
+    ASSERT(Page::nonUtilityPageCount());
+    if (!Page::nonUtilityPageCount())
+        return;
 
     m_resumeTime = MonotonicTime::now();
     m_pauseTime = MonotonicTime();
@@ -150,7 +158,11 @@
 
 void SMILTimeContainer::setElapsed(SMILTime time)
 {
-    // If the documment didn't begin yet, record a new start time, we'll seek to once its possible.
+    ASSERT(Page::nonUtilityPageCount());
+    if (!Page::nonUtilityPageCount())
+        return;
+
+    // If the document didn't begin yet, record a new start time, we'll seek to once its possible.
     if (!m_beginTime) {
         m_presetStartTime = Seconds(time.value());
         return;
@@ -183,6 +195,10 @@
     if (!fireTime.isFinite())
         return;
 
+    ASSERT(Page::nonUtilityPageCount());
+    if (!Page::nonUtilityPageCount())
+        return;
+
     SMILTime delay = std::max(fireTime - elapsed, minimumDelay);
     m_timer.startOneShot(1_s * delay.value());
 }
@@ -238,6 +254,10 @@
 
 void SMILTimeContainer::updateAnimations(SMILTime elapsed, bool seekToTime)
 {
+    ASSERT(Page::nonUtilityPageCount());
+    if (!Page::nonUtilityPageCount())
+        return;
+
     // Don't mutate the DOM while updating the animations.
     EventQueueScope scope;
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to