Refactored app cache clear on exit code to happen in the object owning the files.

chrome/browser/appcache/chrome_appcache_service.cc

Index: chrome/browser/appcache/chrome_appcache_service.cc
diff --git a/chrome/browser/appcache/chrome_appcache_service.cc b/chrome/browser/appcache/chrome_appcache_service.cc
index c01b1d7d36cff67b7171ac9f09885cc7d73dd386..5313d06b6c9e9c650a17b6d08b081a2098e44d63 100644
--- a/chrome/browser/appcache/chrome_appcache_service.cc
+++ b/chrome/browser/appcache/chrome_appcache_service.cc
@@ -22,7 +22,8 @@ ChromeAppCacheService::ChromeAppCacheService() {
 
 void ChromeAppCacheService::InitializeOnIOThread(
     const FilePath& profile_path, bool is_incognito,
-    scoped_refptr<HostContentSettingsMap> content_settings_map) {
+    scoped_refptr<HostContentSettingsMap> content_settings_map,
+    bool clear_local_state_on_exit) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   if (!has_initialized_thread_ids) {
@@ -33,16 +34,23 @@ void ChromeAppCacheService::InitializeOnIOThread(
   host_contents_settings_map_ = content_settings_map;
   registrar_.Add(
       this, NotificationType::PURGE_MEMORY, NotificationService::AllSources());
+  SetClearLocalStateOnExit(clear_local_state_on_exit);
+  if (!is_incognito)
+    cache_path_ = profile_path.Append(chrome::kAppCacheDirname);
 
   // Init our base class.
-  Initialize(
-      is_incognito ? FilePath() : profile_path.Append(chrome::kAppCacheDirname),
-      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE));
+  Initialize(cache_path_ ,
+             BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE));
   set_appcache_policy(this);
 }
 
 ChromeAppCacheService::~ChromeAppCacheService() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  if (clear_local_state_on_exit_ && !cache_path_.empty())
+    BrowserThread::PostTask(
+        BrowserThread::FILE, FROM_HERE,
+        NewRunnableFunction(file_util::Delete, cache_path_, true));

[michaeln, 2011/01/05 00:59:14]

I think if this task was posted to the DB thread from the base class's dtor
(after having reset the storage_ data member to NULL), then you'd be guaranteed
that all of the files within the directory had been closed.

AppCacheStorageImpl holds ptrs to the AppCacheDiskCache and the
AppCacheDatabase. The disk cache closes all file prior to return from it's dtor.
The database deletion occurs on the DB thread some time after
AppCacheStorageImpl has been deleted (DeleteSoon is used on the AppCacheDatabase
instance).

[pastarmovj, 2011/01/05 10:47:07]

After discussing this with Jochen, we decided that it is still better to keep
this clean up code in the ChromeAppCacheService class as it is Chrome specific.
However to ensure the code is only run after the destructor of the base class
has been run we post a task to the IO thread to run the actual deletion on the
DB thread. This will ensure it is run after the db has been closed. 

However if you prefer I will move this step to the base class.

The original code in the first revision of this CL went even further and deleted
the file in the actual database class. This is the safest solution as deletion
is perfectly synchronized with the rest of the db handling code but as you can
see from the size of the commit it required much bigger intervention on the
code. This also answers your question in the previous comment why am I asking
for the structure of the appcache and the need of the other commit that deleted
the file cache separately. 

 }
 
 void ChromeAppCacheService::SetOriginQuotaInMemory(
@@ -58,9 +66,9 @@ void ChromeAppCacheService::ResetOriginQuotaInMemory(const GURL& origin) {
     storage()->ResetOriginQuotaInMemory(origin);
 }
 
-// static
-void ChromeAppCacheService::ClearLocalState(const FilePath& profile_path) {
-  file_util::Delete(profile_path.Append(chrome::kAppCacheDirname), true);
+void ChromeAppCacheService::SetClearLocalStateOnExit(bool clear_local_state) {
+  AutoLock lock(lock_);

[michaeln, 2011/01/05 00:59:14]

Instead of a lock, it might be better to post a task to set the flag on the IO
thread. As currently coded, you have a race condition around this flag. What if
the pref is changed after the 'service' was constructed but before
InitializeOnIOThread has been run.

This is a fairly common pattern to avoid locks.

void ChromeAppCacheService::SetClearLocalStateOnExit(bool clear_local_state) {
  if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
    PostTask(SetClearLocalStateOnExit, clear_local_state);
    return;
  }
  clear_local_state_on_exit_ = clear_local_state;
}

[pastarmovj, 2011/01/05 10:47:07]

Done.

+  clear_local_state_on_exit_ = clear_local_state;
 }
 
 bool ChromeAppCacheService::CanLoadAppCache(const GURL& manifest_url) {