Modified: branches/safari-537.75-branch/Source/WebCore/ChangeLog (165209 => 165210)
--- branches/safari-537.75-branch/Source/WebCore/ChangeLog 2014-03-06 21:27:30 UTC (rev 165209)
+++ branches/safari-537.75-branch/Source/WebCore/ChangeLog 2014-03-06 21:33:11 UTC (rev 165210)
@@ -1,5 +1,45 @@
2014-03-06 Matthew Hanson <[email protected]>
+ Merge r165145.
+
+ 2014-03-05 Daniel Bates <[email protected]>
+ And Alexey Proskuryakov <[email protected]>
+
+ ASSERT(newestManifest) fails in WebCore::ApplicationCacheGroup::didFinishLoadingManifest()
+ https://bugs.webkit.org/show_bug.cgi?id=129753
+ <rdar://problem/12069835>
+
+ Reviewed by Alexey Proskuryakov.
+
+ Fixes an issue where an assertion failure would occur when visiting a web site whose on-disk
+ app cache doesn't contain a manifest resource.
+
+ For some reason an app cache for a web site may be partially written to disk. In particular, the
+ app cache may only contain a CacheGroups entry. That is, the manifest resource and origin records
+ may not be persisted to disk. From looking over the code, we're unclear how such a situation can occur
+ and hence have been unable to create such an app cache. We were able to reproduce this issue using
+ an app cache database file that was provided by a person that was affected by this issue.
+
+ No test included because it's not straightforward to write a test for this change.
+
+ * loader/appcache/ApplicationCacheGroup.cpp:
+ (WebCore::ApplicationCacheGroup::checkIfLoadIsComplete): Assert that m_cacheBeingUpdated->manifestResource()
+ is non-null. Currently we only document this assumption in a code comment. Also separated a single assertion
+ _expression_ into two assertion expressions to make it straightforward to identify the failing sub-_expression_
+ on failure.
+ * loader/appcache/ApplicationCacheStorage.cpp:
+ (WebCore::ApplicationCacheStorage::store): Modified to call ApplicationCacheStorage::deleteCacheGroupRecord()
+ to remove a cache group and associated cache records (if applicable) before inserting a cache group entry.
+ This replacement approach will ultimately repair incomplete app cache data for people affected by this bug.
+ (WebCore::ApplicationCacheStorage::loadCache): Log an error and return nullptr if the cache we loaded doesn't
+ have a manifest resource.
+ (WebCore::ApplicationCacheStorage::deleteCacheGroupRecord): Added.
+ (WebCore::ApplicationCacheStorage::deleteCacheGroup): Extracted deletion logic for cache group record into
+ ApplicationCacheStorage::deleteCacheGroupRecord().
+ * loader/appcache/ApplicationCacheStorage.h:
+
+2014-03-06 Matthew Hanson <[email protected]>
+
Merge r156716.
2013-10-01 Myles C. Maxfield <[email protected]>
Modified: branches/safari-537.75-branch/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp (165209 => 165210)
--- branches/safari-537.75-branch/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp 2014-03-06 21:27:30 UTC (rev 165209)
+++ branches/safari-537.75-branch/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp 2014-03-06 21:33:11 UTC (rev 165210)
@@ -926,7 +926,9 @@
// a failure of the cache storage to save the newest cache due to hitting
// the maximum size. In such a case, m_manifestResource may be 0, as
// the manifest was already set on the newest cache object.
- ASSERT(cacheStorage().isMaximumSizeReached() && m_calledReachedMaxAppCacheSize);
+ ASSERT(m_cacheBeingUpdated->manifestResource());
+ ASSERT(cacheStorage().isMaximumSizeReached());
+ ASSERT(m_calledReachedMaxAppCacheSize);
}
RefPtr<ApplicationCache> oldNewestCache = (m_newestCache == m_cacheBeingUpdated) ? RefPtr<ApplicationCache>() : m_newestCache;
Modified: branches/safari-537.75-branch/Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp (165209 => 165210)
--- branches/safari-537.75-branch/Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp 2014-03-06 21:27:30 UTC (rev 165209)
+++ branches/safari-537.75-branch/Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp 2014-03-06 21:33:11 UTC (rev 165210)
@@ -675,6 +675,12 @@
ASSERT(group->storageID() == 0);
ASSERT(journal);
+ // For some reason, an app cache may be partially written to disk. In particular, there may be
+ // a cache group with an identical manifest URL and associated cache entries. We want to remove
+ // this cache group and its associated cache entries so that we can create it again (below) as
+ // a way to repair it.
+ deleteCacheGroupRecord(group->manifestURL());
+
SQLiteStatement statement(m_database, "INSERT INTO CacheGroups (manifestHostHash, manifestURL, origin) VALUES (?, ?, ?)");
if (statement.prepare() != SQLResultOk)
return false;
@@ -1149,7 +1155,12 @@
if (result != SQLResultDone)
LOG_ERROR("Could not load cache resources, error \"%s\"", m_database.lastErrorMsg());
-
+
+ if (!cache->manifestResource()) {
+ LOG_ERROR("Could not load application cache because there was no manifest resource");
+ return nullptr;
+ }
+
// Load the online whitelist
SQLiteStatement whitelistStatement(m_database, "SELECT url FROM CacheWhitelistURLs WHERE cache=?");
if (whitelistStatement.prepare() != SQLResultOk)
@@ -1381,6 +1392,35 @@
return true;
}
+bool ApplicationCacheStorage::deleteCacheGroupRecord(const String& manifestURL)
+{
+ SQLiteStatement idStatement(m_database, "SELECT id FROM CacheGroups WHERE manifestURL=?");
+ if (idStatement.prepare() != SQLResultOk)
+ return false;
+
+ idStatement.bindText(1, manifestURL);
+
+ int result = idStatement.step();
+ if (result != SQLResultRow)
+ return false;
+
+ int64_t groupId = idStatement.getColumnInt64(0);
+
+ SQLiteStatement cacheStatement(m_database, "DELETE FROM Caches WHERE cacheGroup=?");
+ if (cacheStatement.prepare() != SQLResultOk)
+ return false;
+
+ SQLiteStatement groupStatement(m_database, "DELETE FROM CacheGroups WHERE id=?");
+ if (groupStatement.prepare() != SQLResultOk)
+ return false;
+
+ cacheStatement.bindInt64(1, groupId);
+ executeStatement(cacheStatement);
+ groupStatement.bindInt64(1, groupId);
+ executeStatement(groupStatement);
+ return true;
+}
+
bool ApplicationCacheStorage::deleteCacheGroup(const String& manifestURL)
{
SQLiteTransaction deleteTransaction(m_database);
@@ -1393,36 +1433,10 @@
openDatabase(false);
if (!m_database.isOpen())
return false;
-
- SQLiteStatement idStatement(m_database, "SELECT id FROM CacheGroups WHERE manifestURL=?");
- if (idStatement.prepare() != SQLResultOk)
+ if (!deleteCacheGroupRecord(manifestURL)) {
+ LOG_ERROR("Could not delete cache group record, error \"%s\"", m_database.lastErrorMsg());
return false;
-
- idStatement.bindText(1, manifestURL);
-
- int result = idStatement.step();
- if (result == SQLResultDone)
- return false;
-
- if (result != SQLResultRow) {
- LOG_ERROR("Could not load cache group id, error \"%s\"", m_database.lastErrorMsg());
- return false;
}
-
- int64_t groupId = idStatement.getColumnInt64(0);
-
- SQLiteStatement cacheStatement(m_database, "DELETE FROM Caches WHERE cacheGroup=?");
- if (cacheStatement.prepare() != SQLResultOk)
- return false;
-
- SQLiteStatement groupStatement(m_database, "DELETE FROM CacheGroups WHERE id=?");
- if (groupStatement.prepare() != SQLResultOk)
- return false;
-
- cacheStatement.bindInt64(1, groupId);
- executeStatement(cacheStatement);
- groupStatement.bindInt64(1, groupId);
- executeStatement(groupStatement);
}
deleteTransaction.commit();
Modified: branches/safari-537.75-branch/Source/WebCore/loader/appcache/ApplicationCacheStorage.h (165209 => 165210)
--- branches/safari-537.75-branch/Source/WebCore/loader/appcache/ApplicationCacheStorage.h 2014-03-06 21:27:30 UTC (rev 165209)
+++ branches/safari-537.75-branch/Source/WebCore/loader/appcache/ApplicationCacheStorage.h 2014-03-06 21:33:11 UTC (rev 165210)
@@ -110,6 +110,7 @@
bool store(ApplicationCacheGroup*, GroupStorageIDJournal*);
bool store(ApplicationCache*, ResourceStorageIDJournal*);
bool store(ApplicationCacheResource*, unsigned cacheStorageID);
+ bool deleteCacheGroupRecord(const String& manifestURL);
bool ensureOriginRecord(const SecurityOrigin*);
bool shouldStoreResourceAsFlatFile(ApplicationCacheResource*);
