Changeset 236662 in webkit


Ignore:
Timestamp:
Oct 1, 2018 9:17:29 AM (6 years ago)
Author:
youenn@apple.com
Message:

[macOS Sierra] Layout Test http/wpt/cache-storage/cache-put-keys.https.any.worker.html is a flaky failure
​https://bugs.webkit.org/show_bug.cgi?id=184204

Reviewed by Chris Dumez.

Source/WebKit:

NetworkCache::Storage by default limits the length to read to 1500 milliseconds.
This is good for the HTTP cache since networking might be faster.
It does not work for DOM cache which needs to get these resources even if it takes a long time.

Since this is disabled by a Mode::Testing option, use it for DOMCache after renaming it to Mode::AvoidRandomness.

Add a bunch of release logging to help debugging error cases.

  • NetworkProcess/cache/CacheStorageEngineCaches.cpp:

(WebKit::CacheStorage::Caches::retrieveOriginFromDirectory):
(WebKit::CacheStorage::Caches::initialize):
(WebKit::CacheStorage::Caches::writeCachesToDisk):
(WebKit::CacheStorage::Caches::readRecord):

  • NetworkProcess/cache/NetworkCache.cpp:

(WebKit::NetworkCache::Cache::open):

  • NetworkProcess/cache/NetworkCacheStorage.cpp:

(WebKit::NetworkCache::Storage::dispatchReadOperation):
(WebKit::NetworkCache::Storage::shrinkIfNeeded):

  • NetworkProcess/cache/NetworkCacheStorage.h:

LayoutTests:

  • platform/mac-wk2/TestExpectations:
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r236661 r236662  
     12018-10-01  Youenn Fablet  <youenn@apple.com>
     2
     3        [macOS Sierra] Layout Test http/wpt/cache-storage/cache-put-keys.https.any.worker.html is a flaky failure
     4        https://bugs.webkit.org/show_bug.cgi?id=184204
     5
     6        Reviewed by Chris Dumez.
     7
     8        * platform/mac-wk2/TestExpectations:
     9
    1102018-10-01  Chris Dumez  <cdumez@apple.com>
    211

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

    r236625 r236662  
    877877imported/w3c/web-platform-tests/payment-request/user-accepts-payment-request-algo-manual.https.html [ Skip ]
    878878
    879 webkit.org/b/184204 [ Sierra ] http/wpt/cache-storage/cache-put-keys.https.any.worker.html [ Pass Failure ]
    880 
    881879webkit.org/b/189094 [ HighSierra+ ] accessibility/mac/focus-setting-selection-syncronizing-not-clearing.html [ Skip ]
    882880

  • trunk/Source/WebKit/ChangeLog

    r236659 r236662  
     12018-10-01  Youenn Fablet  <youenn@apple.com>
     2
     3        [macOS Sierra] Layout Test http/wpt/cache-storage/cache-put-keys.https.any.worker.html is a flaky failure
     4        https://bugs.webkit.org/show_bug.cgi?id=184204
     5
     6        Reviewed by Chris Dumez.
     7
     8        NetworkCache::Storage by default limits the length to read to 1500 milliseconds.
     9        This is good for the HTTP cache since networking might be faster.
     10        It does not work for DOM cache which needs to get these resources even if it takes a long time.
     11
     12        Since this is disabled by a Mode::Testing option, use it for DOMCache after renaming it to Mode::AvoidRandomness.
     13
     14        Add a bunch of release logging to help debugging error cases.
     15
     16        * NetworkProcess/cache/CacheStorageEngineCaches.cpp:
     17        (WebKit::CacheStorage::Caches::retrieveOriginFromDirectory):
     18        (WebKit::CacheStorage::Caches::initialize):
     19        (WebKit::CacheStorage::Caches::writeCachesToDisk):
     20        (WebKit::CacheStorage::Caches::readRecord):
     21        * NetworkProcess/cache/NetworkCache.cpp:
     22        (WebKit::NetworkCache::Cache::open):
     23        * NetworkProcess/cache/NetworkCacheStorage.cpp:
     24        (WebKit::NetworkCache::Storage::dispatchReadOperation):
     25        (WebKit::NetworkCache::Storage::shrinkIfNeeded):
     26        * NetworkProcess/cache/NetworkCacheStorage.h:
     27
    1282018-10-01  Olivier Blin  <olivier.blin@softathome.com>
    229

  • trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp

    r235169 r236662  
    2727#include "CacheStorageEngine.h"
    2828
     29#include "Logging.h"
    2930#include "NetworkCacheCoders.h"
    3031#include "NetworkCacheIOChannel.h"
    … …  
    6970            ASSERT(RunLoop::isMain());
    7071            if (error) {
     72                RELEASE_LOG_ERROR(CacheStorage, "Caches::retrieveOriginFromDirectory failed reading channel with error %d", error);
    7173                completionHandler(std::nullopt);
    7274                return;
    … …  
    139141    }
    140142
    141     auto storage = Storage::open(m_rootPath, Storage::Mode::Normal);
     143    auto storage = Storage::open(m_rootPath, Storage::Mode::AvoidRandomness);
    142144    if (!storage) {
     145        RELEASE_LOG_ERROR(CacheStorage, "Caches::initialize failed opening storage");
    143146        callback(Error::WriteDisk);
    144147        return;
    … …  
    151154    storeOrigin([this] (std::optional<Error>&& error) mutable {
    152155        if (error) {
     156            RELEASE_LOG_ERROR(CacheStorage, "Caches::initialize failed storing origin with error %d", *error);
     157
    153158            auto pendingCallbacks = WTFMove(m_pendingInitializationCallbacks);
    154159            for (auto& callback : pendingCallbacks)
    … …  
    163168
    164169            if (!result.has_value()) {
     170                RELEASE_LOG_ERROR(CacheStorage, "Caches::initialize failed reading caches from disk with error %d", result.error());
     171
    165172                auto pendingCallbacks = WTFMove(m_pendingInitializationCallbacks);
    166173                for (auto& callback : pendingCallbacks)
    … …  
    368375}
    369376
    370 static inline Expected<Vector<std::pair<String, String>>, Error> decodeCachesNames(const Data& data, int error)
    371 {
    372     if (error)
    373         return makeUnexpected(Error::ReadDisk);
    374 
     377static inline Expected<Vector<std::pair<String, String>>, Error> decodeCachesNames(const Data& data)
     378{
    375379    WTF::Persistence::Decoder decoder(data.data(), data.size());
    376380    uint64_t count;
    … …  
    416420        }
    417421
    418         auto result = decodeCachesNames(data, error);
     422        if (error) {
     423            RELEASE_LOG_ERROR(CacheStorage, "Caches::readCachesFromDisk failed reading caches from disk with error %d", error);
     424            callback(makeUnexpected(Error::ReadDisk));
     425            return;
     426        }
     427
     428        auto result = decodeCachesNames(data);
    419429        if (!result.has_value()) {
     430            RELEASE_LOG_ERROR(CacheStorage, "Caches::decodeCachesNames failed decoding caches with error %d", result.error());
    420431            callback(makeUnexpected(result.error()));
    421432            return;
    … …  
    447458    m_engine->writeFile(cachesListFilename(m_rootPath), encodeCacheNames(m_caches), [this, protectedThis = makeRef(*this), callback = WTFMove(callback)](std::optional<Error>&& error) mutable {
    448459        m_isWritingCachesToDisk = false;
     460        if (error)
     461            RELEASE_LOG_ERROR(CacheStorage, "Caches::writeCachesToDisk failed writing caches to disk with error %d", *error);
     462
    449463        callback(WTFMove(error));
    450464        while (!m_pendingWritingCachesToDiskCallbacks.isEmpty() && !m_isWritingCachesToDisk)
    … …  
    510524    m_storage->retrieve(key, 4, [protectedStorage = makeRef(*m_storage), callback = WTFMove(callback)](std::unique_ptr<Storage::Record> storage, const Storage::Timings&) mutable {
    511525        if (!storage) {
     526            RELEASE_LOG_ERROR(CacheStorage, "Caches::readRecord failed reading record from disk");
    512527            callback(makeUnexpected(Error::ReadDisk));
    513528            return false;
    … …  
    516531        auto record = Cache::decode(*storage);
    517532        if (!record) {
     533            RELEASE_LOG_ERROR(CacheStorage, "Caches::readRecord failed decoding record from disk");
    518534            callback(makeUnexpected(Error::ReadDisk));
    519535            return false;

  • trunk/Source/WebKit/NetworkProcess/cache/NetworkCache.cpp

    r236485 r236662  
    6262RefPtr<Cache> Cache::open(const String& cachePath, OptionSet<Option> options)
    6363{
    64     auto storage = Storage::open(cachePath, options.contains(Option::TestingMode) ? Storage::Mode::Testing : Storage::Mode::Normal);
     64    auto storage = Storage::open(cachePath, options.contains(Option::TestingMode) ? Storage::Mode::AvoidRandomness : Storage::Mode::Normal);
    6565
    6666    LOG(NetworkCache, "(NetworkProcess) opened cache storage, success %d", !!storage);

  • trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp

    r235220 r236662  
    658658
    659659    // Avoid randomness during testing.
    660     if (m_mode != Mode::Testing) {
     660    if (m_mode != Mode::AvoidRandomness) {
    661661        // I/O pressure may make disk operations slow. If they start taking very long time we rather go to network.
    662662        const Seconds readTimeout = 1500_ms;
    … …  
    10711071
    10721072    // Avoid randomness caused by cache shrinks.
    1073     if (m_mode == Mode::Testing)
     1073    if (m_mode == Mode::AvoidRandomness)
    10741074        return;
    10751075

  • trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h

    r235169 r236662  
    4848class Storage : public ThreadSafeRefCounted<Storage> {
    4949public:
    50     enum class Mode { Normal, Testing };
     50    enum class Mode { Normal, AvoidRandomness };
    5151    static RefPtr<Storage> open(const String& cachePath, Mode);
    5252
Note: See TracChangeset for help on using the changeset viewer.