Attachment 341298 Details for Bug 185984 – Patch

[patch] Patch

bug-185984-20180525103641.patch (text/plain), 17.23 KB, created by Chris Dumez on 2018-05-25 10:36:42 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-05-25 10:36:42 PDT

Size: 17.23 KB

patch

obsolete

>Subversion Revision: 232189
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f1ee6fc4351efc493a26502ebb3f7dc799819a43..a72f732c644e7d5e146323b53690edd6f0ca7cdf 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-05-25  Chris Dumez  <cdumez@apple.com>
>+
>+        Minor ApplicationCacheStorage clean up
>+        https://bugs.webkit.org/show_bug.cgi?id=185984
>+
>+        Reviewed by Youenn Fablet.
>+
>+        * loader/appcache/ApplicationCacheStorage.cpp:
>+        (WebCore::ApplicationCacheStorage::getManifestURLs):
>+        (WebCore::ApplicationCacheStorage::deleteCacheGroup):
>+        (WebCore::ApplicationCacheStorage::originsWithCache):
>+        (WebCore::ApplicationCacheStorage::deleteAllCaches):
>+        (WebCore::ApplicationCacheStorage::deleteCacheForOrigin):
>+        (WebCore::ApplicationCacheStorage::ApplicationCacheStorage):
>+        (WebCore::ApplicationCacheStorage::cacheDirectory const): Deleted.
>+        (WebCore::ApplicationCacheStorage::cacheGroupSize): Deleted.
>+        (WebCore::ApplicationCacheStorage::getOriginsWithCache): Deleted.
>+        (WebCore::ApplicationCacheStorage::create): Deleted.
>+        * loader/appcache/ApplicationCacheStorage.h:
>+        (WebCore::ApplicationCacheStorage::create):
>+
> 2018-05-25  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Implement border and padding computation
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 4c7331b26dc77653b41a1aa83423742dcb4ed563..95e3c21997363857f3d26458557a485d67806861 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-05-25  Chris Dumez  <cdumez@apple.com>
>+
>+        Minor ApplicationCacheStorage clean up
>+        https://bugs.webkit.org/show_bug.cgi?id=185984
>+
>+        Reviewed by Youenn Fablet.
>+
>+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
>+        (WebKit::WebsiteDataStore::fetchDataAndApply):
>+        * WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:
>+        (WKBundlePageCopyOriginsWithApplicationCache):
>+
> 2018-05-25  David Kilzer  <ddkilzer@apple.com>
> 
>         Fix issues with -dealloc methods found by clang static analyzer
>diff --git a/Source/WebKitLegacy/mac/ChangeLog b/Source/WebKitLegacy/mac/ChangeLog
>index 8d75c62413875088611ca54e04f535322c9d6294..e01b453ded09bef25bfb77ac51da528cbfc30ed3 100644
>--- a/Source/WebKitLegacy/mac/ChangeLog
>+++ b/Source/WebKitLegacy/mac/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-05-25  Chris Dumez  <cdumez@apple.com>
>+
>+        Minor ApplicationCacheStorage clean up
>+        https://bugs.webkit.org/show_bug.cgi?id=185984
>+
>+        Reviewed by Youenn Fablet.
>+
>+        * WebCoreSupport/WebApplicationCache.mm:
>+        (+[WebApplicationCache originsWithCache]):
>+
> 2018-05-25  David Kilzer  <ddkilzer@apple.com>
> 
>         Fix issues with -dealloc methods found by clang static analyzer
>diff --git a/Source/WebKitLegacy/win/ChangeLog b/Source/WebKitLegacy/win/ChangeLog
>index 425f49b0b8ac28249be86e0870d71a833aef5608..ce535f41a9f48dbce13ee21364a1950905511ef9 100644
>--- a/Source/WebKitLegacy/win/ChangeLog
>+++ b/Source/WebKitLegacy/win/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-05-25  Chris Dumez  <cdumez@apple.com>
>+
>+        Minor ApplicationCacheStorage clean up
>+        https://bugs.webkit.org/show_bug.cgi?id=185984
>+
>+        Reviewed by Youenn Fablet.
>+
>+        * WebApplicationCache.cpp:
>+        (WebApplicationCache::originsWithCache):
>+
> 2018-05-24  Chris Dumez  <cdumez@apple.com>
> 
>         Reduce copying of FontCascadeDescription objects by moving them around
>diff --git a/Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp b/Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp
>index ff293899706de895cef7503c0e857fad2fe128d6..0486c7f2ce27f3eca9e7e53c7f61859bcc511c00 100644
>--- a/Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp
>+++ b/Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp
>@@ -356,11 +356,6 @@ void ApplicationCacheStorage::cacheGroupMadeObsolete(ApplicationCacheGroup& grou
>     m_cacheHostSet.remove(urlHostHash(group.manifestURL()));
> }
> 
>-const String& ApplicationCacheStorage::cacheDirectory() const
>-{
>-    return m_cacheDirectory;
>-}
>-
> void ApplicationCacheStorage::setMaximumSize(int64_t size)
> {
>     m_maximumSize = size;
>@@ -1318,52 +1313,24 @@ bool ApplicationCacheStorage::writeDataToUniqueFileInDirectory(SharedBuffer& dat
>     return true;
> }
> 
>-bool ApplicationCacheStorage::getManifestURLs(Vector<URL>* urls)
>+std::optional<Vector<URL>> ApplicationCacheStorage::manifestURLs()
> {
>     SQLiteTransactionInProgressAutoCounter transactionCounter;
> 
>-    ASSERT(urls);
>     openDatabase(false);
>     if (!m_database.isOpen())
>-        return false;
>+        return std::nullopt;
> 
>     SQLiteStatement selectURLs(m_database, "SELECT manifestURL FROM CacheGroups");
> 
>     if (selectURLs.prepare() != SQLITE_OK)
>-        return false;
>+        return std::nullopt;
> 
>+    Vector<URL> urls;
>     while (selectURLs.step() == SQLITE_ROW)
>-        urls->append(URL(ParsedURLString, selectURLs.getColumnText(0)));
>+        urls.append(URL(ParsedURLString, selectURLs.getColumnText(0)));
> 
>-    return true;
>-}
>-
>-bool ApplicationCacheStorage::cacheGroupSize(const String& manifestURL, int64_t* size)
>-{
>-    SQLiteTransactionInProgressAutoCounter transactionCounter;
>-
>-    ASSERT(size);
>-    openDatabase(false);
>-    if (!m_database.isOpen())
>-        return false;
>-
>-    SQLiteStatement statement(m_database, "SELECT sum(Caches.size) FROM Caches INNER JOIN CacheGroups ON Caches.cacheGroup=CacheGroups.id WHERE CacheGroups.manifestURL=?");
>-    if (statement.prepare() != SQLITE_OK)
>-        return false;
>-
>-    statement.bindText(1, manifestURL);
>-
>-    int result = statement.step();
>-    if (result == SQLITE_DONE)
>-        return false;
>-
>-    if (result != SQLITE_ROW) {
>-        LOG_ERROR("Could not get the size of the cache group, error \"%s\"", m_database.lastErrorMsg());
>-        return false;
>-    }
>-
>-    *size = statement.getColumnInt64(0);
>-    return true;
>+    return WTFMove(urls);
> }
> 
> bool ApplicationCacheStorage::deleteCacheGroupRecord(const String& manifestURL)
>@@ -1403,8 +1370,7 @@ bool ApplicationCacheStorage::deleteCacheGroup(const String& manifestURL)
>     SQLiteTransaction deleteTransaction(m_database);
> 
>     // Check to see if the group is in memory.
>-    auto* group = m_cachesInMemory.get(manifestURL);
>-    if (group)
>+    if (auto* group = m_cachesInMemory.get(manifestURL))
>         cacheGroupMadeObsolete(*group);
>     else {
>         // The cache group is not in memory, so remove it from the disk.
>@@ -1506,15 +1472,19 @@ long long ApplicationCacheStorage::flatFileAreaSize()
>     return totalSize;
> }
> 
>-void ApplicationCacheStorage::getOriginsWithCache(HashSet<RefPtr<SecurityOrigin>>& origins)
>+Vector<Ref<SecurityOrigin>> ApplicationCacheStorage::originsWithCache()
> {
>-    Vector<URL> urls;
>-    getManifestURLs(&urls);
>+    auto urls = manifestURLs();
>+    if (!urls)
>+        return { };
> 
>     // Multiple manifest URLs might share the same SecurityOrigin, so we might be creating extra, wasted origins here.
>     // The current schema doesn't allow for a more efficient way of building this list.
>-    for (auto& url : urls)
>-        origins.add(SecurityOrigin::create(url));
>+    Vector<Ref<SecurityOrigin>> origins;
>+    origins.reserveInitialCapacity(urls->size());
>+    for (auto& url : *urls)
>+        origins.uncheckedAppend(SecurityOrigin::create(url));
>+    return origins;
> }
> 
> void ApplicationCacheStorage::deleteAllEntries()
>@@ -1525,26 +1495,24 @@ void ApplicationCacheStorage::deleteAllEntries()
> 
> void ApplicationCacheStorage::deleteAllCaches()
> {
>-    HashSet<RefPtr<SecurityOrigin>> origins;
>-
>-    getOriginsWithCache(origins);
>+    auto origins = originsWithCache();
>     for (auto& origin : origins)
>-        deleteCacheForOrigin(*origin);
>+        deleteCacheForOrigin(origin);
> 
>     vacuumDatabaseFile();
> }
> 
> void ApplicationCacheStorage::deleteCacheForOrigin(const SecurityOrigin& securityOrigin)
> {
>-    Vector<URL> urls;
>-    if (!getManifestURLs(&urls)) {
>+    auto urls = manifestURLs();
>+    if (!urls) {
>         LOG_ERROR("Failed to retrieve ApplicationCache manifest URLs");
>         return;
>     }
> 
>     URL originURL(URL(), securityOrigin.toString());
> 
>-    for (const auto& url : urls) {
>+    for (const auto& url : *urls) {
>         if (!protocolHostAndPortAreEqual(url, originURL))
>             continue;
> 
>@@ -1565,15 +1533,7 @@ int64_t ApplicationCacheStorage::diskUsageForOrigin(const SecurityOrigin& securi
> ApplicationCacheStorage::ApplicationCacheStorage(const String& cacheDirectory, const String& flatFileSubdirectoryName)
>     : m_cacheDirectory(cacheDirectory)
>     , m_flatFileSubdirectoryName(flatFileSubdirectoryName)
>-    , m_maximumSize(ApplicationCacheStorage::noQuota())
>-    , m_isMaximumSizeReached(false)
>-    , m_defaultOriginQuota(ApplicationCacheStorage::noQuota())
> {
> }
> 
>-Ref<ApplicationCacheStorage> ApplicationCacheStorage::create(const String& cacheDirectory, const String& flatFileSubdirectoryName)
>-{
>-    return adoptRef(*new ApplicationCacheStorage(cacheDirectory, flatFileSubdirectoryName));
>-}
>-
>-}
>+} // namespace WebCore
>diff --git a/Source/WebCore/loader/appcache/ApplicationCacheStorage.h b/Source/WebCore/loader/appcache/ApplicationCacheStorage.h
>index 9d17a1bcd06a6ac111a0acaa60764f6b154050ff..48cbf88e5d8903711361895ca1c132a640c7d904 100644
>--- a/Source/WebCore/loader/appcache/ApplicationCacheStorage.h
>+++ b/Source/WebCore/loader/appcache/ApplicationCacheStorage.h
>@@ -51,10 +51,12 @@ public:
>         DiskOrOperationFailure
>     };
> 
>-    WEBCORE_EXPORT static Ref<ApplicationCacheStorage> create(const String& cacheDirectory, const String& flatFileSubdirectoryName);
>+    static Ref<ApplicationCacheStorage> create(const String& cacheDirectory, const String& flatFileSubdirectoryName)
>+    {
>+        return adoptRef(*new ApplicationCacheStorage(cacheDirectory, flatFileSubdirectoryName));
>+    }
>+
> 
>-    const String& cacheDirectory() const;
>-    
>     WEBCORE_EXPORT void setMaximumSize(int64_t size);
>     WEBCORE_EXPORT int64_t maximumSize() const;
>     bool isMaximumSizeReached() const;
>@@ -72,7 +74,6 @@ public:
>     ApplicationCacheGroup* fallbackCacheGroupForURL(const URL&); // Cache that has a fallback entry to load a main resource from if normal loading fails.
> 
>     ApplicationCacheGroup* findOrCreateCacheGroup(const URL& manifestURL);
>-    ApplicationCacheGroup* findInMemoryCacheGroup(const URL& manifestURL) const;
>     void cacheGroupDestroyed(ApplicationCacheGroup&);
>     void cacheGroupMadeObsolete(ApplicationCacheGroup&);
> 
>@@ -86,12 +87,7 @@ public:
> 
>     WEBCORE_EXPORT void empty();
> 
>-    bool getManifestURLs(Vector<URL>* urls);
>-    bool cacheGroupSize(const String& manifestURL, int64_t* size);
>-    bool deleteCacheGroup(const String& manifestURL);
>-    WEBCORE_EXPORT void vacuumDatabaseFile();
>-
>-    WEBCORE_EXPORT void getOriginsWithCache(HashSet<RefPtr<SecurityOrigin>, SecurityOriginHash>&);
>+    WEBCORE_EXPORT Vector<Ref<SecurityOrigin>> originsWithCache();
>     WEBCORE_EXPORT void deleteAllEntries();
> 
>     // FIXME: This should be consolidated with deleteAllEntries().
>@@ -107,10 +103,14 @@ public:
>     static int64_t noQuota() { return std::numeric_limits<int64_t>::max(); }
> 
> private:
>-    ApplicationCacheStorage(const String& cacheDirectory, const String& flatFileSubdirectoryName);
>+    WEBCORE_EXPORT ApplicationCacheStorage(const String& cacheDirectory, const String& flatFileSubdirectoryName);
> 
>     RefPtr<ApplicationCache> loadCache(unsigned storageID);
>     ApplicationCacheGroup* loadCacheGroup(const URL& manifestURL);
>+    std::optional<Vector<URL>> manifestURLs();
>+    ApplicationCacheGroup* findInMemoryCacheGroup(const URL& manifestURL) const;
>+    bool deleteCacheGroup(const String& manifestURL);
>+    void vacuumDatabaseFile();
>     
>     using ResourceStorageIDJournal = StorageIDJournal<ApplicationCacheResource>;
>     using GroupStorageIDJournal = StorageIDJournal<ApplicationCacheGroup>;
>@@ -121,7 +121,7 @@ private:
>     bool deleteCacheGroupRecord(const String& manifestURL);
> 
>     bool ensureOriginRecord(const SecurityOrigin*);
>-    bool shouldStoreResourceAsFlatFile(ApplicationCacheResource*);
>+    static bool shouldStoreResourceAsFlatFile(ApplicationCacheResource*);
>     void deleteTables();
>     bool writeDataToUniqueFileInDirectory(SharedBuffer&, const String& directory, String& outFilename, const String& fileExtension);
> 
>@@ -142,10 +142,10 @@ private:
>     const String m_flatFileSubdirectoryName;
>     String m_cacheFile;
> 
>-    int64_t m_maximumSize;
>-    bool m_isMaximumSizeReached;
>+    int64_t m_maximumSize { noQuota() };
>+    bool m_isMaximumSizeReached { false };
> 
>-    int64_t m_defaultOriginQuota;
>+    int64_t m_defaultOriginQuota { noQuota() };
> 
>     SQLiteDatabase m_database;
> 
>diff --git a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>index 26636d1d54608da2438ed25c775e2d2410d111bd..a9f268cff1baf4f0a89c8785cc99c70556f82b57 100644
>--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>@@ -447,12 +447,11 @@ void WebsiteDataStore::fetchDataAndApply(OptionSet<WebsiteDataType> dataTypes, O
> 
>             WebsiteData websiteData;
> 
>-            HashSet<RefPtr<WebCore::SecurityOrigin>> origins;
>             // FIXME: getOriginsWithCache should return a collection of SecurityOriginDatas.
>-            storage->getOriginsWithCache(origins);
>+            auto origins = storage->originsWithCache();
> 
>             for (auto& origin : origins) {
>-                uint64_t size = fetchOptions.contains(WebsiteDataFetchOption::ComputeSizes) ? storage->diskUsageForOrigin(*origin) : 0;
>+                uint64_t size = fetchOptions.contains(WebsiteDataFetchOption::ComputeSizes) ? storage->diskUsageForOrigin(origin) : 0;
>                 WebsiteData::Entry entry { origin->data(), WebsiteDataType::OfflineWebApplicationCache, size };
> 
>                 websiteData.entries.append(WTFMove(entry));
>diff --git a/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp b/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp
>index 5e8f117d8252fbfe47f577e2db008e4b28e2cd48..a0ea4ec53e6beaeeea0901faabfbde053b0c710c 100644
>--- a/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp
>+++ b/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp
>@@ -686,8 +686,7 @@ void WKBundlePageResetApplicationCacheOriginQuota(WKBundlePageRef page, WKString
> 
> WKArrayRef WKBundlePageCopyOriginsWithApplicationCache(WKBundlePageRef page)
> {
>-    HashSet<RefPtr<WebCore::SecurityOrigin>> origins;
>-    toImpl(page)->corePage()->applicationCacheStorage().getOriginsWithCache(origins);
>+    auto origins = toImpl(page)->corePage()->applicationCacheStorage().originsWithCache();
> 
>     Vector<RefPtr<API::Object>> originIdentifiers;
>     originIdentifiers.reserveInitialCapacity(origins.size());
>diff --git a/Source/WebKitLegacy/mac/WebCoreSupport/WebApplicationCache.mm b/Source/WebKitLegacy/mac/WebCoreSupport/WebApplicationCache.mm
>index ab2c13682a88c7585b4f3f49a55de07748ed9ea0..563b4c38e21dd9c27c2d2d408cbec270bafe297b 100644
>--- a/Source/WebKitLegacy/mac/WebCoreSupport/WebApplicationCache.mm
>+++ b/Source/WebKitLegacy/mac/WebCoreSupport/WebApplicationCache.mm
>@@ -123,14 +123,12 @@ + (void)deleteCacheForOrigin:(WebSecurityOrigin *)origin
> 
> + (NSArray *)originsWithCache
> {
>-    HashSet<RefPtr<SecurityOrigin>> coreOrigins;
>-    webApplicationCacheStorage().getOriginsWithCache(coreOrigins);
>+    auto coreOrigins = webApplicationCacheStorage().originsWithCache();
>     
>     NSMutableArray *webOrigins = [[[NSMutableArray alloc] initWithCapacity:coreOrigins.size()] autorelease];
>     
>-    HashSet<RefPtr<SecurityOrigin>>::const_iterator end = coreOrigins.end();
>-    for (HashSet<RefPtr<SecurityOrigin>>::const_iterator it = coreOrigins.begin(); it != end; ++it) {
>-        RetainPtr<WebSecurityOrigin> webOrigin = adoptNS([[WebSecurityOrigin alloc] _initWithWebCoreSecurityOrigin:(*it).get()]);
>+    for (auto& coreOrigin : coreOrigins) {
>+        auto webOrigin = adoptNS([[WebSecurityOrigin alloc] _initWithWebCoreSecurityOrigin:coreOrigin.ptr()]);
>         [webOrigins addObject:webOrigin.get()];
>     }
>     
>diff --git a/Source/WebKitLegacy/win/WebApplicationCache.cpp b/Source/WebKitLegacy/win/WebApplicationCache.cpp
>index 1ea90178ad8d38f2408775772b9792513aca403d..b85dd15440a6ea5404a8a4852143e59178fec12d 100644
>--- a/Source/WebKitLegacy/win/WebApplicationCache.cpp
>+++ b/Source/WebKitLegacy/win/WebApplicationCache.cpp
>@@ -187,13 +187,12 @@ HRESULT WebApplicationCache::originsWithCache(IPropertyBag** origins)
>     if (!origins)
>         return E_POINTER;
> 
>-    HashSet<RefPtr<WebCore::SecurityOrigin>> coreOrigins;
>-    storage().getOriginsWithCache(coreOrigins);
>+    auto coreOrigins = storage().originsWithCache();
> 
>     RetainPtr<CFMutableArrayRef> arrayItem = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, coreOrigins.size(), &MarshallingHelpers::kIUnknownArrayCallBacks));
> 
>-    for (auto it : coreOrigins)
>-        CFArrayAppendValue(arrayItem.get(), reinterpret_cast<void*>(WebSecurityOrigin::createInstance(&(*it))));
>+    for (auto& coreOrigin : coreOrigins)
>+        CFArrayAppendValue(arrayItem.get(), reinterpret_cast<void*>(WebSecurityOrigin::createInstance(coreOrigin.ptr())));
> 
>     RetainPtr<CFMutableDictionaryRef> dictionary = adoptCF(
>         CFDictionaryCreateMutable(0, 0, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));

Actions: View | Formatted Diff | Diff

Attachments on bug 185984: 341294 | 341298