Changeset 250965 in webkit


Ignore:
Timestamp:
Oct 10, 2019 7:24:07 AM (5 years ago)
Author:
Chris Dumez
Message:

DOMCacheStorage should not prevent pages from entering the back/forward cache
​https://bugs.webkit.org/show_bug.cgi?id=202608

Reviewed by Youenn Fablet.

Source/WebCore:

Make DOMCacheStorage fully suspendable by queueing all its asynchronous promise resolutions
to a SuspendableTaskQueue. This makes sure that no promises are resolved while suspended
in the page cache.

Test: http/tests/navigation/page-cache-domcachestorage-pending-promise.html

  • Modules/cache/DOMCacheStorage.cpp:

(WebCore::DOMCacheStorage::DOMCacheStorage):
(WebCore::DOMCacheStorage::doSequentialMatch):
(WebCore::DOMCacheStorage::match):
(WebCore::DOMCacheStorage::has):
(WebCore::DOMCacheStorage::open):
(WebCore::DOMCacheStorage::doOpen):
(WebCore::DOMCacheStorage::remove):
(WebCore::DOMCacheStorage::doRemove):
(WebCore::DOMCacheStorage::keys):
(WebCore::DOMCacheStorage::canSuspendForDocumentSuspension const):

  • Modules/cache/DOMCacheStorage.h:

LayoutTests:

Add layout test coverage.

  • http/tests/navigation/page-cache-domcachestorage-pending-promise-expected.txt: Added.
  • http/tests/navigation/page-cache-domcachestorage-pending-promise.html: Added.
Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r250963 r250965  
     12019-10-10  Chris Dumez  <cdumez@apple.com>
     2
     3        DOMCacheStorage should not prevent pages from entering the back/forward cache
     4        https://bugs.webkit.org/show_bug.cgi?id=202608
     5
     6        Reviewed by Youenn Fablet.
     7
     8        Add layout test coverage.
     9
     10        * http/tests/navigation/page-cache-domcachestorage-pending-promise-expected.txt: Added.
     11        * http/tests/navigation/page-cache-domcachestorage-pending-promise.html: Added.
     12
    1132019-10-10  Miguel Gomez  <magomez@igalia.com>
    214

  • trunk/LayoutTests/platform/ios-wk1/TestExpectations

    r250944 r250965  
    1313http/tests/cache-storage [ Skip ]
    1414http/tests/navigation/page-cache-domcache-pending-promise.html [ Skip ]
     15http/tests/navigation/page-cache-domcachestorage-pending-promise.html [ Skip ]
    1516imported/w3c/web-platform-tests/fetch/api/request/destination [ Skip ]
    1617imported/w3c/web-platform-tests/fetch/cross-origin-resource-policy [ Skip ]

  • trunk/LayoutTests/platform/mac-wk1/TestExpectations

    r250944 r250965  
    273273http/tests/cache-storage [ Skip ]
    274274http/tests/navigation/page-cache-domcache-pending-promise.html [ Skip ]
     275http/tests/navigation/page-cache-domcachestorage-pending-promise.html [ Skip ]
    275276http/tests/cookies/same-site/fetch-in-cross-origin-service-worker.html [ Skip ]
    276277http/tests/cookies/same-site/fetch-in-same-origin-service-worker.html [ Skip ]

  • trunk/LayoutTests/platform/win/TestExpectations

    r250936 r250965  
    38013801http/tests/cache-storage [ Skip ]
    38023802http/tests/navigation/page-cache-domcache-pending-promise.html [ Skip ]
     3803http/tests/navigation/page-cache-domcachestorage-pending-promise.html [ Skip ]
    38033804http/tests/inspector/network/resource-response-service-worker.html [ Skip ]
    38043805http/tests/workers/service [ Skip ]

  • trunk/Source/WebCore/ChangeLog

    r250958 r250965  
     12019-10-10  Chris Dumez  <cdumez@apple.com>
     2
     3        DOMCacheStorage should not prevent pages from entering the back/forward cache
     4        https://bugs.webkit.org/show_bug.cgi?id=202608
     5
     6        Reviewed by Youenn Fablet.
     7
     8        Make DOMCacheStorage fully suspendable by queueing all its asynchronous promise resolutions
     9        to a SuspendableTaskQueue. This makes sure that no promises are resolved while suspended
     10        in the page cache.
     11
     12        Test: http/tests/navigation/page-cache-domcachestorage-pending-promise.html
     13
     14        * Modules/cache/DOMCacheStorage.cpp:
     15        (WebCore::DOMCacheStorage::DOMCacheStorage):
     16        (WebCore::DOMCacheStorage::doSequentialMatch):
     17        (WebCore::DOMCacheStorage::match):
     18        (WebCore::DOMCacheStorage::has):
     19        (WebCore::DOMCacheStorage::open):
     20        (WebCore::DOMCacheStorage::doOpen):
     21        (WebCore::DOMCacheStorage::remove):
     22        (WebCore::DOMCacheStorage::doRemove):
     23        (WebCore::DOMCacheStorage::keys):
     24        (WebCore::DOMCacheStorage::canSuspendForDocumentSuspension const):
     25        * Modules/cache/DOMCacheStorage.h:
     26
    1272019-10-10  youenn fablet  <youenn@apple.com>
    228

  • trunk/Source/WebCore/Modules/cache/DOMCacheStorage.cpp

    r250786 r250965  
    3232#include "JSFetchResponse.h"
    3333#include "ScriptExecutionContext.h"
    34 
     34#include "SuspendableTaskQueue.h"
    3535
    3636namespace WebCore {
    … …  
    4040    : ActiveDOMObject(&context)
    4141    , m_connection(WTFMove(connection))
     42    , m_taskQueue(SuspendableTaskQueue::create(&context))
    4243{
    4344    suspendIfNeeded();
    4445}
     46
     47DOMCacheStorage::~DOMCacheStorage() = default;
    4548
    4649Optional<ClientOrigin> DOMCacheStorage::origin() const
    … …  
    8790{
    8891    startSequentialMatch(WTF::map(m_caches, copyCache), WTFMove(info), WTFMove(options), [this, pendingActivity = makePendingActivity(*this), promise = WTFMove(promise)](auto&& result) mutable {
    89         if (m_isStopped)
    90             return;
    91         if (result.hasException()) {
    92             promise->reject(result.releaseException());
    93             return;
    94         }
    95         if (!result.returnValue()) {
    96             promise->resolve();
    97             return;
    98         }
    99         promise->resolve<IDLInterface<FetchResponse>>(*result.returnValue());
     92        m_taskQueue->enqueueTask([promise = WTFMove(promise), result = WTFMove(result)]() mutable {
     93            if (result.hasException()) {
     94                promise->reject(result.releaseException());
     95                return;
     96            }
     97            if (!result.returnValue()) {
     98                promise->resolve();
     99                return;
     100            }
     101            promise->resolve<IDLInterface<FetchResponse>>(*result.returnValue());
     102        });
    100103    });
    101104}
    … …  
    105108    retrieveCaches([this, info = WTFMove(info), options = WTFMove(options), promise = WTFMove(promise)](Optional<Exception>&& exception) mutable {
    106109        if (exception) {
    107             promise->reject(WTFMove(exception.value()));
     110            m_taskQueue->enqueueTask([promise = WTFMove(promise), exception = WTFMove(exception.value())]() mutable {
     111                promise->reject(WTFMove(exception));
     112            });
    108113            return;
    109114        }
    … …  
    115120                return;
    116121            }
    117             promise->resolve();
     122            m_taskQueue->enqueueTask([promise = WTFMove(promise)]() mutable {
     123                promise->resolve();
     124            });
    118125            return;
    119126        }
    … …  
    126133{
    127134    retrieveCaches([this, name, promise = WTFMove(promise)](Optional<Exception>&& exception) mutable {
    128         if (exception) {
    129             promise.reject(WTFMove(exception.value()));
    130             return;
    131         }
    132         promise.resolve(m_caches.findMatching([&](auto& item) { return item->name() == name; }) != notFound);
     135        m_taskQueue->enqueueTask([this, name, promise = WTFMove(promise), exception = WTFMove(exception)]() mutable {
     136            if (exception) {
     137                promise.reject(WTFMove(exception.value()));
     138                return;
     139            }
     140            promise.resolve(m_caches.findMatching([&](auto& item) { return item->name() == name; }) != notFound);
     141        });
    133142    });
    134143}
    … …  
    181190    retrieveCaches([this, name, promise = WTFMove(promise)](Optional<Exception>&& exception) mutable {
    182191        if (exception) {
    183             promise.reject(WTFMove(exception.value()));
     192            m_taskQueue->enqueueTask([promise = WTFMove(promise), exception = WTFMove(exception.value())]() mutable {
     193                promise.reject(WTFMove(exception));
     194            });
    184195            return;
    185196        }
    … …  
    192203    auto position = m_caches.findMatching([&](auto& item) { return item->name() == name; });
    193204    if (position != notFound) {
    194         auto& cache = m_caches[position];
    195         promise.resolve(DOMCache::create(*scriptExecutionContext(), String { cache->name() }, cache->identifier(), m_connection.copyRef()));
     205        m_taskQueue->enqueueTask([this, promise = WTFMove(promise), cache = m_caches[position].copyRef()]() mutable {
     206            promise.resolve(DOMCache::create(*scriptExecutionContext(), String { cache->name() }, cache->identifier(), m_connection.copyRef()));
     207        });
    196208        return;
    197209    }
    198210
    199211    m_connection->open(*origin(), name, [this, name, promise = WTFMove(promise), pendingActivity = makePendingActivity(*this)](const CacheIdentifierOrError& result) mutable {
    200         if (!m_isStopped) {
    201             if (!result.has_value())
    202                 promise.reject(DOMCacheEngine::convertToExceptionAndLog(scriptExecutionContext(), result.error()));
    203             else {
    204                 if (result.value().hadStorageError)
    205                     logConsolePersistencyError(scriptExecutionContext(), name);
    206 
    207                 auto cache = DOMCache::create(*scriptExecutionContext(), String { name }, result.value().identifier, m_connection.copyRef());
     212        if (!result.has_value()) {
     213            m_taskQueue->enqueueTask([this, promise = WTFMove(promise), error = result.error()]() mutable {
     214                promise.reject(DOMCacheEngine::convertToExceptionAndLog(scriptExecutionContext(), error));
     215            });
     216        } else {
     217            if (result.value().hadStorageError)
     218                logConsolePersistencyError(scriptExecutionContext(), name);
     219
     220            m_taskQueue->enqueueTask([this, name, promise = WTFMove(promise), identifier = result.value().identifier]() mutable {
     221                auto cache = DOMCache::create(*scriptExecutionContext(), String { name }, identifier, m_connection.copyRef());
    208222                promise.resolve(cache);
    209223                m_caches.append(WTFMove(cache));
    210             }
     224            });
    211225        }
    212226    });
    … …  
    217231    retrieveCaches([this, name, promise = WTFMove(promise)](Optional<Exception>&& exception) mutable {
    218232        if (exception) {
    219             promise.reject(WTFMove(exception.value()));
     233            m_taskQueue->enqueueTask([promise = WTFMove(promise), exception = WTFMove(exception.value())]() mutable {
     234                promise.reject(WTFMove(exception));
     235            });
    220236            return;
    221237        }
    … …  
    228244    auto position = m_caches.findMatching([&](auto& item) { return item->name() == name; });
    229245    if (position == notFound) {
    230         promise.resolve(false);
     246        m_taskQueue->enqueueTask([promise = WTFMove(promise)]() mutable {
     247            promise.resolve(false);
     248        });
    231249        return;
    232250    }
    233251
    234252    m_connection->remove(m_caches[position]->identifier(), [this, name, promise = WTFMove(promise), pendingActivity = makePendingActivity(*this)](const CacheIdentifierOrError& result) mutable {
    235         if (!m_isStopped) {
     253        m_taskQueue->enqueueTask([this, name, promise = WTFMove(promise), result]() mutable {
    236254            if (!result.has_value())
    237255                promise.reject(DOMCacheEngine::convertToExceptionAndLog(scriptExecutionContext(), result.error()));
    … …  
    241259                promise.resolve(!!result.value().identifier);
    242260            }
    243         }
     261        });
    244262    });
    245263}
    … …  
    248266{
    249267    retrieveCaches([this, promise = WTFMove(promise)](Optional<Exception>&& exception) mutable {
    250         if (exception) {
    251             promise.reject(WTFMove(exception.value()));
    252             return;
    253         }
    254 
    255         promise.resolve(WTF::map(m_caches, [] (const auto& cache) {
    256             return cache->name();
    257         }));
     268        m_taskQueue->enqueueTask([this, promise = WTFMove(promise), exception = WTFMove(exception)]() mutable {
     269            if (exception) {
     270                promise.reject(WTFMove(exception.value()));
     271                return;
     272            }
     273
     274            promise.resolve(WTF::map(m_caches, [] (const auto& cache) {
     275                return cache->name();
     276            }));
     277        });
    258278    });
    259279}
    … …  
    271291bool DOMCacheStorage::canSuspendForDocumentSuspension() const
    272292{
    273     return !hasPendingActivity();
     293    return true;
    274294}
    275295

  • trunk/Source/WebCore/Modules/cache/DOMCacheStorage.h

    r243818 r250965  
    3333namespace WebCore {
    3434
     35class SuspendableTaskQueue;
     36
    3537class DOMCacheStorage : public RefCounted<DOMCacheStorage>, public ActiveDOMObject {
    3638public:
    3739    static Ref<DOMCacheStorage> create(ScriptExecutionContext& context, Ref<CacheStorageConnection>&& connection) { return adoptRef(*new DOMCacheStorage(context, WTFMove(connection))); }
     40    ~DOMCacheStorage();
    3841
    3942    using KeysPromise = DOMPromiseDeferred<IDLSequence<IDLDOMString>>;
    … …  
    6366    uint64_t m_updateCounter { 0 };
    6467    Ref<CacheStorageConnection> m_connection;
     68    UniqueRef<SuspendableTaskQueue> m_taskQueue;
    6569    bool m_isStopped { false };
    6670};
Note: See TracChangeset for help on using the changeset viewer.