properly delete old files during an offline cache update. b=389591, r+sr=biesi

netwerk/cache/src/nsDiskCacheDeviceSQL.cpp

@@ -112,8 +112,9 @@ class AutoResetStatement
 class EvictionObserver
 {
   public:
-    EvictionObserver(mozIStorageConnection *db)
+  EvictionObserver(mozIStorageConnection *db,
-      : mDB(db)
+                   nsOfflineCacheEvictionFunction *evictionFunction)
+    : mDB(db), mEvictionFunction(evictionFunction)
     {
       mDB->ExecuteSimpleSQL(
           NS_LITERAL_CSTRING("CREATE TEMP TRIGGER cache_on_delete AFTER DELETE"
@@ -121,16 +122,21 @@ class EvictionObserver
                              " cache_eviction_observer("
                              "  OLD.key, OLD.generation);"
                              " END;"));
+      mEvictionFunction->Reset();
     }
 
     ~EvictionObserver()
     {
       mDB->ExecuteSimpleSQL(
         NS_LITERAL_CSTRING("DROP TRIGGER cache_on_delete;"));
+      mEvictionFunction->Reset();
     }
 
+    void Apply() { return mEvictionFunction->Apply(); }
+
   private:
     mozIStorageConnection *mDB;
+    nsRefPtr<nsOfflineCacheEvictionFunction> mEvictionFunction;
 };
 
 #define DCACHE_HASH_MAX  LL_MAXINT
@@ -157,20 +163,6 @@ DCacheHash(const char * key)
  * nsOfflineCacheEvictionFunction
  */
 
-class nsOfflineCacheEvictionFunction : public mozIStorageFunction {
-public:
-  NS_DECL_ISUPPORTS
-  NS_DECL_MOZISTORAGEFUNCTION
-
-  nsOfflineCacheEvictionFunction(nsOfflineCacheDevice *device)
-    : mDevice(device)
-  {}
-
-private:
-  nsOfflineCacheDevice *mDevice;
-
-};
-
 NS_IMPL_ISUPPORTS1(nsOfflineCacheEvictionFunction, mozIStorageFunction)
 
 // helper function for directly exposing the same data file binding
@@ -201,7 +193,7 @@ GetCacheDataFile(nsIFile *cacheDir, const char *key,
 NS_IMETHODIMP
 nsOfflineCacheEvictionFunction::OnFunctionCall(mozIStorageValueArray *values, nsIVariant **_retval)
 {
-  LOG(("nsOfflineCacheDevice::EvictionObserver\n"));
+  LOG(("nsOfflineCacheEvictionFunction::OnFunctionCall\n"));
 
   *_retval = nsnull;
 
@@ -224,17 +216,29 @@ nsOfflineCacheEvictionFunction::OnFunctionCall(mozIStorageValueArray *values, ns
     return rv;
   }
 
-#if defined(PR_LOGGING)
+  mItems.AppendObject(file);
-  nsCAutoString path;
-  file->GetNativePath(path);
-  LOG(("  removing %s\n", path.get()));
-#endif
-
-  file->Remove(PR_FALSE);
 
   return NS_OK;
 }
 
+void
+nsOfflineCacheEvictionFunction::Apply()
+{
+  LOG(("nsOfflineCacheEvictionFunction::Apply\n"));
+
+  for (PRInt32 i = 0; i < mItems.Count(); i++) {
+#if defined(PR_LOGGING)
+    nsCAutoString path;
+    mItems[i]->GetNativePath(path);
+    LOG(("  removing %s\n", path.get()));
+#endif
+
+    mItems[i]->Remove(PR_FALSE);
+  }
+
+  Reset();
+}
+
 /******************************************************************************
  * nsOfflineCacheDeviceInfo
  */
@@ -761,11 +765,10 @@ nsOfflineCacheDevice::Init()
                          " ON moz_cache (ClientID, Key);"));
   // maybe the index already exists, so don't bother checking for errors.
 
-  nsCOMPtr<mozIStorageFunction> evictionFunction =
+  mEvictionFunction = new nsOfflineCacheEvictionFunction(this);
-    new nsOfflineCacheEvictionFunction(this);
+  if (!mEvictionFunction) return NS_ERROR_OUT_OF_MEMORY;
-  if (!evictionFunction) return NS_ERROR_OUT_OF_MEMORY;
 
-  rv = mDB->CreateFunction(NS_LITERAL_CSTRING("cache_eviction_observer"), 2, evictionFunction);
+  rv = mDB->CreateFunction(NS_LITERAL_CSTRING("cache_eviction_observer"), 2, mEvictionFunction);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // create all (most) of our statements up front
@@ -792,6 +795,7 @@ nsOfflineCacheDevice::Init()
     StatementSql ( mStatement_ListOwners,        "SELECT DISTINCT Domain, URI FROM moz_cache_owners WHERE ClientID = ?;"),
     StatementSql ( mStatement_ListOwnerDomains,  "SELECT DISTINCT Domain FROM moz_cache_owners WHERE ClientID = ?;"),
     StatementSql ( mStatement_ListOwnerURIs,     "SELECT DISTINCT URI FROM moz_cache_owners WHERE ClientID = ? AND Domain = ?;"),
+    StatementSql ( mStatement_DeleteConflicts,   "DELETE FROM moz_cache WHERE rowid IN (SELECT old.rowid FROM moz_cache AS old, moz_cache AS new WHERE old.Key = new.Key AND old.ClientID = ? AND new.ClientID = ?)"),
     StatementSql ( mStatement_DeleteUnowned,     "DELETE FROM moz_cache WHERE rowid IN (SELECT moz_cache.rowid FROM moz_cache LEFT OUTER JOIN moz_cache_owners ON (moz_cache.ClientID = moz_cache_owners.ClientID AND moz_cache.Key = moz_cache_owners.Key) WHERE moz_cache.ClientID = ? AND moz_cache_owners.Domain ISNULL);" ),
     StatementSql ( mStatement_SwapClientID,       "UPDATE OR REPLACE moz_cache SET ClientID = ? WHERE ClientID = ?;")
   };
@@ -810,13 +814,15 @@ nsOfflineCacheDevice::Init()
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Clear up dangling temporary sessions
-  EvictionObserver evictionObserver(mDB);
+  EvictionObserver evictionObserver(mDB, mEvictionFunction);
 
   rv = mDB->ExecuteSimpleSQL(
     NS_LITERAL_CSTRING("DELETE FROM moz_cache"
                        " WHERE (ClientID GLOB \"TempClient*\")"));
   NS_ENSURE_SUCCESS(rv, rv);
 
+  evictionObserver.Apply();
+
   return NS_OK;
 }
 
@@ -826,6 +832,8 @@ nsOfflineCacheDevice::Shutdown()
   NS_ENSURE_TRUE(mDB, NS_ERROR_NOT_INITIALIZED);
 
   mDB = 0;
+  mEvictionFunction = 0;
+
   return NS_OK;
 }
 
@@ -1239,7 +1247,7 @@ nsOfflineCacheDevice::EvictEntries(const char *clientID)
 
   // need trigger to fire user defined function after a row is deleted
   // so we can delete the corresponding data file.
-  EvictionObserver evictionObserver(mDB);
+  EvictionObserver evictionObserver(mDB, mEvictionFunction);
 
   const char *deleteCmd;
   if (clientID)
@@ -1260,6 +1268,8 @@ nsOfflineCacheDevice::EvictEntries(const char *clientID)
     PR_smprintf_free((char *) deleteCmd);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  evictionObserver.Apply();
+
   return NS_OK;
 }
 
@@ -1474,14 +1484,18 @@ nsresult
 nsOfflineCacheDevice::EvictUnownedEntries(const char *clientID)
 {
   LOG(("nsOfflineCacheDevice::EvictUnownedEntries [cid=%s]\n", clientID));
-  EvictionObserver evictionObserver(mDB);
+  EvictionObserver evictionObserver(mDB, mEvictionFunction);
 
   AutoResetStatement statement(mStatement_DeleteUnowned);
   nsresult rv = statement->BindUTF8StringParameter(
                                               0, nsDependentCString(clientID));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  return statement->Execute();
+  rv = statement->Execute();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  evictionObserver.Apply();
+  return NS_OK;
 }
 
 nsresult
@@ -1547,7 +1561,17 @@ nsOfflineCacheDevice::MergeTemporaryClientID(const char *clientID,
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  EvictionObserver evictionObserver(mDB);
+  EvictionObserver evictionObserver(mDB, mEvictionFunction);
+
+  AutoResetStatement deleteStatement(mStatement_DeleteConflicts);
+  rv = deleteStatement->BindUTF8StringParameter(
+                                          0, nsDependentCString(clientID));
+  rv |= deleteStatement->BindUTF8StringParameter(
+                                          1, nsDependentCString(fromClientID));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = deleteStatement->Execute();
+  NS_ENSURE_SUCCESS(rv, rv);
 
   AutoResetStatement swapStatement(mStatement_SwapClientID);
   rv = swapStatement->BindUTF8StringParameter(
@@ -1559,7 +1583,12 @@ nsOfflineCacheDevice::MergeTemporaryClientID(const char *clientID,
   rv = swapStatement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
-  return transaction.Commit();
+  rv = transaction.Commit();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  evictionObserver.Apply();
+
+  return NS_OK;
 }
 
 /**

netwerk/cache/src/nsDiskCacheDeviceSQL.h

@@ -42,9 +42,33 @@
 #include "nsILocalFile.h"
 #include "nsIObserver.h"
 #include "mozIStorageConnection.h"
+#include "mozIStorageFunction.h"
+#include "nsIFile.h"
+#include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
+#include "nsCOMArray.h"
 #include "nsVoidArray.h"
 
+class nsOfflineCacheDevice;
+
+class nsOfflineCacheEvictionFunction : public mozIStorageFunction {
+public:
+  NS_DECL_ISUPPORTS
+  NS_DECL_MOZISTORAGEFUNCTION
+
+  nsOfflineCacheEvictionFunction(nsOfflineCacheDevice *device)
+    : mDevice(device)
+  {}
+
+  void Reset() { mItems.Clear(); }
+  void Apply();
+
+private:
+  nsOfflineCacheDevice *mDevice;
+  nsCOMArray<nsIFile> mItems;
+
+};
+
 class nsOfflineCacheDevice : public nsCacheDevice
 {
 public:
@@ -151,7 +175,9 @@ private:
                           PRUint32 * count,
                           char *** values);
 
-  nsCOMPtr<mozIStorageConnection> mDB;
+  nsCOMPtr<mozIStorageConnection>          mDB;
+  nsRefPtr<nsOfflineCacheEvictionFunction> mEvictionFunction;
+
   nsCOMPtr<mozIStorageStatement>  mStatement_CacheSize;
   nsCOMPtr<mozIStorageStatement>  mStatement_EntryCount;
   nsCOMPtr<mozIStorageStatement>  mStatement_UpdateEntry;
@@ -165,6 +191,7 @@ private:
   nsCOMPtr<mozIStorageStatement>  mStatement_ClearDomain;
   nsCOMPtr<mozIStorageStatement>  mStatement_AddOwnership;
   nsCOMPtr<mozIStorageStatement>  mStatement_CheckOwnership;
+  nsCOMPtr<mozIStorageStatement>  mStatement_DeleteConflicts;
   nsCOMPtr<mozIStorageStatement>  mStatement_DeleteUnowned;
   nsCOMPtr<mozIStorageStatement>  mStatement_ListOwned;
   nsCOMPtr<mozIStorageStatement>  mStatement_ListOwners;