Title: [252064] trunk
Revision
252064
Author
cdu...@apple.com
Date
2019-11-05 11:15:18 -0800 (Tue, 05 Nov 2019)

Log Message

DatabaseContext should not prevent entering the back/forward cache
https://bugs.webkit.org/show_bug.cgi?id=203103
<rdar://problem/56592193>

Reviewed by Geoffrey Garen.

Source/WebCore:

Let pages with active webdatabase transactions into the back/forward cache. We make sure
to queue tasks that run script to the Window event loop, so that they get delayed when
the document is suspended.

This patch also makes sure that the transaction's error callback gets called if the database
gets closed while the transaction is going on. We previously did not happen and it was causing
issues because databases get closed on navigation.

No new tests, updated existing test.

* Modules/webdatabase/Database.cpp:
(WebCore::Database::runTransaction):
* Modules/webdatabase/DatabaseContext.cpp:
(WebCore::DatabaseContext::shouldPreventEnteringBackForwardCache_DEPRECATED const): Deleted.
* Modules/webdatabase/DatabaseContext.h:
* Modules/webdatabase/DatabaseManager.cpp:
(WebCore::DatabaseManager::openDatabase):
* Modules/webdatabase/SQLTransaction.cpp:
(WebCore::SQLTransaction::performPendingCallback):
(WebCore::SQLTransaction::notifyDatabaseThreadIsShuttingDown):
(WebCore::SQLTransaction::callErrorCallbackDueToInterruption):
(WebCore::SQLTransaction::checkAndHandleClosedDatabase):
(WebCore::SQLTransaction::deliverTransactionErrorCallback):
(WebCore::SQLTransaction::deliverSuccessCallback):
(WebCore::SQLTransaction::computeNextStateAndCleanupIfNeeded):
* Modules/webdatabase/SQLTransaction.h:

LayoutTests:

* fast/history/page-cache-webdatabase-pending-transaction-expected.txt:
* fast/history/page-cache-webdatabase-pending-transaction.html:
Update existing test to reflect behavior change.

* platform/gtk/TestExpectations:
* platform/mac/TestExpectations:
Unmark test as flaky.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (252063 => 252064)


--- trunk/LayoutTests/ChangeLog	2019-11-05 18:53:13 UTC (rev 252063)
+++ trunk/LayoutTests/ChangeLog	2019-11-05 19:15:18 UTC (rev 252064)
@@ -1,3 +1,19 @@
+2019-11-05  Chris Dumez  <cdu...@apple.com>
+
+        DatabaseContext should not prevent entering the back/forward cache
+        https://bugs.webkit.org/show_bug.cgi?id=203103
+        <rdar://problem/56592193>
+
+        Reviewed by Geoffrey Garen.
+
+        * fast/history/page-cache-webdatabase-pending-transaction-expected.txt:
+        * fast/history/page-cache-webdatabase-pending-transaction.html:
+        Update existing test to reflect behavior change.
+
+        * platform/gtk/TestExpectations:
+        * platform/mac/TestExpectations:
+        Unmark test as flaky.
+
 2019-11-05  Wenson Hsieh  <wenson_hs...@apple.com>
 
         Native text substitutions interfere with HTML <datalist> options resulting in crash

Modified: trunk/LayoutTests/fast/history/page-cache-webdatabase-pending-transaction-expected.txt (252063 => 252064)


--- trunk/LayoutTests/fast/history/page-cache-webdatabase-pending-transaction-expected.txt	2019-11-05 18:53:13 UTC (rev 252063)
+++ trunk/LayoutTests/fast/history/page-cache-webdatabase-pending-transaction-expected.txt	2019-11-05 19:15:18 UTC (rev 252064)
@@ -1,10 +1,14 @@
-Tests that a page with an open WebDatabase that has pending transactions does not go into the page cache.
+CONSOLE MESSAGE: line 54: Web SQL is deprecated. Please use IndexedDB instead.
+Tests that a page with an open WebDatabase that has pending transactions is able to go into the back/forward cache.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
 pageshow - not from cache
-PASS Page was not restored from page cache
+pagehide - entering cache
+pageshow - from cache
+PASS Page did enter and was restored from the back/forward cache
+PASS All done
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/fast/history/page-cache-webdatabase-pending-transaction.html (252063 => 252064)


--- trunk/LayoutTests/fast/history/page-cache-webdatabase-pending-transaction.html	2019-11-05 18:53:13 UTC (rev 252063)
+++ trunk/LayoutTests/fast/history/page-cache-webdatabase-pending-transaction.html	2019-11-05 19:15:18 UTC (rev 252064)
@@ -2,58 +2,75 @@
 <!DOCTYPE html>
 <html>
 <body>
-<script src=""
+<script src=""
 <script>
-description('Tests that a page with an open WebDatabase that has pending transactions does not go into the page cache.');
-window.jsTestIsAsync = true;
+description('Tests that a page with an open WebDatabase that has pending transactions is able to go into the back/forward cache.');
+jsTestIsAsync = true;
+let restoredFromCache = false;
+let pendingTransactionCount = 0;
 
 if (window.testRunner)
     testRunner.clearAllDatabases();
 
+function checkTestComplete()
+{
+    if (!pendingTransactionCount && restoredFromCache) {
+        testPassed("All done");
+        finishJSTest();
+    }
+}
+
 window.addEventListener("pageshow", function(event) {
     debug("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
-    if (!window.sessionStorage.page_cache_open_webdatabase_test_started)
-        return;
 
-    delete window.sessionStorage.page_cache_open_webdatabase_test_started;
-
-    if (event.persisted)
-        testFailed("Page did enter and was restored from the page cache");
-    else
-        testPassed("Page was not restored from page cache");
-    finishJSTest();
+    if (event.persisted) {
+        testPassed("Page did enter and was restored from the back/forward cache");
+        restoredFromCache = true;
+        checkTestComplete();
+    }
 }, false);
 
 window.addEventListener("pagehide", function(event) {
     debug("pagehide - " + (event.persisted ? "" : "not ") + "entering cache");
-    if (event.persisted) {
-        testFailed("Page entered the page cache.");
+    if (!event.persisted) {
+        testFailed("Page failed to enter the back/forward cache.");
         finishJSTest();
     }
 }, false);
 
+function handleTransactionComplete()
+{
+    if (!pendingTransactionCount) {
+        testFailed("Too many completion handlers");
+        finishJSTest();
+    }
+
+    pendingTransactionCount--;
+    checkTestComplete();
+}
+
 window.addEventListener('load', function() {
-    // Open the database.
-    db = openDatabase("PageCacheTest", "", "Page Cache Test", 32768);
+    setTimeout(() => {
+        db = openDatabase("PageCacheTest", "", "Back Forward Cache Test", 32768);
 
-    db.transaction(function(tx) {
-        // Force a back navigation back to this page.
-        window.sessionStorage.page_cache_open_webdatabase_test_started = true;
-        window.location.href = ""
+        pendingTransactionCount++;
+        db.transaction(function(tx) {
+            window.location.href = ""
 
-        tx.executeSql('CREATE TABLE IF NOT EXISTS LOGS (id unique, log)');
-    });
+            tx.executeSql('CREATE TABLE IF NOT EXISTS LOGS (id unique, log)');
+        }, handleTransactionComplete, handleTransactionComplete);
 
-    db.transaction(function(tx) {
-        tx.executeSql('CREATE TABLE IF NOT EXISTS LOGS2 (id unique, log)');
-    });
+        pendingTransactionCount++;
+        db.transaction(function(tx) {
+            tx.executeSql('CREATE TABLE IF NOT EXISTS LOGS2 (id unique, log)');
+        }, handleTransactionComplete, handleTransactionComplete);
 
-    db.transaction(function(tx) {
-        tx.executeSql('CREATE TABLE IF NOT EXISTS LOGS3 (id unique, log)');
-    });
+        pendingTransactionCount++;
+        db.transaction(function(tx) {
+            tx.executeSql('CREATE TABLE IF NOT EXISTS LOGS3 (id unique, log)');
+        }, handleTransactionComplete, handleTransactionComplete);
+    }, 0);
 }, false);
-
 </script>
-<script src=""
 </body>
 </html>

Modified: trunk/LayoutTests/platform/gtk/TestExpectations (252063 => 252064)


--- trunk/LayoutTests/platform/gtk/TestExpectations	2019-11-05 18:53:13 UTC (rev 252063)
+++ trunk/LayoutTests/platform/gtk/TestExpectations	2019-11-05 19:15:18 UTC (rev 252064)
@@ -1659,7 +1659,6 @@
 webkit.org/b/144864 fast/events/clear-drag-state.html [ Failure Pass ]
 
 webkit.org/b/145051 media/video-rtl.html [ ImageOnlyFailure Pass ]
-webkit.org/b/145052 fast/history/page-cache-webdatabase-pending-transaction.html [ Failure Pass ]
 
 webkit.org/b/145167 transforms/2d/perspective-not-fixed-container.html [ ImageOnlyFailure Pass ]
 

Modified: trunk/LayoutTests/platform/mac/TestExpectations (252063 => 252064)


--- trunk/LayoutTests/platform/mac/TestExpectations	2019-11-05 18:53:13 UTC (rev 252063)
+++ trunk/LayoutTests/platform/mac/TestExpectations	2019-11-05 19:15:18 UTC (rev 252064)
@@ -1198,8 +1198,6 @@
 # rdar://problem/27141291
 [ Sierra+ ] editing/selection/triple-click-in-pre.html [ Failure ]
 
-webkit.org/b/159379 fast/history/page-cache-webdatabase-pending-transaction.html [ Pass Failure ]
-
 # rdar://problem/31243824
 webkit.org/b/158747 media/restore-from-page-cache.html [ Pass Failure Crash ]
 

Modified: trunk/Source/WebCore/ChangeLog (252063 => 252064)


--- trunk/Source/WebCore/ChangeLog	2019-11-05 18:53:13 UTC (rev 252063)
+++ trunk/Source/WebCore/ChangeLog	2019-11-05 19:15:18 UTC (rev 252064)
@@ -1,3 +1,38 @@
+2019-11-05  Chris Dumez  <cdu...@apple.com>
+
+        DatabaseContext should not prevent entering the back/forward cache
+        https://bugs.webkit.org/show_bug.cgi?id=203103
+        <rdar://problem/56592193>
+
+        Reviewed by Geoffrey Garen.
+
+        Let pages with active webdatabase transactions into the back/forward cache. We make sure
+        to queue tasks that run script to the Window event loop, so that they get delayed when
+        the document is suspended.
+
+        This patch also makes sure that the transaction's error callback gets called if the database
+        gets closed while the transaction is going on. We previously did not happen and it was causing
+        issues because databases get closed on navigation.
+
+        No new tests, updated existing test.
+
+        * Modules/webdatabase/Database.cpp:
+        (WebCore::Database::runTransaction):
+        * Modules/webdatabase/DatabaseContext.cpp:
+        (WebCore::DatabaseContext::shouldPreventEnteringBackForwardCache_DEPRECATED const): Deleted.
+        * Modules/webdatabase/DatabaseContext.h:
+        * Modules/webdatabase/DatabaseManager.cpp:
+        (WebCore::DatabaseManager::openDatabase):
+        * Modules/webdatabase/SQLTransaction.cpp:
+        (WebCore::SQLTransaction::performPendingCallback):
+        (WebCore::SQLTransaction::notifyDatabaseThreadIsShuttingDown):
+        (WebCore::SQLTransaction::callErrorCallbackDueToInterruption):
+        (WebCore::SQLTransaction::checkAndHandleClosedDatabase):
+        (WebCore::SQLTransaction::deliverTransactionErrorCallback):
+        (WebCore::SQLTransaction::deliverSuccessCallback):
+        (WebCore::SQLTransaction::computeNextStateAndCleanupIfNeeded):
+        * Modules/webdatabase/SQLTransaction.h:
+
 2019-11-05  Sihui Liu  <sihui_...@apple.com>
 
         REGRESSION (r250754): web page using IDBIndex doesn't load occasionally

Modified: trunk/Source/WebCore/Modules/webdatabase/Database.cpp (252063 => 252064)


--- trunk/Source/WebCore/Modules/webdatabase/Database.cpp	2019-11-05 18:53:13 UTC (rev 252063)
+++ trunk/Source/WebCore/Modules/webdatabase/Database.cpp	2019-11-05 19:15:18 UTC (rev 252064)
@@ -52,6 +52,7 @@
 #include "ScriptExecutionContext.h"
 #include "SecurityOrigin.h"
 #include "VoidCallback.h"
+#include "WindowEventLoop.h"
 #include <wtf/NeverDestroyed.h>
 #include <wtf/RefPtr.h>
 #include <wtf/StdLibExtras.h>
@@ -684,10 +685,11 @@
 
 void Database::runTransaction(RefPtr<SQLTransactionCallback>&& callback, RefPtr<SQLTransactionErrorCallback>&& errorCallback, RefPtr<VoidCallback>&& successCallback, RefPtr<SQLTransactionWrapper>&& wrapper, bool readOnly)
 {
+    ASSERT(isMainThread());
     LockHolder locker(m_transactionInProgressMutex);
     if (!m_isTransactionQueueEnabled) {
         if (errorCallback) {
-            callOnMainThread([errorCallback = makeRef(*errorCallback)]() {
+            m_document->eventLoop().queueTask(TaskSource::Networking, m_document, [errorCallback = makeRef(*errorCallback)]() {
                 errorCallback->handleEvent(SQLError::create(SQLError::UNKNOWN_ERR, "database has been closed"));
             });
         }

Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.cpp (252063 => 252064)


--- trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.cpp	2019-11-05 18:53:13 UTC (rev 252063)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.cpp	2019-11-05 19:15:18 UTC (rev 252064)
@@ -130,15 +130,6 @@
     stopDatabases();
 }
 
-// FIXME: This should never prevent entering the back/forward cache.
-bool DatabaseContext::shouldPreventEnteringBackForwardCache_DEPRECATED() const
-{
-    if (!hasOpenDatabases() || !m_databaseThread)
-        return false;
-
-    return m_databaseThread->hasPendingDatabaseActivity();
-}
-
 DatabaseThread* DatabaseContext::databaseThread()
 {
     if (!m_databaseThread && !m_hasOpenDatabases) {

Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.h (252063 => 252064)


--- trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.h	2019-11-05 18:53:13 UTC (rev 252063)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.h	2019-11-05 19:15:18 UTC (rev 252064)
@@ -73,7 +73,6 @@
 
     void contextDestroyed() override;
     void stop() override;
-    bool shouldPreventEnteringBackForwardCache_DEPRECATED() const override;
     const char* activeDOMObjectName() const override { return "DatabaseContext"; }
 
     RefPtr<DatabaseThread> m_databaseThread;

Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseManager.cpp (252063 => 252064)


--- trunk/Source/WebCore/Modules/webdatabase/DatabaseManager.cpp	2019-11-05 18:53:13 UTC (rev 252063)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseManager.cpp	2019-11-05 19:15:18 UTC (rev 252064)
@@ -38,6 +38,7 @@
 #include "ScriptController.h"
 #include "SecurityOrigin.h"
 #include "SecurityOriginData.h"
+#include "WindowEventLoop.h"
 #include <wtf/NeverDestroyed.h>
 
 namespace WebCore {
@@ -192,6 +193,7 @@
 
 ExceptionOr<Ref<Database>> DatabaseManager::openDatabase(Document& document, const String& name, const String& expectedVersion, const String& displayName, unsigned estimatedSize, RefPtr<DatabaseCallback>&& creationCallback)
 {
+    ASSERT(isMainThread());
     ScriptController::initializeThreading();
 
     bool setVersionInNewDatabase = !creationCallback;
@@ -208,7 +210,7 @@
     if (database->isNew() && creationCallback.get()) {
         LOG(StorageAPI, "Scheduling DatabaseCreationCallbackTask for database %p\n", database.get());
         database->setHasPendingCreationEvent(true);
-        database->m_document->postTask([creationCallback, database] (ScriptExecutionContext&) {
+        database->m_document->eventLoop().queueTask(TaskSource::Networking, database->m_document, [creationCallback, database]() {
             creationCallback->handleEvent(*database);
             database->setHasPendingCreationEvent(false);
         });

Modified: trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.cpp (252063 => 252064)


--- trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.cpp	2019-11-05 18:53:13 UTC (rev 252063)
+++ trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.cpp	2019-11-05 19:15:18 UTC (rev 252064)
@@ -47,6 +47,7 @@
 #include "SQLTransactionErrorCallback.h"
 #include "SQLiteTransaction.h"
 #include "VoidCallback.h"
+#include "WindowEventLoop.h"
 #include <wtf/Optional.h>
 #include <wtf/StdLibExtras.h>
 #include <wtf/Vector.h>
@@ -109,6 +110,7 @@
 
 void SQLTransaction::performPendingCallback()
 {
+    ASSERT(isMainThread());
     LOG(StorageAPI, "Callback %s\n", debugStepName(m_nextStep));
 
     ASSERT(m_nextStep == &SQLTransaction::deliverTransactionCallback
@@ -125,9 +127,25 @@
 
 void SQLTransaction::notifyDatabaseThreadIsShuttingDown()
 {
+    callOnMainThread([this, protectedThis = makeRef(*this)]() mutable {
+        callErrorCallbackDueToInterruption();
+    });
+
     m_backend.notifyDatabaseThreadIsShuttingDown();
 }
 
+void SQLTransaction::callErrorCallbackDueToInterruption()
+{
+    ASSERT(isMainThread());
+    auto errorCallback = m_errorCallbackWrapper.unwrap();
+    if (!errorCallback)
+        return;
+
+    m_database->document().eventLoop().queueTask(TaskSource::Networking, m_database->document(), [errorCallback = WTFMove(errorCallback)]() mutable {
+        errorCallback->handleEvent(SQLError::create(SQLError::DATABASE_ERR, "the database was closed"));
+    });
+}
+
 void SQLTransaction::enqueueStatement(std::unique_ptr<SQLStatement> statement)
 {
     LockHolder locker(m_statementMutex);
@@ -180,6 +198,8 @@
     m_statementQueue.clear();
     m_nextStep = nullptr;
 
+    callErrorCallbackDueToInterruption();
+
     // Release the unneeded callbacks, to break reference cycles.
     m_callbackWrapper.clear();
     m_successCallbackWrapper.clear();
@@ -394,8 +414,11 @@
     // Spec 4.3.2.10: If exists, invoke error callback with the last
     // error to have occurred in this transaction.
     RefPtr<SQLTransactionErrorCallback> errorCallback = m_errorCallbackWrapper.unwrap();
-    if (errorCallback)
-        errorCallback->handleEvent(*m_transactionError);
+    if (errorCallback) {
+        m_database->document().eventLoop().queueTask(TaskSource::Networking, m_database->document(), [errorCallback = WTFMove(errorCallback), transactionError = m_transactionError]() mutable {
+            errorCallback->handleEvent(*transactionError);
+        });
+    }
 
     clearCallbackWrappers();
 
@@ -442,8 +465,11 @@
 {
     // Spec 4.3.2.8: Deliver success callback.
     RefPtr<VoidCallback> successCallback = m_successCallbackWrapper.unwrap();
-    if (successCallback)
-        successCallback->handleEvent();
+    if (successCallback) {
+        m_database->document().eventLoop().queueTask(TaskSource::Networking, m_database->document(), [successCallback = WTFMove(successCallback)]() mutable {
+            successCallback->handleEvent();
+        });
+    }
 
     clearCallbackWrappers();
 
@@ -475,7 +501,8 @@
 
         LOG(StorageAPI, "Callback %s\n", nameForSQLTransactionState(m_nextState));
         return;
-    }
+    } else
+        callErrorCallbackDueToInterruption();
 
     clearCallbackWrappers();
     m_backend.requestTransitToState(SQLTransactionState::CleanupAndTerminate);

Modified: trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.h (252063 => 252064)


--- trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.h	2019-11-05 18:53:13 UTC (rev 252063)
+++ trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.h	2019-11-05 19:15:18 UTC (rev 252064)
@@ -105,6 +105,8 @@
 
     NO_RETURN_DUE_TO_ASSERT void unreachableState();
 
+    void callErrorCallbackDueToInterruption();
+
     void getNextStatement();
     bool runCurrentStatement();
     void handleCurrentStatementError();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to