Bug 1343916 - CacheFile::Truncate doesn't update hashes in CacheFileMetadata, r=valentin

netwerk/cache2/CacheFile.cpp

@@ -885,6 +885,8 @@ CacheFile::OpenOutputStream(CacheOutputCloseListener *aCloseListener, nsIOutputS
     // Remove alt-data
     rv = Truncate(mAltDataOffset);
     if (NS_FAILED(rv)) {
+      LOG(("CacheFile::OpenOutputStream() - Truncating alt-data failed "
+           "[rv=0x%08" PRIx32 "]", static_cast<uint32_t>(rv)));
       return rv;
     }
     mMetadata->SetElement(CacheFileUtils::kAltDataKey, nullptr);
@@ -943,6 +945,8 @@ CacheFile::OpenAlternativeOutputStream(CacheOutputCloseListener *aCloseListener,
     // Truncate old alt-data
     rv = Truncate(mAltDataOffset);
     if (NS_FAILED(rv)) {
+      LOG(("CacheFile::OpenAlternativeOutputStream() - Truncating old alt-data "
+           "failed [rv=0x%08" PRIx32 "]", static_cast<uint32_t>(rv)));
       return rv;
     }
   } else {
@@ -1798,56 +1802,98 @@ CacheFile::Truncate(int64_t aOffset)
 {
   AssertOwnsLock();
 
+  LOG(("CacheFile::Truncate() [this=%p, offset=%" PRId64 "]", this, aOffset));
+
   nsresult rv;
 
   MOZ_ASSERT(aOffset <= mDataSize);
   MOZ_ASSERT(mReady);
 
   uint32_t lastChunk = 0;
-
-  if (aOffset > 0) {
+  if (mDataSize > 0) {
     lastChunk = (mDataSize - 1) / kChunkSize;
   }
 
-  uint32_t bytesInLastChunk = aOffset - lastChunk * kChunkSize;
+  uint32_t newLastChunk = 0;
+  if (aOffset > 0) {
+    newLastChunk = (aOffset - 1) / kChunkSize;
+  }
 
+  uint32_t bytesInNewLastChunk = aOffset - newLastChunk * kChunkSize;
+
+  // Remove all truncated chunks from mCachedChunks
   for (auto iter = mCachedChunks.Iter(); !iter.Done(); iter.Next()) {
     uint32_t idx = iter.Key();
 
-    if (idx > lastChunk) {
+    if (idx > newLastChunk) {
       // This is unused chunk, simply remove it.
+      LOG(("CacheFile::Truncate() - removing cached chunk [idx=%u]", idx));
       iter.Remove();
-      continue;
-    }
-
-    if (idx == lastChunk) {
-      RefPtr<CacheFileChunk>& chunk = iter.Data();
-
-      rv = chunk->Truncate(bytesInLastChunk);
-      if (NS_FAILED(rv)) {
-        return rv;
-      }
     }
   }
 
+  // Discard all truncated chunks in mChunks
   for (auto iter = mChunks.Iter(); !iter.Done(); iter.Next()) {
     uint32_t idx = iter.Key();
-    RefPtr<CacheFileChunk>& chunk = iter.Data();
 
-    if (idx > lastChunk) {
+    if (idx > newLastChunk) {
+      RefPtr<CacheFileChunk>& chunk = iter.Data();
+      LOG(("CacheFile::Truncate() - discarding chunk [idx=%u, chunk=%p]",
+           idx, chunk.get()));
       chunk->mDiscardedChunk = true;
       mDiscardedChunks.AppendElement(chunk);
       iter.Remove();
-      continue;
     }
+  }
 
-    if (idx == lastChunk) {
-      RefPtr<CacheFileChunk>& chunk = iter.Data();
+  // Remove hashes of all removed chunks from the metadata
+  for (uint32_t i = lastChunk; i > newLastChunk; --i) {
+    mMetadata->RemoveHash(i);
+  }
 
-      rv = chunk->Truncate(bytesInLastChunk);
+  // Truncate new last chunk
+  if (bytesInNewLastChunk == kChunkSize) {
+    LOG(("CacheFile::Truncate() - not truncating last chunk."));
+  } else {
+    RefPtr<CacheFileChunk> chunk;
+    if (mChunks.Get(newLastChunk, getter_AddRefs(chunk))) {
+      LOG(("CacheFile::Truncate() - New last chunk %p got from mChunks.",
+           chunk.get()));
+    } else if (mCachedChunks.Get(newLastChunk, getter_AddRefs(chunk))) {
+      LOG(("CacheFile::Truncate() - New last chunk %p got from mCachedChunks.",
+           chunk.get()));
+    } else {
+      // New last chunk isn't loaded but we need to update the hash.
+      MOZ_ASSERT(!mMemoryOnly);
+      MOZ_ASSERT(mHandle);
+
+      rv = GetChunkLocked(newLastChunk, PRELOADER, nullptr,
+                          getter_AddRefs(chunk));
       if (NS_FAILED(rv)) {
         return rv;
       }
+      // We've checked that we don't have this chunk, so no chunk must be
+      // returned.
+      MOZ_ASSERT(!chunk);
+
+      if (!mChunks.Get(newLastChunk, getter_AddRefs(chunk))) {
+        return NS_ERROR_UNEXPECTED;
+      }
+
+      LOG(("CacheFile::Truncate() - New last chunk %p got from preloader.",
+           chunk.get()));
+    }
+
+    rv = chunk->Truncate(bytesInNewLastChunk);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+
+    // If the chunk is ready set the new hash now. If it's still being loaded
+    // CacheChunk::Truncate() made the chunk dirty and the hash will be updated
+    // in OnChunkWritten().
+    if (chunk->IsReady()) {
+      mMetadata->SetHash(newLastChunk, chunk->Hash());
     }
   }
 

netwerk/cache2/CacheFileChunk.cpp

@@ -128,7 +128,7 @@ CacheFileChunkBuffer::SetDataSize(uint32_t aDataSize)
     // empty (see CacheFileChunk::Read). We need to set mBuf::mDataSize
     // accordingly so that DataSize() methods return correct value, but we don't
     // want to allocate the buffer since it wouldn't be used in most cases.
-    (mDataSize == 0 && mBufSize == 0 && mChunk->mState == CacheFileChunk::READING));
+    (mBufSize == 0 && mChunk->mState == CacheFileChunk::READING));
 
   mDataSize = aDataSize;
 }
@@ -565,8 +565,6 @@ CacheFileChunk::Index() const
 CacheHash::Hash16_t
 CacheFileChunk::Hash() const
 {
-  AssertOwnsLock();
-
   MOZ_ASSERT(!mListener);
   MOZ_ASSERT(IsReady());
 
@@ -626,6 +624,12 @@ CacheFileChunk::UpdateDataSize(uint32_t aOffset, uint32_t aLen)
 nsresult
 CacheFileChunk::Truncate(uint32_t aOffset)
 {
+  MOZ_RELEASE_ASSERT(mState == READY || mState == WRITING || mState == READING);
+
+  if (mState == READING) {
+    mIsDirty = true;
+  }
+
   mBuf->SetDataSize(aOffset);
   return NS_OK;
 }
@@ -697,6 +701,11 @@ CacheFileChunk::OnDataRead(CacheFileHandle *aHandle, char *aBuf,
              hash, mExpectedHash, this, mIndex));
         aResult = NS_ERROR_FILE_CORRUPTED;
       } else {
+        if (mBuf->DataSize() < tmpBuf->DataSize()) {
+          // Truncate() was called while the data was being read.
+          tmpBuf->SetDataSize(mBuf->DataSize());
+        }
+
         if (!mBuf->Buf()) {
           // Just swap the buffers if mBuf is still empty
           mBuf.swap(tmpBuf);
@@ -756,8 +765,6 @@ CacheFileChunk::IsKilled()
 bool
 CacheFileChunk::IsReady() const
 {
-  AssertOwnsLock();
-
   return (NS_SUCCEEDED(mStatus) && (mState == READY || mState == WRITING));
 }
 

netwerk/cache2/CacheFileMetadata.cpp

@@ -533,6 +533,23 @@ CacheFileMetadata::SetHash(uint32_t aIndex, CacheHash::Hash16_t aHash)
   return NS_OK;
 }
 
+nsresult
+CacheFileMetadata::RemoveHash(uint32_t aIndex)
+{
+  LOG(("CacheFileMetadata::RemoveHash() [this=%p, idx=%d]", this, aIndex));
+
+  MarkDirty();
+
+  MOZ_ASSERT((aIndex + 1) == mHashCount, "Can remove only last hash!");
+
+  if (aIndex + 1 != mHashCount) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
+  mHashCount--;
+  return NS_OK;
+}
+
 nsresult
 CacheFileMetadata::AddFlags(uint32_t aFlags)
 {

netwerk/cache2/CacheFileMetadata.h

@@ -156,6 +156,7 @@ public:
 
   CacheHash::Hash16_t GetHash(uint32_t aIndex);
   nsresult            SetHash(uint32_t aIndex, CacheHash::Hash16_t aHash);
+  nsresult            RemoveHash(uint32_t aIndex);
 
   nsresult AddFlags(uint32_t aFlags);
   nsresult RemoveFlags(uint32_t aFlags);