AppCaches which belong to hosted apps are not protected from deletion

content/browser/appcache/chrome_appcache_service_unittest.cc

Index: content/browser/appcache/chrome_appcache_service_unittest.cc
diff --git a/content/browser/appcache/chrome_appcache_service_unittest.cc b/content/browser/appcache/chrome_appcache_service_unittest.cc
index 7f99286c83e963e1c3ccddd812d54acd42855c65..1bab633ebecd9b89ceef6cd5c9ee246f65f8ff1f 100644
--- a/content/browser/appcache/chrome_appcache_service_unittest.cc
+++ b/content/browser/appcache/chrome_appcache_service_unittest.cc
@@ -5,25 +5,93 @@
 #include "base/file_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
-#include "base/scoped_temp_dir.h"
 #include "chrome/test/testing_browser_process.h"
 #include "chrome/test/testing_browser_process_test.h"
 #include "content/browser/appcache/chrome_appcache_service.h"
 #include "content/browser/browser_thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "webkit/appcache/appcache_database.h"
 #include "webkit/appcache/appcache_storage_impl.h"
+#include "webkit/quota/mock_special_storage_policy.h"
+
+#include <set>
 
 namespace {
 const FilePath::CharType kTestingAppCacheDirname[] =

[michaeln, 2011/07/07 19:06:51]

is this needed anymore?

[marja(google), 2011/07/13 11:13:16]

Done.

     FILE_PATH_LITERAL("Application Cache");
-}
+
+// Examples of a protected and an unprotected origin, to be used througout the
+// test.
+const GURL kProtectedOrigin("http://www.protected.com/");
+const GURL kNormalOrigin("http://www.normal.com/");
+
+} // unnamed namespace
 
 namespace appcache {
 
+// This subclass overwrites 2 functions (GetAllAppCacheInfo and
+// DeleteAppcachesForOrigin). It enables unit testing the internal logic of
+// ChromeAppCacheInfo (deleting only non-protected origins from the app cache).
+class TestableChromeAppCacheService : public ChromeAppCacheService
+{
+public:
+  explicit TestableChromeAppCacheService(quota::QuotaManagerProxy* proxy);
+
+  // Mock implementations
+  virtual void GetAllAppCacheInfo(AppCacheInfoCollection* collection,
+                                  net::CompletionCallback* callback);
+
+  virtual void DeleteAppCachesForOrigin(const GURL& origin,
+                                        net::CompletionCallback* callback);
+
+  // public so that the test program can read it
+  std::set<GURL> deleted_origins_;
+  bool callback_called_;
+private:
+  // Helper function for the mock implementation
+  static void DeferredCallCallback(net::CompletionCallback* callback, int rv);
+};
+
+TestableChromeAppCacheService::TestableChromeAppCacheService(
+    quota::QuotaManagerProxy* proxy)
+  : ChromeAppCacheService(proxy), callback_called_(false)
+{

[michaeln, 2011/07/07 19:06:51]

style nit: bracket goes on the previous line

+}
+
+void TestableChromeAppCacheService::GetAllAppCacheInfo(
+    AppCacheInfoCollection* collection,
+    net::CompletionCallback* callback)
+{

[michaeln, 2011/07/07 19:06:51]

style nit: bracket goes on the previous line

[marja(google), 2011/07/13 11:13:16]

Done.

+  AppCacheInfoVector info_vector;
+  collection->infos_by_origin[kProtectedOrigin];

[michaeln, 2011/07/07 19:06:51]

Did you mean to make an assignment here to info_vector, or maybe you can remove
the info_vector local variable since it doesn't seem to be needed?

[marja(google), 2011/07/13 11:13:16]

Done.

I rewrote this func; now there shouldn't be any unneeded variables.

+  collection->infos_by_origin[kNormalOrigin];
+  callback->Run(0);
+}
+
+void TestableChromeAppCacheService::DeleteAppCachesForOrigin(
+    const GURL& origin,
+    net::CompletionCallback* callback)
+{

[michaeln, 2011/07/07 19:06:51]

style nit: bracket goes on the previous line

[marja(google), 2011/07/13 11:13:16]

Done.

+  deleted_origins_.insert(origin);
+  // The callback must be called asynchronously, otherwise the code breaks.
+  // Defer to guarentee async completion.
+  MessageLoop::current()->PostTask(
+    FROM_HERE,

[michaeln, 2011/07/07 19:06:51]

style nit: indent the params by 2 more spaces

[marja(google), 2011/07/13 11:13:16]

Done.

+    NewRunnableFunction(&DeferredCallCallback, callback, 0));
+}
+
+void TestableChromeAppCacheService::DeferredCallCallback(
+    net::CompletionCallback* callback, int rv){
+  callback->Run(rv);
+}
+
 class ChromeAppCacheServiceTest : public TestingBrowserProcessTest {
  public:
   ChromeAppCacheServiceTest()
       : message_loop_(MessageLoop::TYPE_IO),
+        ALLOW_THIS_IN_INITIALIZER_LIST(appcache_cleared_callback_(
+            this, &ChromeAppCacheServiceTest::OnAppCacheCleared)),
+        callback_called_(false),
         db_thread_(BrowserThread::DB, &message_loop_),
         file_thread_(BrowserThread::FILE, &message_loop_),
         cache_thread_(BrowserThread::CACHE, &message_loop_),
@@ -32,77 +100,124 @@ class ChromeAppCacheServiceTest : public TestingBrowserProcessTest {
 
  protected:
   MessageLoop message_loop_;
-  ScopedTempDir temp_dir_;
+  net::CompletionCallbackImpl<ChromeAppCacheServiceTest>
+      appcache_cleared_callback_;
+  bool callback_called_;
 
  private:
+  void OnAppCacheCleared(int rv);
+
   BrowserThread db_thread_;
   BrowserThread file_thread_;
   BrowserThread cache_thread_;
   BrowserThread io_thread_;
 };
 
-TEST_F(ChromeAppCacheServiceTest, KeepOnDestruction) {
-  ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-  FilePath appcache_path = temp_dir_.path().Append(kTestingAppCacheDirname);
-  scoped_refptr<ChromeAppCacheService> appcache_service =
-      new ChromeAppCacheService(NULL);
+void ChromeAppCacheServiceTest::OnAppCacheCleared(int rv)
+{

[michaeln, 2011/07/07 19:06:51]

style nit: bracket goes on the previous line

[marja(google), 2011/07/13 11:13:16]

Done.

+  callback_called_ = true;
+}
+
+TEST_F(ChromeAppCacheServiceTest, ClearAppCache) {
+  // Tests ChromeAppCacheService::ClearAppCache(): nonprotected origins should
+  // be deleted an protected origins should not.
+  scoped_refptr<TestableChromeAppCacheService> appcache_service =
+      new TestableChromeAppCacheService(NULL);
   const content::ResourceContext* resource_context = NULL;
+  scoped_refptr<quota::MockSpecialStoragePolicy> mock_policy =
+      new quota::MockSpecialStoragePolicy;
+  mock_policy->AddProtected(kProtectedOrigin);
+  // Passing an empty FilePath is ok; nothing will be saved on the disk during
+  // the test.
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       NewRunnableMethod(appcache_service.get(),
                         &ChromeAppCacheService::InitializeOnIOThread,
-                        appcache_path,
+                        FilePath(""),
                         resource_context,
-                        scoped_refptr<quota::SpecialStoragePolicy>(NULL),
-                        false));
-  // Make the steps needed to initialize the storage of AppCache data.
-  message_loop_.RunAllPending();
-  appcache::AppCacheStorageImpl* storage =
-      static_cast<appcache::AppCacheStorageImpl*>(appcache_service->storage());
-  ASSERT_TRUE(storage->database_->db_connection());
-  ASSERT_EQ(1, storage->NewCacheId());
-  storage->disk_cache();
+                        mock_policy));
+  // Spin the message loop so that the initialization really happens.
   message_loop_.RunAllPending();
 
-  ASSERT_TRUE(file_util::PathExists(appcache_path));
-  ASSERT_TRUE(file_util::PathExists(appcache_path.AppendASCII("Index")));
+  appcache_service->ClearAppCache(&appcache_cleared_callback_);
 
-  appcache_service = NULL;
+  // Spin the message loop so that the asynchronous operations (mainly, calling
+  // the callback) happen.
   message_loop_.RunAllPending();
 
-  ASSERT_TRUE(file_util::PathExists(appcache_path));
+  // The appcache of the protected origin wasn't deleted, but the appcache of
+  // the normal origin was.
+  ASSERT_EQ(appcache_service->deleted_origins_.find(kProtectedOrigin),
+            appcache_service->deleted_origins_.end());
+  ASSERT_NE(appcache_service->deleted_origins_.find(kNormalOrigin),
+            appcache_service->deleted_origins_.end());
+  ASSERT_TRUE(callback_called_);
 }
 
-TEST_F(ChromeAppCacheServiceTest, RemoveOnDestruction) {
-  ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-  FilePath appcache_path = temp_dir_.path().Append(kTestingAppCacheDirname);
-  scoped_refptr<ChromeAppCacheService> appcache_service =
-      new ChromeAppCacheService(NULL);
+TEST_F(ChromeAppCacheServiceTest, ClearEmptyAppCache) {
+  // Tests ChromeAppCacheService::ClearAppCache(): the callback function should
+  // be called even if there are no appcaches to clear.
+  scoped_refptr<TestableChromeAppCacheService> appcache_service =
+      new TestableChromeAppCacheService(NULL);
   const content::ResourceContext* resource_context = NULL;
+  scoped_refptr<quota::MockSpecialStoragePolicy> mock_policy =
+      new quota::MockSpecialStoragePolicy;
+  // Passing an empty FilePath is ok; nothing will be saved on the disk during
+  // the test.
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       NewRunnableMethod(appcache_service.get(),
                         &ChromeAppCacheService::InitializeOnIOThread,
-                        appcache_path,
+                        FilePath(""),
                         resource_context,
-                        scoped_refptr<quota::SpecialStoragePolicy>(NULL),
-                        true));
-  // Make the steps needed to initialize the storage of AppCache data.
+                        mock_policy));
+  // Spin the message loop so that the initialization really happens.
   message_loop_.RunAllPending();
-  appcache::AppCacheStorageImpl* storage =
-      static_cast<appcache::AppCacheStorageImpl*>(appcache_service->storage());
-  ASSERT_TRUE(storage->database_->db_connection());
-  ASSERT_EQ(1, storage->NewCacheId());
-  storage->disk_cache();
+
+  appcache_service->ClearAppCache(&appcache_cleared_callback_);
+
+  // Spin the message loop so that the asynchronous operations (mainly, calling
+  // the callback) happen.
+  message_loop_.RunAllPending();
+
+  ASSERT_TRUE(callback_called_);
+}
+
+TEST_F(ChromeAppCacheServiceTest, ClearAppCacheWithoutCallback) {
+  // Tests ChromeAppCacheService::ClearAppCache(): passing NULL as callback
+  // should be ok and the functionality should be the same as in the case with
+  // the callback.
+  scoped_refptr<TestableChromeAppCacheService> appcache_service =
+      new TestableChromeAppCacheService(NULL);
+  const content::ResourceContext* resource_context = NULL;
+  scoped_refptr<quota::MockSpecialStoragePolicy> mock_policy =
+      new quota::MockSpecialStoragePolicy;
+  mock_policy->AddProtected(kProtectedOrigin);
+  // Passing an empty FilePath is ok; nothing will be saved on the disk during
+  // the test.
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      NewRunnableMethod(appcache_service.get(),
+                        &ChromeAppCacheService::InitializeOnIOThread,
+                        FilePath(""),

[michaeln, 2011/07/07 19:06:51]

i think the default ctor would work here (and elsewhere)... FilePath()

[marja(google), 2011/07/13 11:13:16]

Done.

+                        resource_context,
+                        mock_policy));
+  // Spin the message loop so that the initialization really happens.
   message_loop_.RunAllPending();
 
-  ASSERT_TRUE(file_util::PathExists(appcache_path));
-  ASSERT_TRUE(file_util::PathExists(appcache_path.AppendASCII("Index")));
+  appcache_service->ClearAppCache(NULL);
 
-  appcache_service = NULL;
+  // Spin the message loop so that the asynchronous operations have a chance to
+  // happen.
   message_loop_.RunAllPending();
 
-  ASSERT_FALSE(file_util::PathExists(appcache_path));
+  // The appcache of the protected origin wasn't deleted, but the appcache of
+  // the normal origin was.
+  ASSERT_EQ(appcache_service->deleted_origins_.find(kProtectedOrigin),
+            appcache_service->deleted_origins_.end());
+  ASSERT_NE(appcache_service->deleted_origins_.find(kNormalOrigin),
+            appcache_service->deleted_origins_.end());
+  ASSERT_FALSE(callback_called_);
 }
 
 }  // namespace appcache