Diff
Modified: trunk/Source/WebCore/ChangeLog (279965 => 279966)
--- trunk/Source/WebCore/ChangeLog 2021-07-15 22:26:23 UTC (rev 279965)
+++ trunk/Source/WebCore/ChangeLog 2021-07-15 22:45:24 UTC (rev 279966)
@@ -1,3 +1,18 @@
+2021-07-15 Sihui Liu <[email protected]>
+
+ Do not abort ongoing IDB transaction synchronously on non-imminent PrepareToSuspend message
+ https://bugs.webkit.org/show_bug.cgi?id=227778
+ <rdar://problem/80602557>
+
+ Reviewed by Chris Dumez.
+
+ * Modules/indexeddb/server/IDBServer.cpp:
+ (WebCore::IDBServer::IDBServer::hasDatabaseActivitiesOnMainThread const):
+ * Modules/indexeddb/server/IDBServer.h:
+ * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+ (WebCore::IDBServer::UniqueIDBDatabase::hasActiveTransactions const):
+ * Modules/indexeddb/server/UniqueIDBDatabase.h:
+
2021-07-15 Peng Liu <[email protected]>
Reddit PiP video pauses when scrolling
Modified: trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp (279965 => 279966)
--- trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp 2021-07-15 22:26:23 UTC (rev 279965)
+++ trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp 2021-07-15 22:45:24 UTC (rev 279966)
@@ -767,6 +767,22 @@
FileSystem::makeAllDirectories(newVersionDirectory);
}
+bool IDBServer::hasDatabaseActivitiesOnMainThread() const
+{
+ ASSERT(isMainThread());
+ ASSERT(m_lock.isHeld());
+
+ if (m_sessionID.isEphemeral())
+ return false;
+
+ for (auto& database : m_uniqueIDBDatabaseMap.values()) {
+ if (!database->identifier().isTransient() && database->hasActiveTransactions())
+ return true;
+ }
+
+ return false;
+}
+
void IDBServer::stopDatabaseActivitiesOnMainThread()
{
ASSERT(isMainThread());
Modified: trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.h (279965 => 279966)
--- trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.h 2021-07-15 22:26:23 UTC (rev 279965)
+++ trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.h 2021-07-15 22:45:24 UTC (rev 279966)
@@ -107,6 +107,7 @@
StorageQuotaManager::Decision requestSpace(const ClientOrigin&, uint64_t taskSize);
WEBCORE_EXPORT static uint64_t diskUsage(const String& rootDirectory, const ClientOrigin&);
+ WEBCORE_EXPORT bool hasDatabaseActivitiesOnMainThread() const;
WEBCORE_EXPORT void stopDatabaseActivitiesOnMainThread();
private:
Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp (279965 => 279966)
--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp 2021-07-15 22:26:23 UTC (rev 279965)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp 2021-07-15 22:45:24 UTC (rev 279966)
@@ -1191,6 +1191,13 @@
close();
}
+bool UniqueIDBDatabase::hasActiveTransactions() const
+{
+ ASSERT(isMainThread());
+
+ return !m_inProgressTransactions.isEmpty();
+}
+
void UniqueIDBDatabase::abortActiveTransactions()
{
ASSERT(isMainThread());
Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h (279965 => 279966)
--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h 2021-07-15 22:26:23 UTC (rev 279965)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h 2021-07-15 22:45:24 UTC (rev 279966)
@@ -107,6 +107,7 @@
void handleDelete(IDBConnectionToClient&, const IDBRequestData&);
void immediateClose();
+ bool hasActiveTransactions() const;
void abortActiveTransactions();
bool tryClose();
Modified: trunk/Source/WebKit/ChangeLog (279965 => 279966)
--- trunk/Source/WebKit/ChangeLog 2021-07-15 22:26:23 UTC (rev 279965)
+++ trunk/Source/WebKit/ChangeLog 2021-07-15 22:45:24 UTC (rev 279966)
@@ -1,3 +1,37 @@
+2021-07-15 Sihui Liu <[email protected]>
+
+ Do not abort ongoing IDB transaction synchronously on non-imminent PrepareToSuspend message
+ https://bugs.webkit.org/show_bug.cgi?id=227778
+ <rdar://problem/80602557>
+
+ Reviewed by Chris Dumez.
+
+ Currently we abort IDB transactions and suspend IDB thread synchronously in NetworkProcess::prepareToSuspend.
+ As UI process does not know the file locking state in network process now (because network process tracks
+ the state and takes the assertion by itself), UI process may send non-imminent PrepareToSuspend more often,
+ and we may abort transaction too aggressively (e.g. UI process may send the message as soon as app is in
+ background). To fix the possible regression of data loss, we will schedule a task with 5s delay to abort
+ transactions after prepareToSuspend. In this case, transactions finished in 5s can have a chance to finish.
+
+ * NetworkProcess/IndexedDB/WebIDBServer.cpp:
+ * NetworkProcess/IndexedDB/WebIDBServer.h:
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::suspendIDBServers):
+ (WebKit::NetworkProcess::prepareToSuspend):
+ (WebKit::NetworkProcess::resume):
+ (WebKit::NetworkProcess::createWebIDBServer):
+ * NetworkProcess/NetworkProcess.h:
+ * NetworkProcess/ios/NetworkProcessIOS.mm:
+ (WebKit::NetworkProcess::setIsHoldingLockedFiles):
+ * UIProcess/ProcessAssertion.h:
+ (WebKit::ProcessAssertion::setPrepareForInvalidationHandler):
+ * UIProcess/ios/ProcessAssertionIOS.mm:
+ (-[WKRBSAssertionDelegate dealloc]):
+ (-[WKRBSAssertionDelegate assertionWillInvalidate:]):
+ (WebKit::ProcessAssertion::ProcessAssertion):
+ (WebKit::ProcessAssertion::~ProcessAssertion):
+ (WebKit::ProcessAssertion::processAssertionWillBeInvalidated):
+
2021-07-15 Megan Gardner <[email protected]>
Rename scrollRectIntoView to scrollContainingScrollViewsToRevealRect for clarity.
Modified: trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.cpp (279965 => 279966)
--- trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.cpp 2021-07-15 22:26:23 UTC (rev 279965)
+++ trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.cpp 2021-07-15 22:45:24 UTC (rev 279966)
@@ -115,17 +115,34 @@
});
}
-void WebIDBServer::suspend() WTF_IGNORES_THREAD_SAFETY_ANALYSIS
+bool WebIDBServer::suspend(SuspensionCondition suspensionCondition) WTF_IGNORES_THREAD_SAFETY_ANALYSIS
{
ASSERT(RunLoop::isMain());
if (m_isSuspended)
- return;
+ return true;
+ RELEASE_LOG(ProcessSuspension, "%p - WebIDBServer::suspend(), suspensionCondition=%s", this, suspensionCondition == SuspensionCondition::Always ? "Always" : "IfIdle");
+
m_isSuspended = true;
m_serverLock.lock();
- if (m_server)
+
+ if (!m_server)
+ return true;
+
+ if (suspensionCondition == SuspensionCondition::Always) {
m_server->stopDatabaseActivitiesOnMainThread();
+ return true;
+ }
+
+ // Suspend to avoid starting new transactions if there is no ongoing transaction.
+ if (!m_server->hasDatabaseActivitiesOnMainThread())
+ return true;
+
+ // Resume to allow ongoing transactions to be finished.
+ m_serverLock.unlock();
+ m_isSuspended = false;
+ return false;
}
void WebIDBServer::resume() WTF_IGNORES_THREAD_SAFETY_ANALYSIS
@@ -135,6 +152,8 @@
if (!m_isSuspended)
return;
+ RELEASE_LOG(ProcessSuspension, "%p - WebIDBServer::resume()", this);
+
m_isSuspended = false;
m_serverLock.unlock();
}
Modified: trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.h (279965 => 279966)
--- trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.h 2021-07-15 22:26:23 UTC (rev 279965)
+++ trunk/Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.h 2021-07-15 22:45:24 UTC (rev 279966)
@@ -52,7 +52,8 @@
void closeAndDeleteDatabasesForOrigins(const Vector<WebCore::SecurityOriginData>&, CompletionHandler<void()>&& callback);
void renameOrigin(const WebCore::SecurityOriginData&, const WebCore::SecurityOriginData&, CompletionHandler<void()>&&);
- void suspend();
+ enum class SuspensionCondition : bool { Always, IfIdle };
+ bool suspend(SuspensionCondition = SuspensionCondition::Always);
void resume();
// Message handlers.
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp (279965 => 279966)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2021-07-15 22:26:23 UTC (rev 279965)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2021-07-15 22:45:24 UTC (rev 279966)
@@ -2211,14 +2211,36 @@
prepareToSuspend(true, WTFMove(completionHandler));
}
+void NetworkProcess::suspendIDBServers(bool isSuspensionImminent)
+{
+ m_shouldSuspendIDBServers = true;
+
+ bool allSuspended = true;
+ auto condition = isSuspensionImminent ? WebIDBServer::SuspensionCondition::Always : WebIDBServer::SuspensionCondition::IfIdle;
+ for (auto& server : m_webIDBServers.values())
+ allSuspended &= server->suspend(condition);
+
+ if (allSuspended)
+ return;
+
+ RunLoop::main().dispatchAfter(5_s, [this, weakThis = makeWeakPtr(*this)] {
+ if (!weakThis)
+ return;
+
+ if (!m_shouldSuspendIDBServers)
+ return;
+
+ for (auto& server : m_webIDBServers.values())
+ server->suspend(WebIDBServer::SuspensionCondition::Always);
+ });
+}
+
void NetworkProcess::prepareToSuspend(bool isSuspensionImminent, CompletionHandler<void()>&& completionHandler)
{
RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::prepareToSuspend(), isSuspensionImminent=%d", this, isSuspensionImminent);
#if PLATFORM(IOS_FAMILY)
- for (auto& server : m_webIDBServers.values())
- server->suspend();
- m_shouldSuspendIDBServer = true;
+ suspendIDBServers(isSuspensionImminent);
#endif
lowMemoryHandler(Critical::Yes);
@@ -2282,7 +2304,7 @@
#if PLATFORM(IOS_FAMILY)
for (auto& server : m_webIDBServers.values())
server->resume();
- m_shouldSuspendIDBServer = false;
+ m_shouldSuspendIDBServers = false;
#endif
m_storageManagerSet->resume();
@@ -2347,8 +2369,8 @@
};
auto result = WebIDBServer::create(sessionID, path, WTFMove(spaceRequester));
- if (m_shouldSuspendIDBServer)
- result->suspend();
+ if (m_shouldSuspendIDBServers)
+ result->suspend(WebIDBServer::SuspensionCondition::Always);
return result;
}
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.h (279965 => 279966)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.h 2021-07-15 22:26:23 UTC (rev 279965)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.h 2021-07-15 22:45:24 UTC (rev 279966)
@@ -486,6 +486,7 @@
Ref<WebIDBServer> createWebIDBServer(PAL::SessionID);
void setSessionStorageQuotaManagerIDBRootPath(PAL::SessionID, const String& idbRootPath);
void removeWebIDBServerIfPossible(PAL::SessionID);
+ void suspendIDBServers(bool isSuspensionImminent);
#if ENABLE(SERVICE_WORKER)
void didCreateWorkerContextProcessConnection(const IPC::Attachment&);
@@ -583,7 +584,7 @@
HashMap<PAL::SessionID, String> m_idbDatabasePaths;
HashMap<PAL::SessionID, RefPtr<WebIDBServer>> m_webIDBServers;
- bool m_shouldSuspendIDBServer { false };
+ bool m_shouldSuspendIDBServers { false };
#if ENABLE(SERVICE_WORKER)
struct ServiceWorkerInfo {
Modified: trunk/Source/WebKit/NetworkProcess/ios/NetworkProcessIOS.mm (279965 => 279966)
--- trunk/Source/WebKit/NetworkProcess/ios/NetworkProcessIOS.mm 2021-07-15 22:26:23 UTC (rev 279965)
+++ trunk/Source/WebKit/NetworkProcess/ios/NetworkProcessIOS.mm 2021-07-15 22:45:24 UTC (rev 279966)
@@ -113,6 +113,17 @@
// We synchronously take a process assertion when beginning a SQLite transaction so that we don't get suspended
// while holding a locked file. We would get killed if suspended while holding locked files.
m_holdingLockedFileAssertion = ProcessAssertion::create(getCurrentProcessID(), "Network Process is holding locked files"_s, ProcessAssertionType::FinishTaskUninterruptable, ProcessAssertion::Mode::Sync);
+ m_holdingLockedFileAssertion->setPrepareForInvalidationHandler([this, weakThis = makeWeakPtr(*this)]() mutable {
+ ASSERT(isMainRunLoop());
+ if (!weakThis)
+ return;
+
+ if (!m_shouldSuspendIDBServers)
+ return;
+
+ for (auto& server : m_webIDBServers.values())
+ server->suspend();
+ });
}
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/ProcessAssertion.h (279965 => 279966)
--- trunk/Source/WebKit/UIProcess/ProcessAssertion.h 2021-07-15 22:26:23 UTC (rev 279965)
+++ trunk/Source/WebKit/UIProcess/ProcessAssertion.h 2021-07-15 22:45:24 UTC (rev 279966)
@@ -72,6 +72,7 @@
}
virtual ~ProcessAssertion();
+ void setPrepareForInvalidationHandler(Function<void()>&& handler) { m_prepareForInvalidationHandler = WTFMove(handler); }
void setInvalidationHandler(Function<void()>&& handler) { m_invalidationHandler = WTFMove(handler); }
ProcessAssertionType type() const { return m_assertionType; }
@@ -86,6 +87,7 @@
void acquireSync();
#if PLATFORM(IOS_FAMILY)
+ void processAssertionWillBeInvalidated();
virtual void processAssertionWasInvalidated();
#endif
@@ -98,6 +100,7 @@
RetainPtr<WKRBSAssertionDelegate> m_delegate;
bool m_wasInvalidated { false };
#endif
+ Function<void()> m_prepareForInvalidationHandler;
Function<void()> m_invalidationHandler;
};
Modified: trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm (279965 => 279966)
--- trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm 2021-07-15 22:26:23 UTC (rev 279965)
+++ trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm 2021-07-15 22:45:24 UTC (rev 279966)
@@ -255,6 +255,7 @@
typedef void(^RBSAssertionInvalidationCallbackType)();
@interface WKRBSAssertionDelegate : NSObject<RBSAssertionObserving>
+@property (copy) RBSAssertionInvalidationCallbackType prepareForInvalidationCallback;
@property (copy) RBSAssertionInvalidationCallbackType invalidationCallback;
@end
@@ -261,6 +262,7 @@
@implementation WKRBSAssertionDelegate
- (void)dealloc
{
+ [_prepareForInvalidationCallback release];
[_invalidationCallback release];
[super dealloc];
}
@@ -268,6 +270,12 @@
- (void)assertionWillInvalidate:(RBSAssertion *)assertion
{
RELEASE_LOG(ProcessSuspension, "%p - WKRBSAssertionDelegate: assertionWillInvalidate", self);
+
+ RunLoop::main().dispatch([weakSelf = WeakObjCPtr<WKRBSAssertionDelegate>(self)] {
+ auto strongSelf = weakSelf.get();
+ if (strongSelf && strongSelf.get().prepareForInvalidationCallback)
+ strongSelf.get().prepareForInvalidationCallback();
+ });
}
- (void)assertion:(RBSAssertion *)assertion didInvalidateWithError:(NSError *)error
@@ -339,6 +347,10 @@
RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion: RBS %{public}@ assertion for process with PID=%d was invalidated", this, runningBoardAssertionName, pid);
processAssertionWasInvalidated();
};
+ m_delegate.get().prepareForInvalidationCallback = ^{
+ RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion() RBS %{public}@ assertion for process with PID=%d will be invalidated", this, runningBoardAssertionName, pid);
+ processAssertionWillBeInvalidated();
+ };
}
void ProcessAssertion::acquireAsync(CompletionHandler<void()>&& completionHandler)
@@ -373,6 +385,7 @@
if (m_rbsAssertion) {
m_delegate.get().invalidationCallback = nil;
+ m_delegate.get().prepareForInvalidationCallback = nil;
[m_rbsAssertion removeObserver:m_delegate.get()];
m_delegate = nil;
[m_rbsAssertion invalidate];
@@ -379,6 +392,15 @@
}
}
+void ProcessAssertion::processAssertionWillBeInvalidated()
+{
+ ASSERT(RunLoop::isMain());
+ RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion::processAssertionWillBeInvalidated() PID=%d", this, m_pid);
+
+ if (m_prepareForInvalidationHandler)
+ m_prepareForInvalidationHandler();
+}
+
void ProcessAssertion::processAssertionWasInvalidated()
{
ASSERT(RunLoop::isMain());
