Changeset 252064 in webkit


Ignore:
Timestamp:
Nov 5, 2019 11:15:18 AM (4 years ago)
Author:
Chris Dumez
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.

Location:
trunk
Files:
12 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r252062 r252064  
     12019-11-05  Chris Dumez  <cdumez@apple.com>
     2
     3        DatabaseContext should not prevent entering the back/forward cache
     4        https://bugs.webkit.org/show_bug.cgi?id=203103
     5        <rdar://problem/56592193>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        * fast/history/page-cache-webdatabase-pending-transaction-expected.txt:
     10        * fast/history/page-cache-webdatabase-pending-transaction.html:
     11        Update existing test to reflect behavior change.
     12
     13        * platform/gtk/TestExpectations:
     14        * platform/mac/TestExpectations:
     15        Unmark test as flaky.
     16
    1172019-11-05  Wenson Hsieh  <wenson_hsieh@apple.com>
    218
  • trunk/LayoutTests/fast/history/page-cache-webdatabase-pending-transaction-expected.txt

    r251592 r252064  
    1 Tests that a page with an open WebDatabase that has pending transactions does not go into the page cache.
     1CONSOLE MESSAGE: line 54: Web SQL is deprecated. Please use IndexedDB instead.
     2Tests that a page with an open WebDatabase that has pending transactions is able to go into the back/forward cache.
    23
    34On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
     
    56
    67pageshow - not from cache
    7 PASS Page was not restored from page cache
     8pagehide - entering cache
     9pageshow - from cache
     10PASS Page did enter and was restored from the back/forward cache
     11PASS All done
    812PASS successfullyParsed is true
    913
  • trunk/LayoutTests/fast/history/page-cache-webdatabase-pending-transaction.html

    r251592 r252064  
    33<html>
    44<body>
    5 <script src="../../resources/js-test-pre.js"></script>
     5<script src="../../resources/js-test.js"></script>
    66<script>
    7 description('Tests that a page with an open WebDatabase that has pending transactions does not go into the page cache.');
    8 window.jsTestIsAsync = true;
     7description('Tests that a page with an open WebDatabase that has pending transactions is able to go into the back/forward cache.');
     8jsTestIsAsync = true;
     9let restoredFromCache = false;
     10let pendingTransactionCount = 0;
    911
    1012if (window.testRunner)
    1113    testRunner.clearAllDatabases();
    1214
     15function checkTestComplete()
     16{
     17    if (!pendingTransactionCount && restoredFromCache) {
     18        testPassed("All done");
     19        finishJSTest();
     20    }
     21}
     22
    1323window.addEventListener("pageshow", function(event) {
    1424    debug("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
    15     if (!window.sessionStorage.page_cache_open_webdatabase_test_started)
    16         return;
    1725
    18     delete window.sessionStorage.page_cache_open_webdatabase_test_started;
    19 
    20     if (event.persisted)
    21         testFailed("Page did enter and was restored from the page cache");
    22     else
    23         testPassed("Page was not restored from page cache");
    24     finishJSTest();
     26    if (event.persisted) {
     27        testPassed("Page did enter and was restored from the back/forward cache");
     28        restoredFromCache = true;
     29        checkTestComplete();
     30    }
    2531}, false);
    2632
    2733window.addEventListener("pagehide", function(event) {
    2834    debug("pagehide - " + (event.persisted ? "" : "not ") + "entering cache");
    29     if (event.persisted) {
    30         testFailed("Page entered the page cache.");
     35    if (!event.persisted) {
     36        testFailed("Page failed to enter the back/forward cache.");
    3137        finishJSTest();
    3238    }
    3339}, false);
    3440
     41function handleTransactionComplete()
     42{
     43    if (!pendingTransactionCount) {
     44        testFailed("Too many completion handlers");
     45        finishJSTest();
     46    }
     47
     48    pendingTransactionCount--;
     49    checkTestComplete();
     50}
     51
    3552window.addEventListener('load', function() {
    36     // Open the database.
    37     db = openDatabase("PageCacheTest", "", "Page Cache Test", 32768);
     53    setTimeout(() => {
     54        db = openDatabase("PageCacheTest", "", "Back Forward Cache Test", 32768);
    3855
    39     db.transaction(function(tx) {
    40         // Force a back navigation back to this page.
    41         window.sessionStorage.page_cache_open_webdatabase_test_started = true;
    42         window.location.href = "resources/page-cache-helper.html";
     56        pendingTransactionCount++;
     57        db.transaction(function(tx) {
     58            window.location.href = "resources/page-cache-helper.html";
    4359
    44         tx.executeSql('CREATE TABLE IF NOT EXISTS LOGS (id unique, log)');
    45     });
     60            tx.executeSql('CREATE TABLE IF NOT EXISTS LOGS (id unique, log)');
     61        }, handleTransactionComplete, handleTransactionComplete);
    4662
    47     db.transaction(function(tx) {
    48         tx.executeSql('CREATE TABLE IF NOT EXISTS LOGS2 (id unique, log)');
    49     });
     63        pendingTransactionCount++;
     64        db.transaction(function(tx) {
     65            tx.executeSql('CREATE TABLE IF NOT EXISTS LOGS2 (id unique, log)');
     66        }, handleTransactionComplete, handleTransactionComplete);
    5067
    51     db.transaction(function(tx) {
    52         tx.executeSql('CREATE TABLE IF NOT EXISTS LOGS3 (id unique, log)');
    53     });
     68        pendingTransactionCount++;
     69        db.transaction(function(tx) {
     70            tx.executeSql('CREATE TABLE IF NOT EXISTS LOGS3 (id unique, log)');
     71        }, handleTransactionComplete, handleTransactionComplete);
     72    }, 0);
    5473}, false);
    55 
    5674</script>
    57 <script src="../../resources/js-test-post.js"></script>
    5875</body>
    5976</html>
  • trunk/LayoutTests/platform/gtk/TestExpectations

    r252027 r252064  
    16601660
    16611661webkit.org/b/145051 media/video-rtl.html [ ImageOnlyFailure Pass ]
    1662 webkit.org/b/145052 fast/history/page-cache-webdatabase-pending-transaction.html [ Failure Pass ]
    16631662
    16641663webkit.org/b/145167 transforms/2d/perspective-not-fixed-container.html [ ImageOnlyFailure Pass ]
  • trunk/LayoutTests/platform/mac/TestExpectations

    r251950 r252064  
    11991199[ Sierra+ ] editing/selection/triple-click-in-pre.html [ Failure ]
    12001200
    1201 webkit.org/b/159379 fast/history/page-cache-webdatabase-pending-transaction.html [ Pass Failure ]
    1202 
    12031201# rdar://problem/31243824
    12041202webkit.org/b/158747 media/restore-from-page-cache.html [ Pass Failure Crash ]
  • trunk/Source/WebCore/ChangeLog

    r252060 r252064  
     12019-11-05  Chris Dumez  <cdumez@apple.com>
     2
     3        DatabaseContext should not prevent entering the back/forward cache
     4        https://bugs.webkit.org/show_bug.cgi?id=203103
     5        <rdar://problem/56592193>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        Let pages with active webdatabase transactions into the back/forward cache. We make sure
     10        to queue tasks that run script to the Window event loop, so that they get delayed when
     11        the document is suspended.
     12
     13        This patch also makes sure that the transaction's error callback gets called if the database
     14        gets closed while the transaction is going on. We previously did not happen and it was causing
     15        issues because databases get closed on navigation.
     16
     17        No new tests, updated existing test.
     18
     19        * Modules/webdatabase/Database.cpp:
     20        (WebCore::Database::runTransaction):
     21        * Modules/webdatabase/DatabaseContext.cpp:
     22        (WebCore::DatabaseContext::shouldPreventEnteringBackForwardCache_DEPRECATED const): Deleted.
     23        * Modules/webdatabase/DatabaseContext.h:
     24        * Modules/webdatabase/DatabaseManager.cpp:
     25        (WebCore::DatabaseManager::openDatabase):
     26        * Modules/webdatabase/SQLTransaction.cpp:
     27        (WebCore::SQLTransaction::performPendingCallback):
     28        (WebCore::SQLTransaction::notifyDatabaseThreadIsShuttingDown):
     29        (WebCore::SQLTransaction::callErrorCallbackDueToInterruption):
     30        (WebCore::SQLTransaction::checkAndHandleClosedDatabase):
     31        (WebCore::SQLTransaction::deliverTransactionErrorCallback):
     32        (WebCore::SQLTransaction::deliverSuccessCallback):
     33        (WebCore::SQLTransaction::computeNextStateAndCleanupIfNeeded):
     34        * Modules/webdatabase/SQLTransaction.h:
     35
    1362019-11-05  Sihui Liu  <sihui_liu@apple.com>
    237
  • trunk/Source/WebCore/Modules/webdatabase/Database.cpp

    r251592 r252064  
    5353#include "SecurityOrigin.h"
    5454#include "VoidCallback.h"
     55#include "WindowEventLoop.h"
    5556#include <wtf/NeverDestroyed.h>
    5657#include <wtf/RefPtr.h>
     
    685686void Database::runTransaction(RefPtr<SQLTransactionCallback>&& callback, RefPtr<SQLTransactionErrorCallback>&& errorCallback, RefPtr<VoidCallback>&& successCallback, RefPtr<SQLTransactionWrapper>&& wrapper, bool readOnly)
    686687{
     688    ASSERT(isMainThread());
    687689    LockHolder locker(m_transactionInProgressMutex);
    688690    if (!m_isTransactionQueueEnabled) {
    689691        if (errorCallback) {
    690             callOnMainThread([errorCallback = makeRef(*errorCallback)]() {
     692            m_document->eventLoop().queueTask(TaskSource::Networking, m_document, [errorCallback = makeRef(*errorCallback)]() {
    691693                errorCallback->handleEvent(SQLError::create(SQLError::UNKNOWN_ERR, "database has been closed"));
    692694            });
  • trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.cpp

    r251592 r252064  
    131131}
    132132
    133 // FIXME: This should never prevent entering the back/forward cache.
    134 bool DatabaseContext::shouldPreventEnteringBackForwardCache_DEPRECATED() const
    135 {
    136     if (!hasOpenDatabases() || !m_databaseThread)
    137         return false;
    138 
    139     return m_databaseThread->hasPendingDatabaseActivity();
    140 }
    141 
    142133DatabaseThread* DatabaseContext::databaseThread()
    143134{
  • trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.h

    r251592 r252064  
    7474    void contextDestroyed() override;
    7575    void stop() override;
    76     bool shouldPreventEnteringBackForwardCache_DEPRECATED() const override;
    7776    const char* activeDOMObjectName() const override { return "DatabaseContext"; }
    7877
  • trunk/Source/WebCore/Modules/webdatabase/DatabaseManager.cpp

    r251592 r252064  
    3939#include "SecurityOrigin.h"
    4040#include "SecurityOriginData.h"
     41#include "WindowEventLoop.h"
    4142#include <wtf/NeverDestroyed.h>
    4243
     
    193194ExceptionOr<Ref<Database>> DatabaseManager::openDatabase(Document& document, const String& name, const String& expectedVersion, const String& displayName, unsigned estimatedSize, RefPtr<DatabaseCallback>&& creationCallback)
    194195{
     196    ASSERT(isMainThread());
    195197    ScriptController::initializeThreading();
    196198
     
    209211        LOG(StorageAPI, "Scheduling DatabaseCreationCallbackTask for database %p\n", database.get());
    210212        database->setHasPendingCreationEvent(true);
    211         database->m_document->postTask([creationCallback, database] (ScriptExecutionContext&) {
     213        database->m_document->eventLoop().queueTask(TaskSource::Networking, database->m_document, [creationCallback, database]() {
    212214            creationCallback->handleEvent(*database);
    213215            database->setHasPendingCreationEvent(false);
  • trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.cpp

    r251592 r252064  
    4848#include "SQLiteTransaction.h"
    4949#include "VoidCallback.h"
     50#include "WindowEventLoop.h"
    5051#include <wtf/Optional.h>
    5152#include <wtf/StdLibExtras.h>
     
    110111void SQLTransaction::performPendingCallback()
    111112{
     113    ASSERT(isMainThread());
    112114    LOG(StorageAPI, "Callback %s\n", debugStepName(m_nextStep));
    113115
     
    126128void SQLTransaction::notifyDatabaseThreadIsShuttingDown()
    127129{
     130    callOnMainThread([this, protectedThis = makeRef(*this)]() mutable {
     131        callErrorCallbackDueToInterruption();
     132    });
     133
    128134    m_backend.notifyDatabaseThreadIsShuttingDown();
     135}
     136
     137void SQLTransaction::callErrorCallbackDueToInterruption()
     138{
     139    ASSERT(isMainThread());
     140    auto errorCallback = m_errorCallbackWrapper.unwrap();
     141    if (!errorCallback)
     142        return;
     143
     144    m_database->document().eventLoop().queueTask(TaskSource::Networking, m_database->document(), [errorCallback = WTFMove(errorCallback)]() mutable {
     145        errorCallback->handleEvent(SQLError::create(SQLError::DATABASE_ERR, "the database was closed"));
     146    });
    129147}
    130148
     
    181199    m_nextStep = nullptr;
    182200
     201    callErrorCallbackDueToInterruption();
     202
    183203    // Release the unneeded callbacks, to break reference cycles.
    184204    m_callbackWrapper.clear();
     
    395415    // error to have occurred in this transaction.
    396416    RefPtr<SQLTransactionErrorCallback> errorCallback = m_errorCallbackWrapper.unwrap();
    397     if (errorCallback)
    398         errorCallback->handleEvent(*m_transactionError);
     417    if (errorCallback) {
     418        m_database->document().eventLoop().queueTask(TaskSource::Networking, m_database->document(), [errorCallback = WTFMove(errorCallback), transactionError = m_transactionError]() mutable {
     419            errorCallback->handleEvent(*transactionError);
     420        });
     421    }
    399422
    400423    clearCallbackWrappers();
     
    443466    // Spec 4.3.2.8: Deliver success callback.
    444467    RefPtr<VoidCallback> successCallback = m_successCallbackWrapper.unwrap();
    445     if (successCallback)
    446         successCallback->handleEvent();
     468    if (successCallback) {
     469        m_database->document().eventLoop().queueTask(TaskSource::Networking, m_database->document(), [successCallback = WTFMove(successCallback)]() mutable {
     470            successCallback->handleEvent();
     471        });
     472    }
    447473
    448474    clearCallbackWrappers();
     
    476502        LOG(StorageAPI, "Callback %s\n", nameForSQLTransactionState(m_nextState));
    477503        return;
    478     }
     504    } else
     505        callErrorCallbackDueToInterruption();
    479506
    480507    clearCallbackWrappers();
  • trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.h

    r242776 r252064  
    106106    NO_RETURN_DUE_TO_ASSERT void unreachableState();
    107107
     108    void callErrorCallbackDueToInterruption();
     109
    108110    void getNextStatement();
    109111    bool runCurrentStatement();
Note: See TracChangeset for help on using the changeset viewer.