[webkit-changes] [233562] trunk

Thu, 05 Jul 2018 22:48:48 -0700

Title: [233562] trunk
Revision
233562
Author
[email protected]
Date
2018-07-05 22:48:35 -0700 (Thu, 05 Jul 2018)

Log Message

IndexedDB operations in a Page fail after a StorageProcess crash.
<rdar://problem/41626526> and https://bugs.webkit.org/show_bug.cgi?id=187123

Reviewed by Alex Christensen.

Source/WebCore:

Test: storage/indexeddb/modern/opendatabase-after-storage-crash.html

When the connection to a StorageProcess goes away, explicitly tell all of the WebPages
in the WebProcess about it.

This puts Documents/Workers in an error mode where requests fail instead of timeout.
It also clears the Page's connection so *new* Documents and Workers will get a fresh
new connection that works.

* Modules/indexeddb/client/IDBConnectionToServer.cpp:
(WebCore::IDBClient::IDBConnectionToServer::callResultFunctionLater):
(WebCore::IDBClient::IDBConnectionToServer::deleteDatabase):
(WebCore::IDBClient::IDBConnectionToServer::openDatabase):
(WebCore::IDBClient::IDBConnectionToServer::createObjectStore):
(WebCore::IDBClient::IDBConnectionToServer::deleteObjectStore):
(WebCore::IDBClient::IDBConnectionToServer::renameObjectStore):
(WebCore::IDBClient::IDBConnectionToServer::clearObjectStore):
(WebCore::IDBClient::IDBConnectionToServer::createIndex):
(WebCore::IDBClient::IDBConnectionToServer::deleteIndex):
(WebCore::IDBClient::IDBConnectionToServer::renameIndex):
(WebCore::IDBClient::IDBConnectionToServer::putOrAdd):
(WebCore::IDBClient::IDBConnectionToServer::getRecord):
(WebCore::IDBClient::IDBConnectionToServer::getAllRecords):
(WebCore::IDBClient::IDBConnectionToServer::getCount):
(WebCore::IDBClient::IDBConnectionToServer::deleteRecord):
(WebCore::IDBClient::IDBConnectionToServer::openCursor):
(WebCore::IDBClient::IDBConnectionToServer::iterateCursor):
(WebCore::IDBClient::IDBConnectionToServer::establishTransaction):
(WebCore::IDBClient::IDBConnectionToServer::commitTransaction):
(WebCore::IDBClient::IDBConnectionToServer::didFinishHandlingVersionChangeTransaction):
(WebCore::IDBClient::IDBConnectionToServer::abortTransaction):
(WebCore::IDBClient::IDBConnectionToServer::didFireVersionChangeEvent):
(WebCore::IDBClient::IDBConnectionToServer::confirmDidCloseFromServer):
(WebCore::IDBClient::IDBConnectionToServer::connectionToServerLost):
(WebCore::IDBClient::IDBConnectionToServer::openDBRequestCancelled):
(WebCore::IDBClient::IDBConnectionToServer::databaseConnectionPendingClose):
(WebCore::IDBClient::IDBConnectionToServer::databaseConnectionClosed):
(WebCore::IDBClient::IDBConnectionToServer::abortOpenAndUpgradeNeeded):
(WebCore::IDBClient::IDBConnectionToServer::getAllDatabaseNames):
* Modules/indexeddb/client/IDBConnectionToServer.h:

* Modules/indexeddb/shared/IDBError.h:
(WebCore::IDBError::serverConnectionLostError):

* page/Page.cpp:
(WebCore::Page::setSessionID):
(WebCore::Page::idbConnection):
(WebCore::Page::optionalIDBConnection):
(WebCore::Page::clearIDBConnection):
* page/Page.h:

Source/WebKit:

When the connection to a StorageProcess goes away, explicitly tell all of the WebPages
in the WebProcess about it.

This puts Documents/Workers in an error mode where requests fail instead of timeout.
It also clears the Page's connection so *new* Documents and Workers will get a fresh
new connection that works.

* UIProcess/API/C/WKContext.cpp:
(WKContextTerminateStorageProcess):

* UIProcess/API/Cocoa/WKProcessPool.mm:
(-[WKProcessPool _terminateStorageProcess]):

* UIProcess/Storage/StorageProcessProxy.cpp:
(WebKit::StorageProcessProxy::terminateForTesting):
* UIProcess/Storage/StorageProcessProxy.h:

* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::terminateStorageProcessForTesting):
(WebKit::WebProcessPool::terminateStorageProcess): Deleted.
* UIProcess/WebProcessPool.h:

* WebProcess/Storage/WebToStorageProcessConnection.cpp:
(WebKit::WebToStorageProcessConnection::didClose):
* WebProcess/Storage/WebToStorageProcessConnection.h:
(WebKit::WebToStorageProcessConnection::existingIDBConnectionToServerForIdentifier):

* WebProcess/WebProcess.cpp:
(WebKit::WebProcess::webToStorageProcessConnectionClosed):

LayoutTests:

* storage/indexeddb/modern/opendatabase-after-storage-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (233561 => 233562)


--- trunk/LayoutTests/ChangeLog	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/LayoutTests/ChangeLog	2018-07-06 05:48:35 UTC (rev 233562)
@@ -1,3 +1,12 @@
+2018-07-05  Brady Eidson  <[email protected]>
+
+        IndexedDB operations in a Page fail after a StorageProcess crash.
+        <rdar://problem/41626526> and https://bugs.webkit.org/show_bug.cgi?id=187123
+
+        Reviewed by Alex Christensen.
+
+        * storage/indexeddb/modern/opendatabase-after-storage-crash.html: Added.
+
 2018-07-05  Ryosuke Niwa  <[email protected]>
 
         Youtube video pages crash after a couple of minutes

Modified: trunk/LayoutTests/TestExpectations (233561 => 233562)


--- trunk/LayoutTests/TestExpectations	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/LayoutTests/TestExpectations	2018-07-06 05:48:35 UTC (rev 233562)
@@ -118,6 +118,7 @@
 http/tests/storageAccess/ [ Skip ]
 http/tests/navigation/process-swap-window-open.html [ Skip ]
 http/tests/navigation/useragent-reload.php [ Skip ]
+storage/indexeddb/modern/opendatabase-after-storage-crash.html [ Skip ]
 
 # Only Mac and iOS have an implementation of UIScriptController::doAsyncTask().
 fast/harness/uiscriptcontroller [ Skip ]

Modified: trunk/LayoutTests/platform/wk2/TestExpectations (233561 => 233562)


--- trunk/LayoutTests/platform/wk2/TestExpectations	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/LayoutTests/platform/wk2/TestExpectations	2018-07-06 05:48:35 UTC (rev 233562)
@@ -736,7 +736,9 @@
 http/wpt/cross-origin-resource-policy/ [ Pass ]
 
 http/tests/navigation/useragent-reload.php [ Pass ]
+storage/indexeddb/modern/opendatabase-after-storage-crash.html [ Pass ]
 
+
 ### END OF (5) Progressions, expected successes that are expected failures in WebKit1.
 ########################################

Added: trunk/LayoutTests/storage/indexeddb/modern/opendatabase-after-storage-crash-expected.txt (0 => 233562)


--- trunk/LayoutTests/storage/indexeddb/modern/opendatabase-after-storage-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/modern/opendatabase-after-storage-crash-expected.txt	2018-07-06 05:48:35 UTC (rev 233562)
@@ -0,0 +1 @@
+Successfully opened the database in a new document

Added: trunk/LayoutTests/storage/indexeddb/modern/opendatabase-after-storage-crash.html (0 => 233562)


--- trunk/LayoutTests/storage/indexeddb/modern/opendatabase-after-storage-crash.html	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/modern/opendatabase-after-storage-crash.html	2018-07-06 05:48:35 UTC (rev 233562)
@@ -0,0 +1,60 @@
+<script src=""
+<script src=""
+<body>
+If this test completes quickly instead of hanging indefinitely, it has passed.
+</body>
+<script>
+
+var dbname = setDBNameFromPath() + Date();
+
+function continueTest()
+{
+	var request = window.indexedDB.open(dbname, 2);
+	request._onupgradeneeded_ = function(e) {
+		document.body.innerHTML = "openDatabase call after storage process termination should not have succeeded";
+		if (window.testRunner)
+			testRunner.notifyDone();
+
+	}
+	request._onerror_ = function(e) {
+		// Good, we received an expected error.
+		// Now reload to see if a new document connects successfully
+		if (sessionStorage[location.href]) {
+			document.body.innerHTML = "Should not have reached this code twice!";
+			if (window.testRunner)
+				testRunner.notifyDone();
+		}
+		
+		sessionStorage[location.href] = "Made it";
+		location.reload();
+	}
+}
+
+var request = window.indexedDB.open(dbname, 1);
+request._onupgradeneeded_ = function(e) {
+	if (sessionStorage.finishedFirstOpen) {
+		document.body.innerHTML = "";
+		if (sessionStorage[location.href] != "Made it")
+			document.body.innerHTML += "Expected error in the first pass of the test was NOT logged<br>";
+		document.body.innerHTML += "Successfully opened the database in a new document"
+		if (window.testRunner)
+			testRunner.notifyDone();
+	}
+	
+	sessionStorage.finishedFirstOpen = true;
+	
+	if (window.testRunner) {
+		testRunner.terminateStorageProcess();
+		// One spin of the runloop is *ALMOST* always enough delay here.
+		// Padding it makes this reliable.
+		setTimeout(continueTest, 10);
+	}
+}
+
+request._onerror_ = function(e) {
+	document.body.innerHTML = "Unexpected error during the first database open on document load. " + sessionStorage[location.href];
+	if (window.testRunner)
+		testRunner.notifyDone();	
+}
+
+</script>

Modified: trunk/Source/WebCore/ChangeLog (233561 => 233562)


--- trunk/Source/WebCore/ChangeLog	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebCore/ChangeLog	2018-07-06 05:48:35 UTC (rev 233562)
@@ -1,3 +1,61 @@
+2018-07-05  Brady Eidson  <[email protected]>
+
+        IndexedDB operations in a Page fail after a StorageProcess crash.
+        <rdar://problem/41626526> and https://bugs.webkit.org/show_bug.cgi?id=187123
+
+        Reviewed by Alex Christensen.
+
+        Test: storage/indexeddb/modern/opendatabase-after-storage-crash.html
+
+        When the connection to a StorageProcess goes away, explicitly tell all of the WebPages
+        in the WebProcess about it.
+        
+        This puts Documents/Workers in an error mode where requests fail instead of timeout.
+        It also clears the Page's connection so *new* Documents and Workers will get a fresh 
+        new connection that works.
+        
+        * Modules/indexeddb/client/IDBConnectionToServer.cpp:
+        (WebCore::IDBClient::IDBConnectionToServer::callResultFunctionLater):
+        (WebCore::IDBClient::IDBConnectionToServer::deleteDatabase):
+        (WebCore::IDBClient::IDBConnectionToServer::openDatabase):
+        (WebCore::IDBClient::IDBConnectionToServer::createObjectStore):
+        (WebCore::IDBClient::IDBConnectionToServer::deleteObjectStore):
+        (WebCore::IDBClient::IDBConnectionToServer::renameObjectStore):
+        (WebCore::IDBClient::IDBConnectionToServer::clearObjectStore):
+        (WebCore::IDBClient::IDBConnectionToServer::createIndex):
+        (WebCore::IDBClient::IDBConnectionToServer::deleteIndex):
+        (WebCore::IDBClient::IDBConnectionToServer::renameIndex):
+        (WebCore::IDBClient::IDBConnectionToServer::putOrAdd):
+        (WebCore::IDBClient::IDBConnectionToServer::getRecord):
+        (WebCore::IDBClient::IDBConnectionToServer::getAllRecords):
+        (WebCore::IDBClient::IDBConnectionToServer::getCount):
+        (WebCore::IDBClient::IDBConnectionToServer::deleteRecord):
+        (WebCore::IDBClient::IDBConnectionToServer::openCursor):
+        (WebCore::IDBClient::IDBConnectionToServer::iterateCursor):
+        (WebCore::IDBClient::IDBConnectionToServer::establishTransaction):
+        (WebCore::IDBClient::IDBConnectionToServer::commitTransaction):
+        (WebCore::IDBClient::IDBConnectionToServer::didFinishHandlingVersionChangeTransaction):
+        (WebCore::IDBClient::IDBConnectionToServer::abortTransaction):
+        (WebCore::IDBClient::IDBConnectionToServer::didFireVersionChangeEvent):
+        (WebCore::IDBClient::IDBConnectionToServer::confirmDidCloseFromServer):
+        (WebCore::IDBClient::IDBConnectionToServer::connectionToServerLost):
+        (WebCore::IDBClient::IDBConnectionToServer::openDBRequestCancelled):
+        (WebCore::IDBClient::IDBConnectionToServer::databaseConnectionPendingClose):
+        (WebCore::IDBClient::IDBConnectionToServer::databaseConnectionClosed):
+        (WebCore::IDBClient::IDBConnectionToServer::abortOpenAndUpgradeNeeded):
+        (WebCore::IDBClient::IDBConnectionToServer::getAllDatabaseNames):
+        * Modules/indexeddb/client/IDBConnectionToServer.h:
+
+        * Modules/indexeddb/shared/IDBError.h:
+        (WebCore::IDBError::serverConnectionLostError):
+
+        * page/Page.cpp:
+        (WebCore::Page::setSessionID):
+        (WebCore::Page::idbConnection):
+        (WebCore::Page::optionalIDBConnection):
+        (WebCore::Page::clearIDBConnection):
+        * page/Page.h:
+
 2018-07-05  Ryosuke Niwa  <[email protected]>
 
         Youtube video pages crash after a couple of minutes

Modified: trunk/Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp (233561 => 233562)


--- trunk/Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp	2018-07-06 05:48:35 UTC (rev 233562)
@@ -64,10 +64,21 @@
     return *m_proxy;
 }
 
+void IDBConnectionToServer::callResultFunctionWithErrorLater(ResultFunction function, const IDBResourceIdentifier& requestIdentifier)
+{
+    callOnMainThread([this, protectedThis = makeRef(*this), function, requestIdentifier]() {
+        (this->*function)(IDBResultData::error(requestIdentifier, IDBError::serverConnectionLostError()));
+    });
+}
+
 void IDBConnectionToServer::deleteDatabase(const IDBRequestData& request)
 {
     LOG(IndexedDB, "IDBConnectionToServer::deleteDatabase - %s", request.databaseIdentifier().debugString().utf8().data());
-    m_delegate->deleteDatabase(request);
+    
+    if (m_serverConnectionIsValid)
+        m_delegate->deleteDatabase(request);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didDeleteDatabase, request.requestIdentifier());
 }
 
 void IDBConnectionToServer::didDeleteDatabase(const IDBResultData& resultData)
@@ -79,7 +90,11 @@
 void IDBConnectionToServer::openDatabase(const IDBRequestData& request)
 {
     LOG(IndexedDB, "IDBConnectionToServer::openDatabase - %s (%s) (%" PRIu64 ")", request.databaseIdentifier().debugString().utf8().data(), request.requestIdentifier().loggingString().utf8().data(), request.requestedVersion());
-    m_delegate->openDatabase(request);
+
+    if (m_serverConnectionIsValid)
+        m_delegate->openDatabase(request);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didOpenDatabase, request.requestIdentifier());
 }
 
 void IDBConnectionToServer::didOpenDatabase(const IDBResultData& resultData)
@@ -93,7 +108,10 @@
     LOG(IndexedDB, "IDBConnectionToServer::createObjectStore");
     ASSERT(isMainThread());
 
-    m_delegate->createObjectStore(requestData, info);
+    if (m_serverConnectionIsValid)
+        m_delegate->createObjectStore(requestData, info);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didCreateObjectStore, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didCreateObjectStore(const IDBResultData& resultData)
@@ -107,7 +125,10 @@
     LOG(IndexedDB, "IDBConnectionToServer::deleteObjectStore");
     ASSERT(isMainThread());
 
-    m_delegate->deleteObjectStore(requestData, objectStoreName);
+    if (m_serverConnectionIsValid)
+        m_delegate->deleteObjectStore(requestData, objectStoreName);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didDeleteObjectStore, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didDeleteObjectStore(const IDBResultData& resultData)
@@ -121,7 +142,10 @@
     LOG(IndexedDB, "IDBConnectionToServer::renameObjectStore");
     ASSERT(isMainThread());
 
-    m_delegate->renameObjectStore(requestData, objectStoreIdentifier, newName);
+    if (m_serverConnectionIsValid)
+        m_delegate->renameObjectStore(requestData, objectStoreIdentifier, newName);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didRenameObjectStore, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didRenameObjectStore(const IDBResultData& resultData)
@@ -135,7 +159,10 @@
     LOG(IndexedDB, "IDBConnectionToServer::clearObjectStore");
     ASSERT(isMainThread());
 
-    m_delegate->clearObjectStore(requestData, objectStoreIdentifier);
+    if (m_serverConnectionIsValid)
+        m_delegate->clearObjectStore(requestData, objectStoreIdentifier);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didClearObjectStore, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didClearObjectStore(const IDBResultData& resultData)
@@ -149,7 +176,10 @@
     LOG(IndexedDB, "IDBConnectionToServer::createIndex");
     ASSERT(isMainThread());
 
-    m_delegate->createIndex(requestData, info);
+    if (m_serverConnectionIsValid)
+        m_delegate->createIndex(requestData, info);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didCreateIndex, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didCreateIndex(const IDBResultData& resultData)
@@ -163,7 +193,10 @@
     LOG(IndexedDB, "IDBConnectionToServer::deleteIndex");
     ASSERT(isMainThread());
 
-    m_delegate->deleteIndex(requestData, objectStoreIdentifier, indexName);
+    if (m_serverConnectionIsValid)
+        m_delegate->deleteIndex(requestData, objectStoreIdentifier, indexName);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didDeleteIndex, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didDeleteIndex(const IDBResultData& resultData)
@@ -177,7 +210,10 @@
     LOG(IndexedDB, "IDBConnectionToServer::renameIndex");
     ASSERT(isMainThread());
 
-    m_delegate->renameIndex(requestData, objectStoreIdentifier, indexIdentifier, newName);
+    if (m_serverConnectionIsValid)
+        m_delegate->renameIndex(requestData, objectStoreIdentifier, indexIdentifier, newName);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didRenameIndex, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didRenameIndex(const IDBResultData& resultData)
@@ -191,7 +227,10 @@
     LOG(IndexedDB, "IDBConnectionToServer::putOrAdd");
     ASSERT(isMainThread());
 
-    m_delegate->putOrAdd(requestData, key, value, overwriteMode);
+    if (m_serverConnectionIsValid)
+        m_delegate->putOrAdd(requestData, key, value, overwriteMode);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didPutOrAdd, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didPutOrAdd(const IDBResultData& resultData)
@@ -206,7 +245,10 @@
     ASSERT(isMainThread());
     ASSERT(!getRecordData.keyRangeData.isNull);
 
-    m_delegate->getRecord(requestData, getRecordData);
+    if (m_serverConnectionIsValid)
+        m_delegate->getRecord(requestData, getRecordData);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didGetRecord, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didGetRecord(const IDBResultData& resultData)
@@ -220,7 +262,10 @@
     LOG(IndexedDB, "IDBConnectionToServer::getAllRecords");
     ASSERT(isMainThread());
 
-    m_delegate->getAllRecords(requestData, getAllRecordsData);
+    if (m_serverConnectionIsValid)
+        m_delegate->getAllRecords(requestData, getAllRecordsData);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didGetAllRecords, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didGetAllRecords(const IDBResultData& resultData)
@@ -235,7 +280,10 @@
     ASSERT(isMainThread());
     ASSERT(!keyRangeData.isNull);
 
-    m_delegate->getCount(requestData, keyRangeData);
+    if (m_serverConnectionIsValid)
+        m_delegate->getCount(requestData, keyRangeData);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didGetCount, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didGetCount(const IDBResultData& resultData)
@@ -250,7 +298,10 @@
     ASSERT(isMainThread());
     ASSERT(!keyRangeData.isNull);
 
-    m_delegate->deleteRecord(requestData, keyRangeData);
+    if (m_serverConnectionIsValid)
+        m_delegate->deleteRecord(requestData, keyRangeData);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didDeleteRecord, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didDeleteRecord(const IDBResultData& resultData)
@@ -264,7 +315,10 @@
     LOG(IndexedDB, "IDBConnectionToServer::openCursor");
     ASSERT(isMainThread());
 
-    m_delegate->openCursor(requestData, info);
+    if (m_serverConnectionIsValid)
+        m_delegate->openCursor(requestData, info);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didOpenCursor, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didOpenCursor(const IDBResultData& resultData)
@@ -278,7 +332,10 @@
     LOG(IndexedDB, "IDBConnectionToServer::iterateCursor");
     ASSERT(isMainThread());
 
-    m_delegate->iterateCursor(requestData, data);
+    if (m_serverConnectionIsValid)
+        m_delegate->iterateCursor(requestData, data);
+    else
+        callResultFunctionWithErrorLater(&IDBConnectionToServer::didIterateCursor, requestData.requestIdentifier());
 }
 
 void IDBConnectionToServer::didIterateCursor(const IDBResultData& resultData)
@@ -292,7 +349,8 @@
     LOG(IndexedDB, "IDBConnectionToServer::establishTransaction");
     ASSERT(isMainThread());
 
-    m_delegate->establishTransaction(databaseConnectionIdentifier, info);
+    if (m_serverConnectionIsValid)
+        m_delegate->establishTransaction(databaseConnectionIdentifier, info);
 }
 
 void IDBConnectionToServer::commitTransaction(const IDBResourceIdentifier& transactionIdentifier)
@@ -300,7 +358,13 @@
     LOG(IndexedDB, "IDBConnectionToServer::commitTransaction");
     ASSERT(isMainThread());
 
-    m_delegate->commitTransaction(transactionIdentifier);
+    if (m_serverConnectionIsValid)
+        m_delegate->commitTransaction(transactionIdentifier);
+    else {
+        callOnMainThread([this, protectedThis = makeRef(*this), transactionIdentifier] {
+            didCommitTransaction(transactionIdentifier, IDBError::serverConnectionLostError());
+        });
+    }
 }
 
 void IDBConnectionToServer::didCommitTransaction(const IDBResourceIdentifier& transactionIdentifier, const IDBError& error)
@@ -316,7 +380,8 @@
     LOG(IndexedDB, "IDBConnectionToServer::didFinishHandlingVersionChangeTransaction");
     ASSERT(isMainThread());
 
-    m_delegate->didFinishHandlingVersionChangeTransaction(databaseConnectionIdentifier, transactionIdentifier);
+    if (m_serverConnectionIsValid)
+        m_delegate->didFinishHandlingVersionChangeTransaction(databaseConnectionIdentifier, transactionIdentifier);
 }
 
 void IDBConnectionToServer::abortTransaction(const IDBResourceIdentifier& transactionIdentifier)
@@ -324,7 +389,13 @@
     LOG(IndexedDB, "IDBConnectionToServer::abortTransaction");
     ASSERT(isMainThread());
 
-    m_delegate->abortTransaction(transactionIdentifier);
+    if (m_serverConnectionIsValid)
+        m_delegate->abortTransaction(transactionIdentifier);
+    else {
+        callOnMainThread([this, protectedThis = makeRef(*this), transactionIdentifier] {
+            didAbortTransaction(transactionIdentifier, IDBError::serverConnectionLostError());
+        });
+    }
 }
 
 void IDBConnectionToServer::didAbortTransaction(const IDBResourceIdentifier& transactionIdentifier, const IDBError& error)
@@ -348,7 +419,8 @@
     LOG(IndexedDB, "IDBConnectionToServer::didFireVersionChangeEvent");
     ASSERT(isMainThread());
 
-    m_delegate->didFireVersionChangeEvent(databaseConnectionIdentifier, requestIdentifier);
+    if (m_serverConnectionIsValid)
+        m_delegate->didFireVersionChangeEvent(databaseConnectionIdentifier, requestIdentifier);
 }
 
 void IDBConnectionToServer::didStartTransaction(const IDBResourceIdentifier& transactionIdentifier, const IDBError& error)
@@ -372,7 +444,8 @@
     LOG(IndexedDB, "IDBConnectionToServer::confirmDidCloseFromServer");
     ASSERT(isMainThread());
 
-    m_delegate->confirmDidCloseFromServer(databaseConnectionIdentifier);
+    if (m_serverConnectionIsValid)
+        m_delegate->confirmDidCloseFromServer(databaseConnectionIdentifier);
 }
 
 void IDBConnectionToServer::connectionToServerLost(const IDBError& error)
@@ -379,7 +452,9 @@
 {
     LOG(IndexedDB, "IDBConnectionToServer::connectionToServerLost");
     ASSERT(isMainThread());
-
+    ASSERT(m_serverConnectionIsValid);
+    
+    m_serverConnectionIsValid = false;
     m_proxy->connectionToServerLost(error);
 }
 
@@ -396,7 +471,8 @@
     LOG(IndexedDB, "IDBConnectionToServer::openDBRequestCancelled");
     ASSERT(isMainThread());
 
-    m_delegate->openDBRequestCancelled(requestData);
+    if (m_serverConnectionIsValid)
+        m_delegate->openDBRequestCancelled(requestData);
 }
 
 void IDBConnectionToServer::databaseConnectionPendingClose(uint64_t databaseConnectionIdentifier)
@@ -404,7 +480,8 @@
     LOG(IndexedDB, "IDBConnectionToServer::databaseConnectionPendingClose");
     ASSERT(isMainThread());
 
-    m_delegate->databaseConnectionPendingClose(databaseConnectionIdentifier);
+    if (m_serverConnectionIsValid)
+        m_delegate->databaseConnectionPendingClose(databaseConnectionIdentifier);
 }
 
 void IDBConnectionToServer::databaseConnectionClosed(uint64_t databaseConnectionIdentifier)
@@ -412,7 +489,8 @@
     LOG(IndexedDB, "IDBConnectionToServer::databaseConnectionClosed");
     ASSERT(isMainThread());
 
-    m_delegate->databaseConnectionClosed(databaseConnectionIdentifier);
+    if (m_serverConnectionIsValid)
+        m_delegate->databaseConnectionClosed(databaseConnectionIdentifier);
 }
 
 void IDBConnectionToServer::abortOpenAndUpgradeNeeded(uint64_t databaseConnectionIdentifier, const IDBResourceIdentifier& transactionIdentifier)
@@ -420,7 +498,8 @@
     LOG(IndexedDB, "IDBConnectionToServer::abortOpenAndUpgradeNeeded");
     ASSERT(isMainThread());
 
-    m_delegate->abortOpenAndUpgradeNeeded(databaseConnectionIdentifier, transactionIdentifier);
+    if (m_serverConnectionIsValid)
+        m_delegate->abortOpenAndUpgradeNeeded(databaseConnectionIdentifier, transactionIdentifier);
 }
 
 void IDBConnectionToServer::getAllDatabaseNames(const SecurityOrigin& mainFrameOrigin, const SecurityOrigin& openingOrigin, Function<void (const Vector<String>&)>&& callback)
@@ -429,7 +508,13 @@
 
     m_getAllDatabaseNamesCallbacks.add(++callbackID, WTFMove(callback));
 
-    m_delegate->getAllDatabaseNames(mainFrameOrigin.data(), openingOrigin.data(), callbackID);
+    if (m_serverConnectionIsValid)
+        m_delegate->getAllDatabaseNames(mainFrameOrigin.data(), openingOrigin.data(), callbackID);
+    else {
+        callOnMainThread([this, protectedThis = makeRef(*this), callbackID = callbackID] {
+            didGetAllDatabaseNames(callbackID, { });
+        });
+    }
 }
 
 void IDBConnectionToServer::didGetAllDatabaseNames(uint64_t callbackID, const Vector<String>& databaseNames)

Modified: trunk/Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.h (233561 => 233562)


--- trunk/Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.h	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.h	2018-07-06 05:48:35 UTC (rev 233562)
@@ -55,7 +55,7 @@
 public:
     WEBCORE_EXPORT static Ref<IDBConnectionToServer> create(IDBConnectionToServerDelegate&);
 
-    uint64_t identifier() const;
+    WEBCORE_EXPORT uint64_t identifier() const;
 
     IDBConnectionProxy& proxy();
 
@@ -143,7 +143,11 @@
 private:
     IDBConnectionToServer(IDBConnectionToServerDelegate&);
 
+    typedef void (IDBConnectionToServer::*ResultFunction)(const IDBResultData&);
+    void callResultFunctionWithErrorLater(ResultFunction, const IDBResourceIdentifier& requestIdentifier);
+    
     Ref<IDBConnectionToServerDelegate> m_delegate;
+    bool m_serverConnectionIsValid { true };
 
     HashMap<uint64_t, WTF::Function<void (const Vector<String>&)>> m_getAllDatabaseNamesCallbacks;

Modified: trunk/Source/WebCore/Modules/indexeddb/shared/IDBError.h (233561 => 233562)


--- trunk/Source/WebCore/Modules/indexeddb/shared/IDBError.h	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebCore/Modules/indexeddb/shared/IDBError.h	2018-07-06 05:48:35 UTC (rev 233562)
@@ -41,6 +41,11 @@
     {
         return IDBError { UnknownError, "Database deleted by request of the user"_s };
     }
+    
+    static IDBError serverConnectionLostError()
+    {
+        return IDBError { UnknownError, "Connection to Indexed Database server lost. Refresh the page to try again"_s };
+    }
 
     WEBCORE_EXPORT IDBError& operator=(const IDBError&);

Modified: trunk/Source/WebCore/page/Page.cpp (233561 => 233562)


--- trunk/Source/WebCore/page/Page.cpp	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebCore/page/Page.cpp	2018-07-06 05:48:35 UTC (rev 233562)
@@ -2196,7 +2196,7 @@
 
 #if ENABLE(INDEXED_DATABASE)
     if (sessionID != m_sessionID)
-        m_idbIDBConnectionToServer = nullptr;
+        m_idbConnectionToServer = nullptr;
 #endif
 
     bool privateBrowsingStateChanged = (sessionID.isEphemeral() != m_sessionID.isEphemeral());
@@ -2320,11 +2320,21 @@
 #if ENABLE(INDEXED_DATABASE)
 IDBClient::IDBConnectionToServer& Page::idbConnection()
 {
-    if (!m_idbIDBConnectionToServer)
-        m_idbIDBConnectionToServer = &databaseProvider().idbConnectionToServerForSession(m_sessionID);
+    if (!m_idbConnectionToServer)
+        m_idbConnectionToServer = &databaseProvider().idbConnectionToServerForSession(m_sessionID);
     
-    return *m_idbIDBConnectionToServer;
+    return *m_idbConnectionToServer;
 }
+
+IDBClient::IDBConnectionToServer* Page::optionalIDBConnection()
+{
+    return m_idbConnectionToServer.get();
+}
+
+void Page::clearIDBConnection()
+{
+    m_idbConnectionToServer = nullptr;
+}
 #endif
 
 #if ENABLE(RESOURCE_USAGE)

Modified: trunk/Source/WebCore/page/Page.h (233561 => 233562)


--- trunk/Source/WebCore/page/Page.h	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebCore/page/Page.h	2018-07-06 05:48:35 UTC (rev 233562)
@@ -606,6 +606,8 @@
 
 #if ENABLE(INDEXED_DATABASE)
     IDBClient::IDBConnectionToServer& idbConnection();
+    WEBCORE_EXPORT IDBClient::IDBConnectionToServer* optionalIDBConnection();
+    WEBCORE_EXPORT void clearIDBConnection();
 #endif
 
     void setShowAllPlugins(bool showAll) { m_showAllPlugins = showAll; }
@@ -821,7 +823,7 @@
 #endif
 
 #if ENABLE(INDEXED_DATABASE)
-    RefPtr<IDBClient::IDBConnectionToServer> m_idbIDBConnectionToServer;
+    RefPtr<IDBClient::IDBConnectionToServer> m_idbConnectionToServer;
 #endif
 
     HashSet<String> m_seenPlugins;

Modified: trunk/Source/WebKit/ChangeLog (233561 => 233562)


--- trunk/Source/WebKit/ChangeLog	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebKit/ChangeLog	2018-07-06 05:48:35 UTC (rev 233562)
@@ -1,3 +1,40 @@
+2018-07-05  Brady Eidson  <[email protected]>
+
+        IndexedDB operations in a Page fail after a StorageProcess crash.
+        <rdar://problem/41626526> and https://bugs.webkit.org/show_bug.cgi?id=187123
+
+        Reviewed by Alex Christensen.
+
+        When the connection to a StorageProcess goes away, explicitly tell all of the WebPages
+        in the WebProcess about it.
+        
+        This puts Documents/Workers in an error mode where requests fail instead of timeout.
+        It also clears the Page's connection so *new* Documents and Workers will get a fresh 
+        new connection that works.
+        
+        * UIProcess/API/C/WKContext.cpp:
+        (WKContextTerminateStorageProcess):
+        
+        * UIProcess/API/Cocoa/WKProcessPool.mm:
+        (-[WKProcessPool _terminateStorageProcess]):
+        
+        * UIProcess/Storage/StorageProcessProxy.cpp:
+        (WebKit::StorageProcessProxy::terminateForTesting):
+        * UIProcess/Storage/StorageProcessProxy.h:
+        
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::terminateStorageProcessForTesting):
+        (WebKit::WebProcessPool::terminateStorageProcess): Deleted.
+        * UIProcess/WebProcessPool.h:
+        
+        * WebProcess/Storage/WebToStorageProcessConnection.cpp:
+        (WebKit::WebToStorageProcessConnection::didClose):
+        * WebProcess/Storage/WebToStorageProcessConnection.h:
+        (WebKit::WebToStorageProcessConnection::existingIDBConnectionToServerForIdentifier):
+        
+        * WebProcess/WebProcess.cpp:
+        (WebKit::WebProcess::webToStorageProcessConnectionClosed):
+
 2018-07-05  Simon Fraser  <[email protected]>
 
         Address two possible causes of missing tiles in iOS Safari, and add logging to gather more data about other possible causes

Modified: trunk/Source/WebKit/UIProcess/API/C/WKContext.cpp (233561 => 233562)


--- trunk/Source/WebKit/UIProcess/API/C/WKContext.cpp	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebKit/UIProcess/API/C/WKContext.cpp	2018-07-06 05:48:35 UTC (rev 233562)
@@ -613,7 +613,7 @@
 
 void WKContextTerminateStorageProcess(WKContextRef context)
 {
-    toImpl(context)->terminateStorageProcess();
+    toImpl(context)->terminateStorageProcessForTesting();
 }
 
 ProcessID WKContextGetNetworkProcessIdentifier(WKContextRef contextRef)

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm (233561 => 233562)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm	2018-07-06 05:48:35 UTC (rev 233562)
@@ -425,7 +425,7 @@
 
 - (void)_terminateStorageProcess
 {
-    _processPool->terminateStorageProcess();
+    _processPool->terminateStorageProcessForTesting();
 }
 
 - (void)_terminateNetworkProcess

Modified: trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp (233561 => 233562)


--- trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp	2018-07-06 05:48:35 UTC (rev 233562)
@@ -65,6 +65,24 @@
     ASSERT(m_pendingDeleteWebsiteDataForOriginsCallbacks.isEmpty());
 }
 
+void StorageProcessProxy::terminateForTesting()
+{
+    for (auto& callback : m_pendingFetchWebsiteDataCallbacks.values())
+        callback({ });
+
+    for (auto& callback : m_pendingDeleteWebsiteDataCallbacks.values())
+        callback();
+
+    for (auto& callback : m_pendingDeleteWebsiteDataForOriginsCallbacks.values())
+        callback();
+    
+    m_pendingFetchWebsiteDataCallbacks.clear();
+    m_pendingDeleteWebsiteDataCallbacks.clear();
+    m_pendingDeleteWebsiteDataForOriginsCallbacks.clear();
+    
+    terminate();
+}
+
 void StorageProcessProxy::getLaunchOptions(ProcessLauncher::LaunchOptions& launchOptions)
 {
     launchOptions.processType = ProcessLauncher::ProcessType::Storage;

Modified: trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.h (233561 => 233562)


--- trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.h	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.h	2018-07-06 05:48:35 UTC (rev 233562)
@@ -58,6 +58,8 @@
 
     void getStorageProcessConnection(WebProcessProxy&, Messages::WebProcessProxy::GetStorageProcessConnection::DelayedReply&&);
 
+    void terminateForTesting();
+
 private:
     StorageProcessProxy(WebProcessPool&);

Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (233561 => 233562)


--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2018-07-06 05:48:35 UTC (rev 233562)
@@ -1602,12 +1602,12 @@
         m_networkProcess->send(Messages::NetworkProcess::ClearCachedCredentials(), 0);
 }
 
-void WebProcessPool::terminateStorageProcess()
+void WebProcessPool::terminateStorageProcessForTesting()
 {
     if (!m_storageProcess)
         return;
 
-    m_storageProcess->terminate();
+    m_storageProcess->terminateForTesting();
     m_storageProcess = nullptr;
 }

Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (233561 => 233562)


--- trunk/Source/WebKit/UIProcess/WebProcessPool.h	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h	2018-07-06 05:48:35 UTC (rev 233562)
@@ -272,7 +272,7 @@
     void setAllowsAnySSLCertificateForWebSocket(bool);
 
     void clearCachedCredentials();
-    void terminateStorageProcess();
+    void terminateStorageProcessForTesting();
     void terminateNetworkProcess();
     void terminateServiceWorkerProcesses();
     void disableServiceWorkerProcessTerminationDelay();

Modified: trunk/Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.cpp (233561 => 233562)


--- trunk/Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.cpp	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.cpp	2018-07-06 05:48:35 UTC (rev 233562)
@@ -101,12 +101,11 @@
 
 void WebToStorageProcessConnection::didClose(IPC::Connection& connection)
 {
+    auto protectedThis = makeRef(*this);
+
 #if ENABLE(INDEXED_DATABASE)
     for (auto& connection : m_webIDBConnectionsByIdentifier.values())
         connection->connectionToServerLost();
-
-    m_webIDBConnectionsByIdentifier.clear();
-    m_webIDBConnectionsBySession.clear();
 #endif
 #if ENABLE(SERVICE_WORKER)
     for (auto& connection : m_swConnectionsBySession.values())
@@ -117,6 +116,11 @@
 #endif
 
     WebProcess::singleton().webToStorageProcessConnectionClosed(this);
+
+#if ENABLE(INDEXED_DATABASE)
+    m_webIDBConnectionsByIdentifier.clear();
+    m_webIDBConnectionsBySession.clear();
+#endif
 }
 
 void WebToStorageProcessConnection::didReceiveInvalidMessage(IPC::Connection&, IPC::StringReference messageReceiverName, IPC::StringReference messageName)

Modified: trunk/Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.h (233561 => 233562)


--- trunk/Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.h	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.h	2018-07-06 05:48:35 UTC (rev 233562)
@@ -51,6 +51,7 @@
     IPC::Connection& connection() { return m_connection.get(); }
 
 #if ENABLE(INDEXED_DATABASE)
+    WebIDBConnectionToServer* existingIDBConnectionToServerForIdentifier(uint64_t identifier) { return m_webIDBConnectionsByIdentifier.get(identifier); };
     WebIDBConnectionToServer& idbConnectionToServerForSession(PAL::SessionID);
 #endif
 #if ENABLE(SERVICE_WORKER)

Modified: trunk/Source/WebKit/WebProcess/WebProcess.cpp (233561 => 233562)


--- trunk/Source/WebKit/WebProcess/WebProcess.cpp	2018-07-06 04:56:40 UTC (rev 233561)
+++ trunk/Source/WebKit/WebProcess/WebProcess.cpp	2018-07-06 05:48:35 UTC (rev 233562)
@@ -1179,8 +1179,18 @@
     ASSERT(m_webToStorageProcessConnection);
     ASSERT(m_webToStorageProcessConnection == connection);
 
-    m_webToStorageProcessConnection = nullptr;
+#if ENABLE(INDEXED_DATABASE)
+    for (auto& page : m_pageMap.values()) {
+        auto idbConnection = page->corePage()->optionalIDBConnection();
+        if (!idbConnection)
+            continue;
 
+        if (connection->existingIDBConnectionToServerForIdentifier(idbConnection->identifier())) {
+            ASSERT(idbConnection == &connection->existingIDBConnectionToServerForIdentifier(idbConnection->identifier())->coreConnectionToServer());
+            page->corePage()->clearIDBConnection();
+        }
+    }
+#endif
 #if ENABLE(SERVICE_WORKER)
     if (SWContextManager::singleton().connection()) {
         RELEASE_LOG(ServiceWorker, "Service worker process is exiting because its storage process is gone");
@@ -1187,6 +1197,8 @@
         _exit(EXIT_SUCCESS);
     }
 #endif
+
+    m_webToStorageProcessConnection = nullptr;
 }
 
 WebToStorageProcessConnection& WebProcess::ensureWebToStorageProcessConnection(PAL::SessionID initialSessionID)

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to