<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[242580] trunk/Source/WebKit</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/242580">242580</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2019-03-06 16:43:24 -0800 (Wed, 06 Mar 2019)</dd>
</dl>

<h3>Log Message</h3>
<pre>[iOS] ProcessDidResume() IPC should be sent upon resuming when ProcessWillSuspendImminently() IPC was sent
https://bugs.webkit.org/show_bug.cgi?id=195382
<rdar://problem/48642739>

Reviewed by Geoffrey Garen.

ProcessDidResume() IPC should be sent upon resuming when ProcessWillSuspendImminently() IPC was sent. Previously,
we only send ProcessDidResume() to the WebProcess if PrepareToSuspend() was sent, not when the more urgent
ProcessWillSuspendImminently() IPC was sent.

This mismatch has lead to bugs because the WebProcess does not know it got resumed and failed to unfreeze the
layers it froze upon suspension.

* UIProcess/ProcessAssertion.h:
(WebKit::ProcessAssertionClient::~ProcessAssertionClient):

* UIProcess/ProcessThrottler.cpp:
(WebKit::ProcessThrottler::updateAssertion):
(WebKit::ProcessThrottler::uiAssertionWillExpireImminently):
(WebKit::ProcessThrottler::assertionWillExpireImminently): Deleted.

* UIProcess/ProcessThrottler.h:
* UIProcess/ios/ProcessAssertionIOS.mm:
(-[WKProcessAssertionBackgroundTaskManager _notifyClientsOfImminentSuspension]):
(WebKit::ProcessAssertion::~ProcessAssertion):
(WebKit::ProcessAndUIAssertion::~ProcessAndUIAssertion):
(WebKit::ProcessAndUIAssertion::setClient):

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::didCompletePageTransition):
Revert change that was landed in <a href="http://trac.webkit.org/projects/webkit/changeset/242098">r242098</a> to work around this ProcessThrottler bug.

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::applicationWillEnterForeground):
Revert change that was landed in <a href="http://trac.webkit.org/projects/webkit/changeset/242554">r242554</a> to work around this ProcessThrottler bug.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKitChangeLog">trunk/Source/WebKit/ChangeLog</a></li>
<li><a href="#trunkSourceWebKitUIProcessProcessAssertionh">trunk/Source/WebKit/UIProcess/ProcessAssertion.h</a></li>
<li><a href="#trunkSourceWebKitUIProcessProcessThrottlercpp">trunk/Source/WebKit/UIProcess/ProcessThrottler.cpp</a></li>
<li><a href="#trunkSourceWebKitUIProcessProcessThrottlerh">trunk/Source/WebKit/UIProcess/ProcessThrottler.h</a></li>
<li><a href="#trunkSourceWebKitUIProcessiosProcessAssertionIOSmm">trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm</a></li>
<li><a href="#trunkSourceWebKitWebProcessWebPageWebPagecpp">trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp</a></li>
<li><a href="#trunkSourceWebKitWebProcessWebPageiosWebPageIOSmm">trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKitChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/ChangeLog (242579 => 242580)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/ChangeLog    2019-03-07 00:38:01 UTC (rev 242579)
+++ trunk/Source/WebKit/ChangeLog       2019-03-07 00:43:24 UTC (rev 242580)
</span><span class="lines">@@ -1,3 +1,41 @@
</span><ins>+2019-03-06  Chris Dumez  <cdumez@apple.com>
+
+        [iOS] ProcessDidResume() IPC should be sent upon resuming when ProcessWillSuspendImminently() IPC was sent
+        https://bugs.webkit.org/show_bug.cgi?id=195382
+        <rdar://problem/48642739>
+
+        Reviewed by Geoffrey Garen.
+
+        ProcessDidResume() IPC should be sent upon resuming when ProcessWillSuspendImminently() IPC was sent. Previously,
+        we only send ProcessDidResume() to the WebProcess if PrepareToSuspend() was sent, not when the more urgent
+        ProcessWillSuspendImminently() IPC was sent.
+
+        This mismatch has lead to bugs because the WebProcess does not know it got resumed and failed to unfreeze the
+        layers it froze upon suspension.
+
+        * UIProcess/ProcessAssertion.h:
+        (WebKit::ProcessAssertionClient::~ProcessAssertionClient):
+
+        * UIProcess/ProcessThrottler.cpp:
+        (WebKit::ProcessThrottler::updateAssertion):
+        (WebKit::ProcessThrottler::uiAssertionWillExpireImminently):
+        (WebKit::ProcessThrottler::assertionWillExpireImminently): Deleted.
+
+        * UIProcess/ProcessThrottler.h:
+        * UIProcess/ios/ProcessAssertionIOS.mm:
+        (-[WKProcessAssertionBackgroundTaskManager _notifyClientsOfImminentSuspension]):
+        (WebKit::ProcessAssertion::~ProcessAssertion):
+        (WebKit::ProcessAndUIAssertion::~ProcessAndUIAssertion):
+        (WebKit::ProcessAndUIAssertion::setClient):
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::didCompletePageTransition):
+        Revert change that was landed in r242098 to work around this ProcessThrottler bug.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::applicationWillEnterForeground):
+        Revert change that was landed in r242554 to work around this ProcessThrottler bug.
+
</ins><span class="cx"> 2019-03-06  Alex Christensen  <achristensen@webkit.org>
</span><span class="cx"> 
</span><span class="cx">         Add gettid to allowed syscalls on Mac
</span></span></pre></div>
<a id="trunkSourceWebKitUIProcessProcessAssertionh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/UIProcess/ProcessAssertion.h (242579 => 242580)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/UIProcess/ProcessAssertion.h 2019-03-07 00:38:01 UTC (rev 242579)
+++ trunk/Source/WebKit/UIProcess/ProcessAssertion.h    2019-03-07 00:43:24 UTC (rev 242580)
</span><span class="lines">@@ -51,8 +51,8 @@
</span><span class="cx"> 
</span><span class="cx"> class ProcessAssertionClient {
</span><span class="cx"> public:
</span><del>-    virtual ~ProcessAssertionClient() { };
-    virtual void assertionWillExpireImminently() = 0;
</del><ins>+    virtual ~ProcessAssertionClient() { }
+    virtual void uiAssertionWillExpireImminently() = 0;
</ins><span class="cx"> };
</span><span class="cx"> 
</span><span class="cx"> class ProcessAssertion
</span></span></pre></div>
<a id="trunkSourceWebKitUIProcessProcessThrottlercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/UIProcess/ProcessThrottler.cpp (242579 => 242580)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/UIProcess/ProcessThrottler.cpp       2019-03-07 00:38:01 UTC (rev 242579)
+++ trunk/Source/WebKit/UIProcess/ProcessThrottler.cpp  2019-03-07 00:43:24 UTC (rev 242580)
</span><span class="lines">@@ -68,10 +68,12 @@
</span><span class="cx">     
</span><span class="cx"> void ProcessThrottler::updateAssertion()
</span><span class="cx"> {
</span><ins>+    bool shouldBeRunnable = m_foregroundCounter.value() || m_backgroundCounter.value();
+
</ins><span class="cx">     // If the process is currently runnable but will be suspended then first give it a chance to complete what it was doing
</span><span class="cx">     // and clean up - move it to the background and send it a message to notify. Schedule a timeout so it can't stay running
</span><span class="cx">     // in the background for too long.
</span><del>-    if (m_assertion && m_assertion->state() != AssertionState::Suspended && !m_foregroundCounter.value() && !m_backgroundCounter.value()) {
</del><ins>+    if (m_assertion && m_assertion->state() != AssertionState::Suspended && !shouldBeRunnable) {
</ins><span class="cx">         ++m_suspendMessageCount;
</span><span class="cx">         RELEASE_LOG(ProcessSuspension, "%p - ProcessThrottler::updateAssertion() sending PrepareToSuspend IPC", this);
</span><span class="cx">         m_process.sendPrepareToSuspend();
</span><span class="lines">@@ -81,14 +83,16 @@
</span><span class="cx">         return;
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    bool shouldBeRunnable = m_foregroundCounter.value() || m_backgroundCounter.value();
</del><ins>+    if (shouldBeRunnable) {
+        // If we're currently waiting for the Web process to do suspension cleanup, but no longer need to be suspended, tell the Web process to cancel the cleanup.
+        if (m_suspendTimer.isActive())
+            m_process.sendCancelPrepareToSuspend();
</ins><span class="cx"> 
</span><del>-    // If we're currently waiting for the Web process to do suspension cleanup, but no longer need to be suspended, tell the Web process to cancel the cleanup.
-    if (m_suspendTimer.isActive() && shouldBeRunnable)
-        m_process.sendCancelPrepareToSuspend();
-    
-    if (m_assertion && m_assertion->state() == AssertionState::Suspended && shouldBeRunnable)
-        m_process.sendProcessDidResume();
</del><ins>+        if ((m_assertion && m_assertion->state() == AssertionState::Suspended) || m_uiAssertionExpired) {
+            m_process.sendProcessDidResume();
+            m_uiAssertionExpired = false;
+        }
+    }
</ins><span class="cx"> 
</span><span class="cx">     updateAssertionNow();
</span><span class="cx"> }
</span><span class="lines">@@ -125,9 +129,10 @@
</span><span class="cx">     ASSERT(m_suspendMessageCount >= 0);
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void ProcessThrottler::assertionWillExpireImminently()
</del><ins>+void ProcessThrottler::uiAssertionWillExpireImminently()
</ins><span class="cx"> {
</span><span class="cx">     m_process.sendProcessWillSuspendImminently();
</span><ins>+    m_uiAssertionExpired = true;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebKitUIProcessProcessThrottlerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/UIProcess/ProcessThrottler.h (242579 => 242580)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/UIProcess/ProcessThrottler.h 2019-03-07 00:38:01 UTC (rev 242579)
+++ trunk/Source/WebKit/UIProcess/ProcessThrottler.h    2019-03-07 00:43:24 UTC (rev 242580)
</span><span class="lines">@@ -68,7 +68,7 @@
</span><span class="cx">     void suspendTimerFired();
</span><span class="cx"> 
</span><span class="cx">     // ProcessAssertionClient
</span><del>-    void assertionWillExpireImminently() override;
</del><ins>+    void uiAssertionWillExpireImminently() override;
</ins><span class="cx"> 
</span><span class="cx">     ProcessThrottlerClient& m_process;
</span><span class="cx">     std::unique_ptr<ProcessAssertion> m_assertion;
</span><span class="lines">@@ -77,6 +77,7 @@
</span><span class="cx">     BackgroundActivityCounter m_backgroundCounter;
</span><span class="cx">     int m_suspendMessageCount { 0 };
</span><span class="cx">     bool m_shouldTakeUIBackgroundAssertion;
</span><ins>+    bool m_uiAssertionExpired { false };
</ins><span class="cx"> };
</span><span class="cx"> 
</span><span class="cx"> inline ProcessThrottler::ForegroundActivityToken ProcessThrottler::foregroundActivityToken() const
</span></span></pre></div>
<a id="trunkSourceWebKitUIProcessiosProcessAssertionIOSmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm (242579 => 242580)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm 2019-03-07 00:38:01 UTC (rev 242579)
+++ trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm    2019-03-07 00:43:24 UTC (rev 242580)
</span><span class="lines">@@ -98,7 +98,7 @@
</span><span class="cx">     ASSERT(RunLoop::isMain());
</span><span class="cx"> 
</span><span class="cx">     for (auto* client : copyToVector(_clients))
</span><del>-        client->assertionWillExpireImminently();
</del><ins>+        client->uiAssertionWillExpireImminently();
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> - (void)_updateBackgroundTask
</span><span class="lines">@@ -208,9 +208,6 @@
</span><span class="cx"> {
</span><span class="cx">     m_assertion.get().invalidationHandler = nil;
</span><span class="cx"> 
</span><del>-    if (ProcessAssertionClient* client = this->client())
-        [[WKProcessAssertionBackgroundTaskManager shared] removeClient:*client];
-
</del><span class="cx">     RELEASE_LOG(ProcessSuspension, "%p - ~ProcessAssertion() Releasing process assertion", this);
</span><span class="cx">     [m_assertion invalidate];
</span><span class="cx"> }
</span><span class="lines">@@ -259,6 +256,9 @@
</span><span class="cx"> {
</span><span class="cx">     if (m_isHoldingBackgroundAssertion)
</span><span class="cx">         [[WKProcessAssertionBackgroundTaskManager shared] decrementNeedsToRunInBackgroundCount];
</span><ins>+
+    if (auto* client = this->client())
+        [[WKProcessAssertionBackgroundTaskManager shared] removeClient:*client];
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void ProcessAndUIAssertion::setState(AssertionState assertionState)
</span><span class="lines">@@ -270,7 +270,7 @@
</span><span class="cx"> void ProcessAndUIAssertion::setClient(ProcessAssertionClient& newClient)
</span><span class="cx"> {
</span><span class="cx">     [[WKProcessAssertionBackgroundTaskManager shared] addClient:newClient];
</span><del>-    if (ProcessAssertionClient* oldClient = this->client())
</del><ins>+    if (auto* oldClient = this->client())
</ins><span class="cx">         [[WKProcessAssertionBackgroundTaskManager shared] removeClient:*oldClient];
</span><span class="cx">     ProcessAssertion::setClient(newClient);
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebKitWebProcessWebPageWebPagecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (242579 => 242580)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp       2019-03-07 00:38:01 UTC (rev 242579)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp  2019-03-07 00:43:24 UTC (rev 242580)
</span><span class="lines">@@ -3147,10 +3147,9 @@
</span><span class="cx">     if (m_LayerTreeFreezeReasons.contains(LayerTreeFreezeReason::ProcessSuspended)) {
</span><span class="cx">         RELEASE_LOG_ERROR(ProcessSuspension, "%p - WebPage (PageID=%" PRIu64 ") - LayerTreeFreezeReason::ProcessSuspended was set when removing LayerTreeFreezeReason::PageTransition; current reasons are %d",
</span><span class="cx">             this, m_pageID, m_LayerTreeFreezeReasons.toRaw());
</span><ins>+        ASSERT_NOT_REACHED();
</ins><span class="cx">     }
</span><span class="cx"> 
</span><del>-    // FIXME: In iOS, we sometimes never unset ProcessSuspended. See <rdar://problem/48154508>.
-    unfreezeLayerTree(LayerTreeFreezeReason::ProcessSuspended);
</del><span class="cx">     RELEASE_LOG_IF_ALLOWED("%p - WebPage - Did complete page transition", this);
</span><span class="cx"> 
</span><span class="cx">     bool isInitialEmptyDocument = !m_mainFrame;
</span></span></pre></div>
<a id="trunkSourceWebKitWebProcessWebPageiosWebPageIOSmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (242579 => 242580)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2019-03-07 00:38:01 UTC (rev 242579)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm    2019-03-07 00:43:24 UTC (rev 242580)
</span><span class="lines">@@ -3010,8 +3010,6 @@
</span><span class="cx">     m_isSuspendedUnderLock = false;
</span><span class="cx">     cancelMarkLayersVolatile();
</span><span class="cx"> 
</span><del>-    // FIXME: In iOS, we sometimes never unset ProcessSuspended. See <rdar://problem/48538837>.
-    unfreezeLayerTree(LayerTreeFreezeReason::ProcessSuspended);
</del><span class="cx">     unfreezeLayerTree(LayerTreeFreezeReason::BackgroundApplication);
</span><span class="cx"> 
</span><span class="cx">     [[NSNotificationCenter defaultCenter] postNotificationName:WebUIApplicationWillEnterForegroundNotification object:nil userInfo:@{@"isSuspendedUnderLock": @(isSuspendedUnderLock)}];
</span></span></pre>
</div>
</div>

</body>
</html>