<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[165145] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/165145">165145</a></dd>
<dt>Author</dt> <dd>dbates@webkit.org</dd>
<dt>Date</dt> <dd>2014-03-05 17:02:45 -0800 (Wed, 05 Mar 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre> And Alexey Proskuryakov <ap@apple.com>
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:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreloaderappcacheApplicationCacheGroupcpp">trunk/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp</a></li>
<li><a href="#trunkSourceWebCoreloaderappcacheApplicationCacheStoragecpp">trunk/Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp</a></li>
<li><a href="#trunkSourceWebCoreloaderappcacheApplicationCacheStorageh">trunk/Source/WebCore/loader/appcache/ApplicationCacheStorage.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (165144 => 165145)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2014-03-06 00:37:33 UTC (rev 165144)
+++ trunk/Source/WebCore/ChangeLog 2014-03-06 01:02:45 UTC (rev 165145)
</span><span class="lines">@@ -1,3 +1,39 @@
</span><ins>+2014-03-05 Daniel Bates <dabates@apple.com>
+ And Alexey Proskuryakov <ap@apple.com>
+
+ 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:
+
</ins><span class="cx"> 2014-03-05 Oliver Hunt <oliver@apple.com>
</span><span class="cx">
</span><span class="cx"> Support caching of custom setters
</span></span></pre></div>
<a id="trunkSourceWebCoreloaderappcacheApplicationCacheGroupcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp (165144 => 165145)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp 2014-03-06 00:37:33 UTC (rev 165144)
+++ trunk/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp 2014-03-06 01:02:45 UTC (rev 165145)
</span><span class="lines">@@ -902,7 +902,9 @@
</span><span class="cx"> // a failure of the cache storage to save the newest cache due to hitting
</span><span class="cx"> // the maximum size. In such a case, m_manifestResource may be 0, as
</span><span class="cx"> // the manifest was already set on the newest cache object.
</span><del>- ASSERT(cacheStorage().isMaximumSizeReached() && m_calledReachedMaxAppCacheSize);
</del><ins>+ ASSERT(m_cacheBeingUpdated->manifestResource());
+ ASSERT(cacheStorage().isMaximumSizeReached());
+ ASSERT(m_calledReachedMaxAppCacheSize);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> RefPtr<ApplicationCache> oldNewestCache = (m_newestCache == m_cacheBeingUpdated) ? RefPtr<ApplicationCache>() : m_newestCache;
</span></span></pre></div>
<a id="trunkSourceWebCoreloaderappcacheApplicationCacheStoragecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp (165144 => 165145)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp 2014-03-06 00:37:33 UTC (rev 165144)
+++ trunk/Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp 2014-03-06 01:02:45 UTC (rev 165145)
</span><span class="lines">@@ -694,6 +694,12 @@
</span><span class="cx"> ASSERT(group->storageID() == 0);
</span><span class="cx"> ASSERT(journal);
</span><span class="cx">
</span><ins>+ // 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());
+
</ins><span class="cx"> SQLiteStatement statement(m_database, "INSERT INTO CacheGroups (manifestHostHash, manifestURL, origin) VALUES (?, ?, ?)");
</span><span class="cx"> if (statement.prepare() != SQLResultOk)
</span><span class="cx"> return false;
</span><span class="lines">@@ -1175,7 +1181,12 @@
</span><span class="cx">
</span><span class="cx"> if (result != SQLResultDone)
</span><span class="cx"> LOG_ERROR("Could not load cache resources, error \"%s\"", m_database.lastErrorMsg());
</span><del>-
</del><ins>+
+ if (!cache->manifestResource()) {
+ LOG_ERROR("Could not load application cache because there was no manifest resource");
+ return nullptr;
+ }
+
</ins><span class="cx"> // Load the online whitelist
</span><span class="cx"> SQLiteStatement whitelistStatement(m_database, "SELECT url FROM CacheWhitelistURLs WHERE cache=?");
</span><span class="cx"> if (whitelistStatement.prepare() != SQLResultOk)
</span><span class="lines">@@ -1417,6 +1428,36 @@
</span><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+bool ApplicationCacheStorage::deleteCacheGroupRecord(const String& manifestURL)
+{
+ ASSERT(SQLiteDatabaseTracker::hasTransactionInProgress());
+ 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;
+}
+
</ins><span class="cx"> bool ApplicationCacheStorage::deleteCacheGroup(const String& manifestURL)
</span><span class="cx"> {
</span><span class="cx"> SQLiteTransactionInProgressAutoCounter transactionCounter;
</span><span class="lines">@@ -1431,36 +1472,10 @@
</span><span class="cx"> openDatabase(false);
</span><span class="cx"> if (!m_database.isOpen())
</span><span class="cx"> return false;
</span><del>-
- SQLiteStatement idStatement(m_database, "SELECT id FROM CacheGroups WHERE manifestURL=?");
- if (idStatement.prepare() != SQLResultOk)
</del><ins>+ if (!deleteCacheGroupRecord(manifestURL)) {
+ LOG_ERROR("Could not delete cache group record, error \"%s\"", m_database.lastErrorMsg());
</ins><span class="cx"> return false;
</span><del>-
- 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;
</del><span class="cx"> }
</span><del>-
- 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);
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> deleteTransaction.commit();
</span></span></pre></div>
<a id="trunkSourceWebCoreloaderappcacheApplicationCacheStorageh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/appcache/ApplicationCacheStorage.h (165144 => 165145)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/loader/appcache/ApplicationCacheStorage.h 2014-03-06 00:37:33 UTC (rev 165144)
+++ trunk/Source/WebCore/loader/appcache/ApplicationCacheStorage.h 2014-03-06 01:02:45 UTC (rev 165145)
</span><span class="lines">@@ -110,6 +110,7 @@
</span><span class="cx"> bool store(ApplicationCacheGroup*, GroupStorageIDJournal*);
</span><span class="cx"> bool store(ApplicationCache*, ResourceStorageIDJournal*);
</span><span class="cx"> bool store(ApplicationCacheResource*, unsigned cacheStorageID);
</span><ins>+ bool deleteCacheGroupRecord(const String& manifestURL);
</ins><span class="cx">
</span><span class="cx"> bool ensureOriginRecord(const SecurityOrigin*);
</span><span class="cx"> bool shouldStoreResourceAsFlatFile(ApplicationCacheResource*);
</span></span></pre>
</div>
</div>
</body>
</html>