461325 - Cache implicit entries even when manifest is not changed

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Honza Bambas (:mayhemer)]

When we navigate to a page with manifest reference that is not yet contained in the cache identified by the manifest it must be cached in it. This doesn't happen when the manifest is not modified (we end up with noupdate state).

Comment 1

[Honza Bambas (:mayhemer)]

Attachment: v1

I am using new array side by the mDocuments array. I cannot extract URI of the documents directly because I need nsIDocument interface and that could not be included (dep on content is not permitted).

This fix makes actually bug 462562 worse because we try to cache (request again) non existing pages with this patch.

Comment 2

[Honza Bambas (:mayhemer)]

Attachment: v2

Added req for layout, content, widget. This allows usage of nsIDocument interface to extract URI directly from documents.

Comment 3

[Honza Bambas (:mayhemer)]

Attachment: v3

Fixing bug 462562 as well. Now we correctly recognize if a resource was load from an offline cache.

Comment 4

[Dave Camp (:dcamp)]

Comment on attachment 346252 [details] [diff] [review]
v3

>* * *
>* * *
>
>diff --git a/content/base/src/nsContentSink.cpp b/content/base/src/nsContentSink.cpp
>--- a/content/base/src/nsContentSink.cpp
>+++ b/content/base/src/nsContentSink.cpp
>@@ -1015,24 +1015,33 @@ nsContentSink::ProcessOfflineManifest(ns
>   // Check for a manifest= attribute.
>   nsAutoString manifestSpec;
>   aElement->GetAttr(kNameSpaceID_None, nsGkAtoms::manifest, manifestSpec);
> 
>   // Grab the application cache the document was loaded from, if any.
>   nsCOMPtr<nsIApplicationCache> applicationCache;
> 
>   nsCOMPtr<nsIApplicationCacheChannel> applicationCacheChannel =
>     do_QueryInterface(mDocument->GetChannel());
>   if (applicationCacheChannel) {
>-    rv = applicationCacheChannel->GetApplicationCache(
>-      getter_AddRefs(applicationCache));
>+    PRBool loadFromApplicationCache;
>+    rv = applicationCacheChannel->GetLoadFromApplicationCache(
>+      &loadFromApplicationCache);
>     if (NS_FAILED(rv)) {
>       return;
>+    }
>+
>+    if (loadFromApplicationCache) {
>+      rv = applicationCacheChannel->GetApplicationCache(
>+        getter_AddRefs(applicationCache));
>+      if (NS_FAILED(rv)) {
>+        return;
>+      }
>     }
>   }
> 
>   if (manifestSpec.IsEmpty() && !applicationCache) {
>     // Not loaded from an application cache, and no manifest
>     // attribute.  Nothing to do here.
>     return;
>   }
> 
>   CacheSelectionAction action = CACHE_SELECTION_NONE;
>diff --git a/dom/tests/mochitest/ajax/offline/foreign2.html b/dom/tests/mochitest/ajax/offline/foreign2.html
>--- a/dom/tests/mochitest/ajax/offline/foreign2.html
>+++ b/dom/tests/mochitest/ajax/offline/foreign2.html
>@@ -37,21 +37,21 @@ function onLoaded()
>   var foreign1cache = appCacheService.getActiveCache(
>     "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/foreign1.cacheManifest");
>   OfflineTest.ok(foreign1cache, "Foreign 1 cache loaded");
>   
>   var foreign2cache = appCacheService.getActiveCache(
>     "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/foreign2.cacheManifest");
>   OfflineTest.ok(!foreign2cache, "Foreign 2 cache not present");
> 
>   foreign1cache = appCacheService.chooseApplicationCache(
>     "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/foreign2.html");
>-  OfflineTest.ok(!foreign1cache, "foreign2.html not choosed from foreign1 cache");
>+  OfflineTest.ok(!foreign1cache, "foreign2.html not chosen from foreign1 cache");
>   
>   try
>   {
>     OfflineTest.ok(applicationCache.status == Components.interfaces.nsIDOMOfflineResourceList.UNCACHED,
>         "there is no associated application cache");
>   }
>   catch (ex)
>   {
>     OfflineTest.ok(false, "applicationCache.status must not throw an exception");  
>   }
>diff --git a/dom/tests/mochitest/ajax/offline/offlineTests.js b/dom/tests/mochitest/ajax/offline/offlineTests.js
>--- a/dom/tests/mochitest/ajax/offline/offlineTests.js
>+++ b/dom/tests/mochitest/ajax/offline/offlineTests.js
>@@ -244,45 +244,48 @@ getActiveSession: function()
> 
> priv: function(func)
> {
>   var self = this;
>   return function() {
>     netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
>     func(arguments);
>   }
> },
> 
>-checkCustomCache: function(group, url, expectEntry)
>+checkCustomCache: function(group, url, expectEntry, mustBeValid)
> {
>   var serv = Cc["@mozilla.org/network/application-cache-service;1"]
>              .getService(Ci.nsIApplicationCacheService);
>   var cache = serv.getActiveCache(group);
>   var cacheSession = null;
>   if (cache) {
>     var cacheService = Cc["@mozilla.org/network/cache-service;1"]
>                        .getService(Ci.nsICacheService);
>     cacheSession = cacheService.createSession(cache.clientID,
>                                       Ci.nsICache.STORE_OFFLINE,
>                                       true);
>   }
>   
>-  this._checkCache(cacheSession, url, expectEntry);
>+  this._checkCache(cacheSession, url, expectEntry, mustBeValid);
> },
> 
>-checkCache: function(url, expectEntry)
>+checkCache: function(url, expectEntry, mustBeValid)
> {
>   var cacheSession = this.getActiveSession();
>-  this._checkCache(cacheSession, url, expectEntry);
>+  this._checkCache(cacheSession, url, expectEntry, mustBeValid);
> },
>   
>-_checkCache: function(cacheSession, url, expectEntry)
>+_checkCache: function(cacheSession, url, expectEntry, mustBeValid)
> {
>+  if (mustBeValid === 'undefined')
>+    mustBeValid = false;
>+
>   if (!cacheSession) {
>     if (expectEntry) {
>       this.ok(false, url + " should exist in the offline cache");
>     } else {
>       this.ok(true, url + " should not exist in the offline cache");
>     }
>     return;
>   }
> 
>   try {
>@@ -296,23 +299,23 @@ _checkCache: function(cacheSession, url,
>   } catch (e) {
>     if (e.result == NS_ERROR_CACHE_KEY_NOT_FOUND) {
>       if (expectEntry) {
>         this.ok(false, url + " should exist in the offline cache");
>       } else {
>         this.ok(true, url + " should not exist in the offline cache");
>       }
>     } else if (e.result == NS_ERROR_CACHE_KEY_WAIT_FOR_VALIDATION) {
>       // There was a cache key that we couldn't access yet, that's good enough.
>       if (expectEntry) {
>-        this.ok(true, url + " should exist in the offline cache");
>+        this.ok(!mustBeValid, url + " should exist in the offline cache");
>       } else {
>-        this.ok(false, url + " should not exist in the offline cache");
>+        this.ok(mustBeValid, url + " should not exist in the offline cache");
>       }
>     } else {
>       throw e;
>     }
>   }
> },
> 
> putData: function(serverPath, contentType, data)
> {
>   if (!data.length)  
>diff --git a/dom/tests/mochitest/ajax/offline/test_offlineMode.html b/dom/tests/mochitest/ajax/offline/test_offlineMode.html
>--- a/dom/tests/mochitest/ajax/offline/test_offlineMode.html
>+++ b/dom/tests/mochitest/ajax/offline/test_offlineMode.html
>@@ -69,22 +69,21 @@ function finalize()
> {
>   window.clearTimeout(gCompleteTimeout);
> 
>   var ioserv = Cc["@mozilla.org/network/io-service;1"]
>       .getService(Ci.nsIIOService);
>       
>   if (!ioserv.offline)
>   {
>     OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/notonwhitelist.html", true);
>     OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/updatingIFrame.html", true);
>-    OfflineTest.todo(false, "Bug 461325 - implicit entry should be in cache");
>-    //OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/updatingImplicit.html", true);
>+    OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/updatingImplicit.html", true);
> 
>     OfflineTest.is(gGotExplicitVersion, 1, "Explicit entry loaded");
>     OfflineTest.is(gGotImplicitVersion, 1, "Implicit entry loaded");
>     OfflineTest.is(gGotDynamicVersion, 1, "Dynamic entry loaded");
> 
>     gGotExplicitVersion = 0;
>     gGotImplicitVersion = 0;
>     gGotDynamicVersion = 0;
>     
> 
>@@ -112,21 +111,21 @@ function finalize()
>     aFrame.location = "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/notonwhitelist.html";
>     // Starts the chain all over again but in offline mode.
>   }
>   else
>   {
>     gImplicitWindow.close();
>     
>     ioserv.offline = false;
>     
>     OfflineTest.is(gGotExplicitVersion, 1, "Explicit entry loaded");
>-    OfflineTest.todo(gGotImplicitVersion == 1, "Bug 461325 - Implicit entry loaded");
>+    OfflineTest.is(gGotImplicitVersion, 1, "Implicit entry loaded");
>     OfflineTest.is(gGotDynamicVersion, 1, "Dynamic entry loaded");
>     OfflineTest.ok(gGotOnError, "Got onerror event invoked by implicit page load in offline mode");
> 
>     OfflineTest.teardown();
>     OfflineTest.finish();
>   }
> }
> 
> SimpleTest.waitForExplicitFinish();
> 
>diff --git a/dom/tests/mochitest/ajax/offline/test_updatingManifest.html b/dom/tests/mochitest/ajax/offline/test_updatingManifest.html
>--- a/dom/tests/mochitest/ajax/offline/test_updatingManifest.html
>+++ b/dom/tests/mochitest/ajax/offline/test_updatingManifest.html
>@@ -191,22 +191,21 @@ function implicitCached()
>   
>   // Fallback entries
>   OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/fallback.html", true);
>   OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/fallback2.html", false);
>   
>   // Whitelist entries
>   OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/onwhitelist.html", false);
>   checkFallbackAndWhitelisting("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/onwhitelist.html", "", true);
>   
>   // Implicit entries
>-  OfflineTest.todo(false, "Bug 461325");
>-  //OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/updatingImplicit.html", true);
>+  OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/updatingImplicit.html", true);
>   
>   // Dynamic entries
>   OfflineTest.checkCache("http://localhost:8888/tests/SimpleTest/EventUtils.js", true);
>   
>   // Fallback URI selection check
>   checkFallbackAndWhitelisting("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/namespace1/opp.html",
>       "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/fallback.html", false);
>   checkFallbackAndWhitelisting("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/namespace1/sub/opp.html",
>       "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/fallback.html", false);
>   
>@@ -241,22 +240,21 @@ function manifestUpdated()
>   
>     // Fallback entries
>     OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/fallback.html", true);
>     OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/fallback2.html", true);
>   
>     // Whitelist entries
>     OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/onwhitelist.html", false);
>     checkFallbackAndWhitelisting("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/onwhitelist.html", "", false);
> 
>     // Implicit entries
>-    OfflineTest.todo(false, "Bug 461325");
>-    //OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/updatingImplicit.html", true);
>+    OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/updatingImplicit.html", true);
> 
>     // Dynamic entries
>     OfflineTest.checkCache("http://localhost:8888/tests/SimpleTest/EventUtils.js", true);
>     
>     // Fallback URI selection check
>     checkFallbackAndWhitelisting("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/namespace1/opp.html",
>         "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/fallback.html", false);
>     checkFallbackAndWhitelisting("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/namespace1/sub/opp.html",
>         "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/fallback2.html", false);
>     
>@@ -287,22 +285,21 @@ function manifestUpdated()
>   
>     // Fallback entries
>     OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/fallback.html", false);
>     OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/fallback2.html", true);
>   
>     // Whitelist entries
>     OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/onwhitelist.html", false);
>     checkFallbackAndWhitelisting("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/onwhitelist.html", "", true);
>   
>     // Implicit entries
>-    OfflineTest.todo(false, "Bug 461325");
>-    //OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/updatingImplicit.html", true);
>+    OfflineTest.checkCache("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/updatingImplicit.html", true);
> 
>     // Dynamic entries
>     OfflineTest.checkCache("http://localhost:8888/tests/SimpleTest/EventUtils.js", true);
>     
>     // Fallback URI selection check
>     checkFallbackAndWhitelisting("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/namespace1/opp.html", 
>         "", false);
>     checkFallbackAndWhitelisting("http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/namespace1/sub/opp.html",
>         "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/fallback2.html", false);
>     
>diff --git a/netwerk/base/public/nsIApplicationCacheChannel.idl b/netwerk/base/public/nsIApplicationCacheChannel.idl
>--- a/netwerk/base/public/nsIApplicationCacheChannel.idl
>+++ b/netwerk/base/public/nsIApplicationCacheChannel.idl
>@@ -37,30 +37,37 @@
>  *
>  * ***** END LICENSE BLOCK ***** */
> 
> #include "nsISupports.idl"
> 
> interface nsIApplicationCache;
> 
> /**
>  * Interface implemented by channels that support application caches.
>  */
>-[scriptable, uuid(a27beec8-b40c-4919-ad68-f1576264713c)]
>+[scriptable, uuid(9acfd21c-9c07-459f-8dae-ed2ffba23ddc)]
> interface nsIApplicationCacheChannel : nsISupports
> {
>     /**
>      * The application cache associated with this channel.
>      *
>      * NS_ERROR_ALREADY_OPENED will be thrown if set after AsyncOpen()
>      * is called.
>      */
>     attribute nsIApplicationCache applicationCache;
>+
>+    /**
>+     * TRUE when the resource went from the application cache. This might
>+     * be false even there is assigned an application cache e.g. in case of fallback
>+     * of load of an entry matching bypass namespace.
>+     */
>+    readonly attribute PRBool loadFromApplicationCache;
> 
>     /**
>      * When true, the channel will ask its notification callbacks for
>      * an application cache if one is not explicitly provided.  Default
>      * value is true.
>      *
>      * NS_ERROR_ALREADY_OPENED will be thrown if set after AsyncOpen()
>      * is called.
>      */
>     attribute boolean inheritApplicationCache;
>diff --git a/netwerk/protocol/http/src/nsHttpChannel.cpp b/netwerk/protocol/http/src/nsHttpChannel.cpp
>--- a/netwerk/protocol/http/src/nsHttpChannel.cpp
>+++ b/netwerk/protocol/http/src/nsHttpChannel.cpp
>@@ -125,20 +125,21 @@ nsHttpChannel::nsHttpChannel()
>     , mUploadStreamHasHeaders(PR_FALSE)
>     , mAuthRetryPending(PR_FALSE)
>     , mSuppressDefensiveAuth(PR_FALSE)
>     , mResuming(PR_FALSE)
>     , mInitedCacheEntry(PR_FALSE)
>     , mCacheForOfflineUse(PR_FALSE)
>     , mCachingOpportunistically(PR_FALSE)
>     , mFallbackChannel(PR_FALSE)
>     , mInheritApplicationCache(PR_TRUE)
>     , mChooseApplicationCache(PR_FALSE)
>+    , mLoadFromApplicationCache(PR_FALSE)
>     , mTracingEnabled(PR_TRUE)
> {
>     LOG(("Creating nsHttpChannel @%x\n", this));
> 
>     // grab a reference to the handler to ensure that it doesn't go away.
>     nsHttpHandler *handler = gHttpHandler;
>     NS_ADDREF(handler);
> }
> 
> nsHttpChannel::~nsHttpChannel()
>@@ -1478,20 +1479,21 @@ nsHttpChannel::ProcessFallback(PRBool *f
> 
>     return NS_OK;
> }
> 
> nsresult
> nsHttpChannel::OpenCacheEntry(PRBool offline, PRBool *delayed)
> {
>     nsresult rv;
> 
>     *delayed = PR_FALSE;
>+    mLoadFromApplicationCache = PR_FALSE;
> 
>     LOG(("nsHttpChannel::OpenCacheEntry [this=%x]", this));
> 
>     // make sure we're not abusing this function
>     NS_PRECONDITION(!mCacheEntry, "cache entry already open");
> 
>     nsCAutoString cacheKey;
> 
>     if (mRequestHead.Method() == nsHttp::Post) {
>         // If the post id is already set then this is an attempt to replay
>@@ -1639,20 +1641,25 @@ nsHttpChannel::OpenCacheEntry(PRBool off
>                 mLoadFlags & LOAD_DOCUMENT_URI) {
>                 // Document loads for items in an opportunistic namespace
>                 // should be placed in the offline cache.
>                 nsCString clientID;
>                 mApplicationCache->GetClientID(clientID);
> 
>                 mCacheForOfflineUse = !clientID.IsEmpty();
>                 SetOfflineCacheClientID(clientID);
>                 mCachingOpportunistically = PR_TRUE;
>             }
>+        }
>+        else if (NS_SUCCEEDED(rv)) {
>+            // We successfully opened an offline cache session and the entry,
>+            // now indiciate we load from the offline cache.
>+            mLoadFromApplicationCache = PR_TRUE;
>         }
>     }
> 
>     if (!mCacheEntry && !waitingForValidation) {
>         rv = gHttpHandler->GetCacheSession(storagePolicy,
>                                            getter_AddRefs(session));
>         if (NS_FAILED(rv)) return rv;
> 
>         rv = session->OpenCacheEntry(cacheKey, accessRequested, PR_FALSE,
>                                      getter_AddRefs(mCacheEntry));
>@@ -5226,20 +5233,29 @@ NS_IMETHODIMP
> NS_IMETHODIMP
> nsHttpChannel::SetApplicationCache(nsIApplicationCache *appCache)
> {
>     NS_ENSURE_TRUE(!mWasOpened, NS_ERROR_ALREADY_OPENED);
> 
>     mApplicationCache = appCache;
>     return NS_OK;
> }
> 
> NS_IMETHODIMP
>+nsHttpChannel::GetLoadFromApplicationCache(PRBool *aLoadFromApplicationCache)
>+{
>+    NS_ENSURE_ARG(*aLoadFromApplicationCache);
>+
>+    *aLoadFromApplicationCache = mLoadFromApplicationCache;
>+    return NS_OK;
>+}
>+
>+NS_IMETHODIMP
> nsHttpChannel::GetInheritApplicationCache(PRBool *aInherit)
> {
>     *aInherit = mInheritApplicationCache;
>     return NS_OK;
> }
> 
> NS_IMETHODIMP
> nsHttpChannel::SetInheritApplicationCache(PRBool aInherit)
> {
>     NS_ENSURE_TRUE(!mWasOpened, NS_ERROR_ALREADY_OPENED);
>diff --git a/netwerk/protocol/http/src/nsHttpChannel.h b/netwerk/protocol/http/src/nsHttpChannel.h
>--- a/netwerk/protocol/http/src/nsHttpChannel.h
>+++ b/netwerk/protocol/http/src/nsHttpChannel.h
>@@ -326,20 +326,21 @@ private:
>     PRUint32                          mInitedCacheEntry         : 1;
>     PRUint32                          mCacheForOfflineUse       : 1;
>     // True if mCacheForOfflineUse was set because we were caching
>     // opportunistically.
>     PRUint32                          mCachingOpportunistically : 1;
>     // True if we are loading a fallback cache entry from the
>     // application cache.
>     PRUint32                          mFallbackChannel          : 1;
>     PRUint32                          mInheritApplicationCache  : 1;
>     PRUint32                          mChooseApplicationCache   : 1;
>+    PRUint32                          mLoadFromApplicationCache : 1;
>     PRUint32                          mTracingEnabled           : 1;
> 
>     class nsContentEncodings : public nsIUTF8StringEnumerator
>     {
>     public:
>         NS_DECL_ISUPPORTS
>         NS_DECL_NSIUTF8STRINGENUMERATOR
> 
>         nsContentEncodings(nsIHttpChannel* aChannel, const char* aEncodingHeader);
>         virtual ~nsContentEncodings();
>diff --git a/uriloader/prefetch/Makefile.in b/uriloader/prefetch/Makefile.in
>--- a/uriloader/prefetch/Makefile.in
>+++ b/uriloader/prefetch/Makefile.in
>@@ -40,20 +40,23 @@ srcdir		= @srcdir@
> srcdir		= @srcdir@
> VPATH		= @srcdir@
> 
> include $(DEPTH)/config/autoconf.mk
> 
> MODULE		= prefetch
> LIBRARY_NAME	= prefetch_s
> LIBXUL_LIBRARY	= 1
> REQUIRES	= xpcom \
> 		  dom \
>+		  content \
>+		  widget \
>+		  layout \
> 		  string \
> 		  necko \
> 		  uriloader \
> 		  nkcache \
> 		  chardet \
> 		  pref \
> 		  caps \
> 		  $(NULL)
> 
> CPPSRCS = \
>diff --git a/uriloader/prefetch/nsOfflineCacheUpdate.cpp b/uriloader/prefetch/nsOfflineCacheUpdate.cpp
>--- a/uriloader/prefetch/nsOfflineCacheUpdate.cpp
>+++ b/uriloader/prefetch/nsOfflineCacheUpdate.cpp
>@@ -40,23 +40,26 @@
> 
> #include "nsCPrefetchService.h"
> #include "nsCURILoader.h"
> #include "nsIApplicationCacheContainer.h"
> #include "nsIApplicationCacheChannel.h"
> #include "nsIApplicationCacheService.h"
> #include "nsICache.h"
> #include "nsICacheService.h"
> #include "nsICacheSession.h"
> #include "nsICachingChannel.h"
>+#include "nsIContent.h"
> #include "nsIDocumentLoader.h"
>+#include "nsIDOMElement.h"
> #include "nsIDOMWindow.h"
> #include "nsIDOMOfflineResourceList.h"
>+#include "nsIDocument.h"
> #include "nsIObserverService.h"
> #include "nsIURL.h"
> #include "nsIWebProgress.h"
> #include "nsICryptoHash.h"
> #include "nsICacheEntryDescriptor.h"
> #include "nsIPermissionManager.h"
> #include "nsIPrincipal.h"
> #include "nsIPrefBranch.h"
> #include "nsIPrefService.h"
> #include "nsNetCID.h"
>@@ -1122,20 +1125,21 @@ nsOfflineCacheUpdate::LoadCompleted()
>             mSucceeded = PR_FALSE;
>             NotifyError();
>             Finish();
>             return;
>         }
> 
>         if (!doUpdate) {
>             mSucceeded = PR_FALSE;
>             NotifyNoUpdate();
>             Finish();
>+            ScheduleImplicit();
>             return;
>         }
> 
>         rv = mApplicationCache->MarkEntry(mManifestItem->mCacheKey,
>                                           mManifestItem->mItemType);
>         if (NS_FAILED(rv)) {
>             mSucceeded = PR_FALSE;
>             NotifyError();
>             Finish();
>             return;
>@@ -1427,20 +1431,110 @@ nsOfflineCacheUpdate::NotifyCompleted(ns
>     LOG(("nsOfflineCacheUpdate::NotifyCompleted [%p, %p]", this, aItem));
> 
>     nsCOMArray<nsIOfflineCacheUpdateObserver> observers;
>     nsresult rv = GatherObservers(observers);
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     for (PRInt32 i = 0; i < observers.Count(); i++) {
>         observers[i]->ItemCompleted(this, aItem);
>     }
> 
>+    return NS_OK;
>+}
>+
>+void
>+nsOfflineCacheUpdate::AddDocument(nsIDOMDocument *aDocument)
>+{
>+    // Add document only if it were not loaded from an offline cache.
>+    // If it were loaded from an offline cache then it has already been
>+    // associated with it and must not be again cached as implicit (reasons
>+    // we collect documents here).
>+    nsCOMPtr<nsIDocument> document = do_QueryInterface(aDocument);
>+    if (!document)
>+        return;
>+
>+    nsIChannel* channel = document->GetChannel();
>+    nsCOMPtr<nsIApplicationCacheChannel> appCacheChannel =
>+        do_QueryInterface(channel);
>+    if (!appCacheChannel)
>+        return;
>+
>+    PRBool loadFromAppCache;
>+    appCacheChannel->GetLoadFromApplicationCache(&loadFromAppCache);
>+    if (loadFromAppCache)
>+        return;
>+
>+    mDocuments.AppendObject(aDocument);
>+};
>+
>+nsresult
>+nsOfflineCacheUpdate::ScheduleImplicit()
>+{
>+    if (mDocuments.Count() == 0)
>+        return NS_OK;
>+
>+    nsresult rv;
>+
>+    nsRefPtr<nsOfflineCacheUpdate> update = new nsOfflineCacheUpdate();
>+    NS_ENSURE_TRUE(update, NS_ERROR_OUT_OF_MEMORY);
>+
>+    nsCAutoString clientID;
>+    if (mPreviousApplicationCache) {
>+        rv = mPreviousApplicationCache->GetClientID(clientID);
>+        NS_ENSURE_SUCCESS(rv, rv);
>+    }
>+    else {
>+        clientID = mClientID;
>+    }
>+
>+    rv = update->InitPartial(mManifestURI, clientID, mDocumentURI);
>+    NS_ENSURE_SUCCESS(rv, rv);
>+
>+    PRBool added = PR_FALSE;
>+    for (PRInt32 i = 0; i < mDocuments.Count(); i++) {
>+        nsIDOMDocument* domDoc = mDocuments[i];
>+        nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc);
>+        if (!doc)
>+            continue;
>+
>+        nsIURI* uri = doc->GetDocumentURI();
>+        if (!uri)
>+            continue;
>+
>+        nsIContent* content = doc->GetRootContent();
>+        nsCOMPtr<nsIDOMElement> root = do_QueryInterface(content);
>+        if (!root)
>+            continue;
>+
>+        nsAutoString manifestSpec;
>+        rv = root->GetAttribute(NS_LITERAL_STRING("manifest"), manifestSpec);
>+        NS_ENSURE_SUCCESS(rv, rv);
>+
>+        nsCOMPtr<nsIURI> manifestURI;
>+        NS_NewURI(getter_AddRefs(manifestURI), manifestSpec,
>+                  doc->GetDocumentCharacterSet().get(),
>+                  doc->GetDocumentURI());
>+        if (!manifestURI)
>+            continue;
>+
>+        rv = update->AddURI(uri, nsIApplicationCache::ITEM_IMPLICIT);
>+        NS_ENSURE_SUCCESS(rv, rv);
>+
>+        added = PR_TRUE;
>+    }
>+
>+    if (!added)
>+      return NS_OK;
>+
>+    rv = update->Schedule();
>+    NS_ENSURE_SUCCESS(rv, rv);
>+
>     return NS_OK;
> }
> 
> nsresult
> nsOfflineCacheUpdate::AssociateDocument(nsIDOMDocument *aDocument)
> {
>     // Check that the document that requested this update was
>     // previously associated with an application cache.  If not, it
>     // should be associated with the new one.
>     nsCOMPtr<nsIApplicationCacheContainer> container =
>diff --git a/uriloader/prefetch/nsOfflineCacheUpdate.h b/uriloader/prefetch/nsOfflineCacheUpdate.h
>--- a/uriloader/prefetch/nsOfflineCacheUpdate.h
>+++ b/uriloader/prefetch/nsOfflineCacheUpdate.h
>@@ -209,24 +209,21 @@ public:
>     ~nsOfflineCacheUpdate();
> 
>     static nsresult GetCacheKey(nsIURI *aURI, nsACString &aKey);
> 
>     nsresult Init();
> 
>     nsresult Begin();
>     nsresult Cancel();
> 
>     void LoadCompleted();
>-
>-    void AddDocument(nsIDOMDocument *aDocument) {
>-        mDocuments.AppendObject(aDocument);
>-    };
>+    void AddDocument(nsIDOMDocument *aDocument);
> 
> private:
>     nsresult HandleManifest(PRBool *aDoUpdate);
>     nsresult AddURI(nsIURI *aURI, PRUint32 aItemType);
> 
>     nsresult ProcessNextURI();
> 
>     // Adds items from the previous cache witha type matching aType.
>     // If namespaceFilter is non-null, only items matching the
>     // specified namespaces will be added.
>@@ -234,20 +231,21 @@ private:
>                               nsTArray<nsCString>* namespaceFilter = nsnull);
> 
>     nsresult GatherObservers(nsCOMArray<nsIOfflineCacheUpdateObserver> &aObservers);
>     nsresult NotifyError();
>     nsresult NotifyChecking();
>     nsresult NotifyNoUpdate();
>     nsresult NotifyDownloading();
>     nsresult NotifyStarted(nsOfflineCacheUpdateItem *aItem);
>     nsresult NotifyCompleted(nsOfflineCacheUpdateItem *aItem);
>     nsresult AssociateDocument(nsIDOMDocument *aDocument);
>+    nsresult ScheduleImplicit();
>     nsresult Finish();
> 
>     enum {
>         STATE_UNINITIALIZED,
>         STATE_INITIALIZED,
>         STATE_CHECKING,
>         STATE_DOWNLOADING,
>         STATE_CANCELLED,
>         STATE_FINISHED
>     } mState;

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 346252 [details] [diff] [review]
v3

+++ b/netwerk/base/public/nsIApplicationCacheChannel.idl
+     * TRUE when the resource went from the application cache. This might

s/went/came/ ?

also this should be loadedFrom*, not loadFrom*

+    NS_ENSURE_ARG(*aLoadFromApplicationCache);

that doesn't make sense. you probably want NS_ENSURE_ARG(aLoadFromApplicationCache), but I don't think that's a useful check either.

+    // Add document only if it were not loaded from an offline cache.
+    // If it were loaded from an offline cache then it has already been

s/were/was/

+    mDocuments.AppendObject(aDocument);
+};


shouldn't have a semicolon here

Comment 6

[Honza Bambas (:mayhemer)]

Attachment: v3.1

Fixed sr comments.

Comment 7

[Dave Camp (:dcamp)]

Attachment: v3.2

New patch just fixes some whitespace issues.

Comment 8

[Dave Camp (:dcamp)]

... and was landed as http://hg.mozilla.org/mozilla-central/rev/3502bada8a5c

Comment 9

[Dave Camp (:dcamp)]

Sorry, was landed as http://hg.mozilla.org/mozilla-central/rev/038df99ac499