Bug 405693 - "don't update identical offline cache manifests" [p=honzab@allpeers.com (Honza Bambas) r=dcamp sr=biesi a=blocking1.9+]
authorreed@reedloden.com
Mon, 28 Jan 2008 23:54:54 -0800
changeset 10900 19c7c4c69d18ec44efda473ead7a318441b4516e
parent 10899 11e5451f690b4a6a377dc16efe32c5f5a6edfd70
child 10901 8f739f9a99af3e342b7536966e4966ccf61afe87
push idunknown
push userunknown
push dateunknown
reviewersdcamp, biesi, blocking1.9
bugs405693
milestone1.9b3pre
Bug 405693 - "don't update identical offline cache manifests" [p=honzab@allpeers.com (Honza Bambas) r=dcamp sr=biesi a=blocking1.9+]
dom/tests/mochitest/ajax/offline/Makefile.in
dom/tests/mochitest/ajax/offline/test_identicalManifest.html
netwerk/base/public/nsICachingChannel.idl
netwerk/protocol/http/src/nsHttpChannel.cpp
uriloader/prefetch/nsOfflineCacheUpdate.cpp
uriloader/prefetch/nsOfflineCacheUpdate.h
--- a/dom/tests/mochitest/ajax/offline/Makefile.in
+++ b/dom/tests/mochitest/ajax/offline/Makefile.in
@@ -46,16 +46,17 @@ include $(DEPTH)/config/autoconf.mk
 include $(topsrcdir)/config/rules.mk
 
 _TEST_FILES	= \
 	offlineTests.js \
 	test_badManifestMagic.html \
 	test_badManifestMime.html \
 	test_missingFile.html \
 	test_simpleManifest.html \
+	test_identicalManifest.html \
 	test_offlineIFrame.html \
 	badManifestMagic.cacheManifest \
 	badManifestMagic.cacheManifest^headers^ \
 	missingFile.cacheManifest \
 	missingFile.cacheManifest^headers^ \
 	simpleManifest.cacheManifest \
 	simpleManifest.cacheManifest^headers^ \
 	simpleManifest.notmanifest \
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/ajax/offline/test_identicalManifest.html
@@ -0,0 +1,79 @@
+<html xmlns="http://www.w3.org/1999/xhtml" manifest="http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/simpleManifest.cacheManifest">
+<head>
+<title>identical manifest test</title>
+
+<script type="text/javascript" src="/MochiKit/packed.js"></script>
+<script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+<script type="text/javascript" src="/tests/dom/tests/mochitest/ajax/offline/offlineTests.js"></script>
+
+<script type="text/javascript">
+
+var gGotChecking = false;
+var gGotDownloading = false;
+
+function noUpdate()
+{
+  OfflineTest.ok(gGotChecking, "Should get a checking event");
+  OfflineTest.ok(!gGotDownloading, "Should not get a downloading event");
+
+  // The document that requested the manifest should be in the cache
+  OfflineTest.checkCache(window.location.href, true);
+
+  OfflineTest.checkCache("http://localhost:8888/tests/SimpleTest/SimpleTest.js", true);
+  OfflineTest.checkCache("http://localhost:8888/MochiKit/packed.js", true);
+  OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/offlineTests.js", true);
+  
+  OfflineTest.teardown();
+
+  OfflineTest.finish();
+}
+
+function manifestUpdated()
+{
+  OfflineTest.ok(gGotChecking, "Should get a checking event");
+  OfflineTest.ok(gGotDownloading, "Should get a downloading event");
+
+  // The manifest itself should be in the cache
+  OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/simpleManifest.cacheManifest", true);
+
+  // The document that requested the manifest should be in the cache
+  OfflineTest.checkCache(window.location.href, true);
+
+  // The entries from the manifest should be in the cache
+  OfflineTest.checkCache("http://localhost:8888/tests/SimpleTest/SimpleTest.js", true);
+  OfflineTest.checkCache("http://localhost:8888/MochiKit/packed.js", true);
+  OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/offlineTests.js", true);
+
+  // The bad entries from the manifest should not be in the cache
+  OfflineTest.checkCache("https://localhost:8888/MochiKit/packed.js", false);
+  OfflineTest.checkCache("bad:/uri/invalid", false);
+
+  // Now make sure applicationCache.update() does what we expect.
+  applicationCache.oncached = OfflineTest.failEvent;
+  applicationCache.onnoupdate = OfflineTest.priv(noUpdate);
+  
+  gGotChecking = false;
+  gGotDownloading = false;
+  applicationCache.update();
+}
+
+
+if (OfflineTest.setup()) {
+  applicationCache.onerror = OfflineTest.failEvent;
+  applicationCache.onnoupdate = OfflineTest.failEvent;
+
+  applicationCache.onchecking = function() { gGotChecking = true; };
+  applicationCache.ondownloading = function() { gGotDownloading = true; };
+  applicationCache.oncached = OfflineTest.priv(manifestUpdated);
+}
+
+SimpleTest.waitForExplicitFinish();
+
+</script>
+
+</head>
+
+<body>
+
+</body>
+</html>
--- a/netwerk/base/public/nsICachingChannel.idl
+++ b/netwerk/base/public/nsICachingChannel.idl
@@ -45,17 +45,17 @@ interface nsIFile;
  * to affect its behavior with respect to how it uses the cache service.
  *
  * This interface provides:
  *   1) Support for "stream as file" semantics (for JAR and plugins).
  *   2) Support for "pinning" cached data in the cache (for printing and save-as).
  *   3) Support for uniquely identifying cached data in cases when the URL
  *      is insufficient (e.g., HTTP form submission).
  */
-[scriptable, uuid(09556ba7-b13d-47d2-b154-fe690b063899)]
+[scriptable, uuid(830D4BCB-3E46-4011-9BDA-51A5D1AF891F)]
 interface nsICachingChannel : nsISupports
 {
     /**
      * Set/get the cache token... uniquely identifies the data in the cache.
      * Holding a reference to this token prevents the cached data from being
      * removed.
      * 
      * A cache token retrieved from a particular instance of nsICachingChannel
@@ -65,16 +65,25 @@ interface nsICachingChannel : nsISupport
      * identified by the cache token and not try to validate it.
      *
      * The cache token can be QI'd to a nsICacheEntryInfo if more detail
      * about the cache entry is needed (e.g., expiration time).
      */
     attribute nsISupports cacheToken;
 
     /**
+     * The same as above but accessing the offline app cache token if there
+     * is any.
+     *
+     * @throws
+     *      NS_ERROR_NOT_AVAILABLE when there is not offline cache token
+     */
+    attribute nsISupports offlineCacheToken;
+
+    /**
      * Set/get the cache key... uniquely identifies the data in the cache
      * for this channel.  Holding a reference to this key does NOT prevent
      * the cached data from being removed.
      * 
      * A cache key retrieved from a particular instance of nsICachingChannel
      * could be set on another instance of nsICachingChannel provided the
      * underlying implementations are compatible and provided the new 
      * channel instance was created with the same URI.  The implementation of
--- a/netwerk/protocol/http/src/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/src/nsHttpChannel.cpp
@@ -1379,31 +1379,37 @@ nsHttpChannel::OpenCacheEntry(PRBool off
         accessRequested = nsICache::ACCESS_READ;
     }
     else if (BYPASS_LOCAL_CACHE(mLoadFlags))
         accessRequested = nsICache::ACCESS_WRITE; // replace cache entry
     else
         accessRequested = nsICache::ACCESS_READ_WRITE; // normal browsing
 
     nsCOMPtr<nsICacheSession> session;
-    rv = gHttpHandler->GetCacheSession(storagePolicy,
-                                       getter_AddRefs(session));
-    if (NS_FAILED(rv)) return rv;
-
-    // we'll try to synchronously open the cache entry... however, it may be
-    // in use and not yet validated, in which case we'll try asynchronously
-    // opening the cache entry.
-    rv = session->OpenCacheEntry(cacheKey, accessRequested, PR_FALSE,
-                                 getter_AddRefs(mCacheEntry));
-
-    if ((mLoadFlags & LOAD_CHECK_OFFLINE_CACHE) &&
-        !(NS_SUCCEEDED(rv) || rv == NS_ERROR_CACHE_WAIT_FOR_VALIDATION)) {
-        // couldn't find it in the main cache, check the offline cache
-
-        storagePolicy = nsICache::STORE_OFFLINE;
+    if (mLoadFlags & LOAD_CHECK_OFFLINE_CACHE) {
+        // when LOAD_CHECK_OFFLINE_CACHE set prefer the offline cache
+        rv = gHttpHandler->GetCacheSession(nsICache::STORE_OFFLINE,
+                                           getter_AddRefs(session));
+        if (NS_FAILED(rv)) return rv;
+
+        // we'll try to synchronously open the cache entry... however, it may be
+        // in use and not yet validated, in which case we'll try asynchronously
+        // opening the cache entry.
+        //
+        // we need open in ACCESS_READ only because we don't want to overwrite
+        // the offline cache entry non-atomically so ACCESS_READ
+        // will prevent us from writing to the HTTP-offline cache as a
+        // normal cache entry.
+        rv = session->OpenCacheEntry(cacheKey, nsICache::ACCESS_READ, PR_FALSE,
+                                     getter_AddRefs(mCacheEntry));
+    }
+
+    if (!(mLoadFlags & LOAD_CHECK_OFFLINE_CACHE) ||
+        (NS_FAILED(rv) && rv != NS_ERROR_CACHE_WAIT_FOR_VALIDATION)) 
+    {
         rv = gHttpHandler->GetCacheSession(storagePolicy,
                                            getter_AddRefs(session));
         if (NS_FAILED(rv)) return rv;
 
         rv = session->OpenCacheEntry(cacheKey, accessRequested, PR_FALSE,
                                      getter_AddRefs(mCacheEntry));
     }
 
@@ -4546,16 +4552,31 @@ nsHttpChannel::GetCacheToken(nsISupports
 
 NS_IMETHODIMP
 nsHttpChannel::SetCacheToken(nsISupports *token)
 {
     return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
+nsHttpChannel::GetOfflineCacheToken(nsISupports **token)
+{
+    NS_ENSURE_ARG_POINTER(token);
+    if (!mOfflineCacheEntry)
+        return NS_ERROR_NOT_AVAILABLE;
+    return CallQueryInterface(mOfflineCacheEntry, token);
+}
+
+NS_IMETHODIMP
+nsHttpChannel::SetOfflineCacheToken(nsISupports *token)
+{
+    return NS_ERROR_NOT_IMPLEMENTED;
+}
+
+NS_IMETHODIMP
 nsHttpChannel::GetCacheKey(nsISupports **key)
 {
     nsresult rv;
     NS_ENSURE_ARG_POINTER(key);
 
     LOG(("nsHttpChannel::GetCacheKey [this=%x]\n", this));
 
     *key = nsnull;
--- a/uriloader/prefetch/nsOfflineCacheUpdate.cpp
+++ b/uriloader/prefetch/nsOfflineCacheUpdate.cpp
@@ -44,16 +44,18 @@
 #include "nsICacheService.h"
 #include "nsICacheSession.h"
 #include "nsICachingChannel.h"
 #include "nsIDOMWindow.h"
 #include "nsIDOMOfflineResourceList.h"
 #include "nsIObserverService.h"
 #include "nsIOfflineCacheSession.h"
 #include "nsIWebProgress.h"
+#include "nsICryptoHash.h"
+#include "nsICacheEntryDescriptor.h"
 #include "nsNetCID.h"
 #include "nsNetUtil.h"
 #include "nsServiceManagerUtils.h"
 #include "nsStreamUtils.h"
 #include "nsThreadUtils.h"
 #include "prlog.h"
 
 static nsOfflineCacheUpdateService *gOfflineCacheUpdateService = nsnull;
@@ -119,17 +121,18 @@ nsOfflineCacheUpdateItem::~nsOfflineCach
 
 nsresult
 nsOfflineCacheUpdateItem::OpenChannel()
 {
     nsresult rv = NS_NewChannel(getter_AddRefs(mChannel),
                                 mURI,
                                 nsnull, nsnull, this,
                                 nsIRequest::LOAD_BACKGROUND |
-                                nsICachingChannel::LOAD_ONLY_IF_MODIFIED);
+                                nsICachingChannel::LOAD_ONLY_IF_MODIFIED |
+                                nsICachingChannel::LOAD_CHECK_OFFLINE_CACHE);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // configure HTTP specific stuff
     nsCOMPtr<nsIHttpChannel> httpChannel =
         do_QueryInterface(mChannel);
     if (httpChannel) {
         httpChannel->SetReferrer(mReferrerURI);
         httpChannel->SetRequestHeader(NS_LITERAL_CSTRING("X-Moz"),
@@ -388,16 +391,17 @@ nsOfflineCacheUpdateItem::GetStatus(PRUi
 
 nsOfflineManifestItem::nsOfflineManifestItem(nsOfflineCacheUpdate *aUpdate,
                                              nsIURI *aURI,
                                              nsIURI *aReferrerURI,
                                              const nsACString &aClientID)
     : nsOfflineCacheUpdateItem(aUpdate, aURI, aReferrerURI, aClientID)
     , mParserState(PARSE_INIT)
     , mNeedsUpdate(PR_TRUE)
+    , mManifestHashInitialized(PR_FALSE)
 {
 }
 
 nsOfflineManifestItem::~nsOfflineManifestItem()
 {
 }
 
 //-----------------------------------------------------------------------------
@@ -411,23 +415,47 @@ nsOfflineManifestItem::ReadManifest(nsII
                                     const char *aFromSegment,
                                     PRUint32 aOffset,
                                     PRUint32 aCount,
                                     PRUint32 *aBytesConsumed)
 {
     nsOfflineManifestItem *manifest =
         static_cast<nsOfflineManifestItem*>(aClosure);
 
+    nsresult rv;
+
     *aBytesConsumed = aCount;
 
     if (manifest->mParserState == PARSE_ERROR) {
         // parse already failed, ignore this
         return NS_OK;
     }
 
+    if (!manifest->mManifestHashInitialized) {
+        // Avoid re-creation of crypto hash when it fails from some reason the first time
+        manifest->mManifestHashInitialized = PR_TRUE;
+
+        manifest->mManifestHash = do_CreateInstance("@mozilla.org/security/hash;1", &rv);
+        if (NS_SUCCEEDED(rv)) {
+            rv = manifest->mManifestHash->Init(nsICryptoHash::MD5);
+            if (NS_FAILED(rv)) {
+                manifest->mManifestHash = nsnull;
+                LOG(("Could not initialize manifest hash for byte-to-byte check, rv=%08x", rv));
+            }
+        }
+    }
+
+    if (manifest->mManifestHash) {
+        rv = manifest->mManifestHash->Update(reinterpret_cast<const PRUint8 *>(aFromSegment), aCount);
+        if (NS_FAILED(rv)) {
+            manifest->mManifestHash = nsnull;
+            LOG(("Could not update manifest hash, rv=%08x", rv));
+        }
+    }
+
     manifest->mReadBuf.Append(aFromSegment, aCount);
 
     nsCString::const_iterator begin, iter, end;
     manifest->mReadBuf.BeginReading(begin);
     manifest->mReadBuf.EndReading(end);
 
     for (iter = begin; iter != end; iter++) {
         if (*iter == '\r' || *iter == '\n') {
@@ -542,16 +570,88 @@ nsOfflineManifestItem::HandleManifestLin
         // ignore these for now.
         break;
     }
     }
 
     return NS_OK;
 }
 
+nsresult 
+nsOfflineManifestItem::GetOldManifestContentHash(nsIRequest *aRequest)
+{
+    nsresult rv;
+
+    nsCOMPtr<nsICachingChannel> cachingChannel = do_QueryInterface(aRequest, &rv);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    // load the main cache token that is actually the old offline cache token and 
+    // read previous manifest content hash value
+    nsCOMPtr<nsISupports> cacheToken;
+    cachingChannel->GetCacheToken(getter_AddRefs(cacheToken));
+    if (cacheToken) {
+        nsCOMPtr<nsICacheEntryDescriptor> cacheDescriptor(do_QueryInterface(cacheToken, &rv));
+        NS_ENSURE_SUCCESS(rv, rv);
+    
+        rv = cacheDescriptor->GetMetaDataElement("offline-manifest-hash", getter_Copies(mOldManifestHashValue));
+        if (NS_FAILED(rv))
+            mOldManifestHashValue.Truncate();
+    }
+
+    return NS_OK;
+}
+
+nsresult 
+nsOfflineManifestItem::CheckNewManifestContentHash(nsIRequest *aRequest)
+{
+    nsresult rv;
+
+    if (!mManifestHash) {
+        // Nothing to compare against...
+        return NS_OK;
+    }
+
+    nsCString newManifestHashValue;
+    rv = mManifestHash->Finish(PR_TRUE, newManifestHashValue);
+    mManifestHash = nsnull;
+
+    if (NS_FAILED(rv)) {
+        LOG(("Could not finish manifest hash, rv=%08x", rv));
+        // This is not critical error
+        return NS_OK;
+    }
+
+    if (!ParseSucceeded()) {
+        // Parsing failed, the hash is not valid
+        return NS_OK;
+    }
+
+    if (mOldManifestHashValue == newManifestHashValue) {
+        LOG(("Update not needed, downloaded manifest content is byte-for-byte identical"));
+        mNeedsUpdate = PR_FALSE;
+    }
+
+    // Store the manifest content hash value to the new
+    // offline cache token
+    nsCOMPtr<nsICachingChannel> cachingChannel = do_QueryInterface(aRequest, &rv);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    nsCOMPtr<nsISupports> cacheToken;
+    cachingChannel->GetOfflineCacheToken(getter_AddRefs(cacheToken));
+    if (cacheToken) {
+        nsCOMPtr<nsICacheEntryDescriptor> cacheDescriptor(do_QueryInterface(cacheToken, &rv));
+        NS_ENSURE_SUCCESS(rv, rv);
+    
+        rv = cacheDescriptor->SetMetaDataElement("offline-manifest-hash", PromiseFlatCString(newManifestHashValue).get());
+        NS_ENSURE_SUCCESS(rv, rv);
+    }
+
+    return NS_OK;
+}
+
 NS_IMETHODIMP
 nsOfflineManifestItem::OnStartRequest(nsIRequest *aRequest,
                                       nsISupports *aContext)
 {
     nsresult rv;
 
     nsCOMPtr<nsIHttpChannel> channel = do_QueryInterface(aRequest, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -572,16 +672,19 @@ nsOfflineManifestItem::OnStartRequest(ns
 
     if (!contentType.EqualsLiteral("text/cache-manifest")) {
         LOG(("Rejected cache manifest with Content-Type %s (expecting text/cache-manifest)",
              contentType.get()));
         mParserState = PARSE_ERROR;
         return NS_ERROR_ABORT;
     }
 
+    rv = GetOldManifestContentHash(aRequest);
+    NS_ENSURE_SUCCESS(rv, rv);
+
     return nsOfflineCacheUpdateItem::OnStartRequest(aRequest, aContext);
 }
 
 NS_IMETHODIMP
 nsOfflineManifestItem::OnDataAvailable(nsIRequest *aRequest,
                                        nsISupports *aContext,
                                        nsIInputStream *aStream,
                                        PRUint32 aOffset,
@@ -616,20 +719,19 @@ nsOfflineManifestItem::OnStopRequest(nsI
     mReadBuf.EndReading(end);
     nsresult rv = HandleManifestLine(begin, end);
     NS_ENSURE_SUCCESS(rv, rv);
 
     if (mBytesRead == 0) {
         // we didn't need to read (because LOAD_ONLY_IF_MODIFIED was
         // specified.)
         mNeedsUpdate = PR_FALSE;
-
-        // XXX: The spec calls for a byte-by-byte comparison of the manifest,
-        // with an exact match skipping the update.  Need to implement this
-        // or we'll be fetching way too often.
+    } else {
+        rv = CheckNewManifestContentHash(aRequest);
+        NS_ENSURE_SUCCESS(rv, rv);
     }
 
     return nsOfflineCacheUpdateItem::OnStopRequest(aRequest, aContext, aStatus);
 }
 
 //-----------------------------------------------------------------------------
 // nsOfflineCacheUpdate::nsISupports
 //-----------------------------------------------------------------------------
--- a/uriloader/prefetch/nsOfflineCacheUpdate.h
+++ b/uriloader/prefetch/nsOfflineCacheUpdate.h
@@ -58,19 +58,22 @@
 #include "nsIRunnable.h"
 #include "nsIStreamListener.h"
 #include "nsIURI.h"
 #include "nsIWebProgressListener.h"
 #include "nsClassHashtable.h"
 #include "nsString.h"
 #include "nsTArray.h"
 #include "nsWeakReference.h"
+#include "nsICryptoHash.h"
 
 class nsOfflineCacheUpdate;
 
+class nsICacheEntryDescriptor;
+
 class nsOfflineCacheUpdateItem : public nsIDOMLoadStatus
                                , public nsIStreamListener
                                , public nsIRunnable
                                , public nsIInterfaceRequestor
                                , public nsIChannelEventSink
 {
 public:
     NS_DECL_ISUPPORTS
@@ -128,27 +131,46 @@ private:
                                   const char *aFromSegment,
                                   PRUint32 aOffset,
                                   PRUint32 aCount,
                                   PRUint32 *aBytesConsumed);
 
     nsresult HandleManifestLine(const nsCString::const_iterator &aBegin,
                                 const nsCString::const_iterator &aEnd);
 
+    /**
+     * Saves "offline-manifest-hash" meta data from the old offline cache
+     * token to mOldManifestHashValue member to be compared on
+     * successfull load.
+     */
+    nsresult GetOldManifestContentHash(nsIRequest *aRequest);
+    /**
+     * This method setups the mNeedsUpdate to PR_FALSE when hash value
+     * of the just downloaded manifest file is the same as stored in cache's 
+     * "offline-manifest-hash" meta data. Otherwise stores the new value
+     * to this meta data.
+     */
+    nsresult CheckNewManifestContentHash(nsIRequest *aRequest);
+
     enum {
         PARSE_INIT,
         PARSE_CACHE_ENTRIES,
         PARSE_FALLBACK_ENTRIES,
         PARSE_NETWORK_ENTRIES,
         PARSE_ERROR
     } mParserState;
 
     nsCString mReadBuf;
     nsCOMArray<nsIURI> mExplicitURIs;
     PRBool mNeedsUpdate;
+
+    // manifest hash data
+    nsCOMPtr<nsICryptoHash> mManifestHash;
+    PRBool mManifestHashInitialized;
+    nsCString mOldManifestHashValue;
 };
 
 class nsOfflineCacheUpdate : public nsIOfflineCacheUpdate
 {
 public:
     NS_DECL_ISUPPORTS
     NS_DECL_NSIOFFLINECACHEUPDATE