Attachment 341294 Details for Bug 185984 – Patch

[patch] Patch

bug-185984-20180525093620.patch (text/plain), 15.45 KB, created by Chris Dumez on 2018-05-25 09:36:20 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-05-25 09:36:20 PDT

Size: 15.45 KB

patch

obsolete

>Subversion Revision: 232188
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f1ee6fc4351efc493a26502ebb3f7dc799819a43..a7bc1265d9fcce6cc5643c4ac1171834631e7890 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 NOBODY (OOPS!).
>+
>+        * 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..9348e963f6bbb6e91be5a8e5c3d3fe6c36bd0c64 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 NOBODY (OOPS!).
>+
>+        * 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..0041b852c06f6b78b5d6e9a0c3a278ef468d4a08 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 NOBODY (OOPS!).
>+
>+        * 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..519956dbfad906325222290d76b651da5bda6cc1 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 NOBODY (OOPS!).
>+
>+        * 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..edcb8c28df2ee9310d80a9c52b0a08fed29d26c7 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,11 +1313,10 @@ bool ApplicationCacheStorage::writeDataToUniqueFileInDirectory(SharedBuffer& dat
>     return true;
> }
> 
>-bool ApplicationCacheStorage::getManifestURLs(Vector<URL>* urls)
>+bool ApplicationCacheStorage::getManifestURLs(Vector<URL>& urls)
> {
>     SQLiteTransactionInProgressAutoCounter transactionCounter;
> 
>-    ASSERT(urls);
>     openDatabase(false);
>     if (!m_database.isOpen())
>         return false;
>@@ -1333,39 +1327,11 @@ bool ApplicationCacheStorage::getManifestURLs(Vector<URL>* urls)
>         return false;
> 
>     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;
>-}
>-
> bool ApplicationCacheStorage::deleteCacheGroupRecord(const String& manifestURL)
> {
>     ASSERT(SQLiteDatabaseTracker::hasTransactionInProgress());
>@@ -1403,8 +1369,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 +1471,17 @@ long long ApplicationCacheStorage::flatFileAreaSize()
>     return totalSize;
> }
> 
>-void ApplicationCacheStorage::getOriginsWithCache(HashSet<RefPtr<SecurityOrigin>>& origins)
>+HashSet<RefPtr<SecurityOrigin>> ApplicationCacheStorage::originsWithCache()
> {
>     Vector<URL> urls;
>-    getManifestURLs(&urls);
>+    getManifestURLs(urls);
> 
>     // 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.
>+    HashSet<RefPtr<SecurityOrigin>> origins;
>     for (auto& url : urls)
>         origins.add(SecurityOrigin::create(url));
>+    return origins;
> }
> 
> void ApplicationCacheStorage::deleteAllEntries()
>@@ -1525,9 +1492,7 @@ void ApplicationCacheStorage::deleteAllEntries()
> 
> void ApplicationCacheStorage::deleteAllCaches()
> {
>-    HashSet<RefPtr<SecurityOrigin>> origins;
>-
>-    getOriginsWithCache(origins);
>+    auto origins = originsWithCache();
>     for (auto& origin : origins)
>         deleteCacheForOrigin(*origin);
> 
>@@ -1537,7 +1502,7 @@ void ApplicationCacheStorage::deleteAllCaches()
> void ApplicationCacheStorage::deleteCacheForOrigin(const SecurityOrigin& securityOrigin)
> {
>     Vector<URL> urls;
>-    if (!getManifestURLs(&urls)) {
>+    if (!getManifestURLs(urls)) {
>         LOG_ERROR("Failed to retrieve ApplicationCache manifest URLs");
>         return;
>     }
>@@ -1565,15 +1530,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..a5e1465401e3bceba445b83a36c466b5d59a3efe 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 HashSet<RefPtr<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);
>+    bool getManifestURLs(Vector<URL>& urls);
>+    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..d1cfc64777f3a36d0ab7dc313dca857fe4a09962 100644
>--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>@@ -447,9 +447,8 @@ 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;
>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..f6b5445ef70527330265b1d7e2272f56496bc986 100644
>--- a/Source/WebKitLegacy/mac/WebCoreSupport/WebApplicationCache.mm
>+++ b/Source/WebKitLegacy/mac/WebCoreSupport/WebApplicationCache.mm
>@@ -123,8 +123,7 @@ + (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];
>     
>diff --git a/Source/WebKitLegacy/win/WebApplicationCache.cpp b/Source/WebKitLegacy/win/WebApplicationCache.cpp
>index 1ea90178ad8d38f2408775772b9792513aca403d..8ad3e2e8c4117ae010a2e4d4ba762328a955e4d7 100644
>--- a/Source/WebKitLegacy/win/WebApplicationCache.cpp
>+++ b/Source/WebKitLegacy/win/WebApplicationCache.cpp
>@@ -187,8 +187,7 @@ 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));
>

Actions: View | Formatted Diff | Diff

Attachments on bug 185984: 341294 | 341298