AppCaches which belong to hosted apps are not protected from deletion

chrome/browser/profiles/profile_impl.cc

Index: chrome/browser/profiles/profile_impl.cc
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index 6599b978a8cba3bdcec2051fc9b552119086d46c..ff1c9ce3fb8ab606e37d7af0ceb2aa33475f0167 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -14,6 +14,7 @@
 #include "base/path_service.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
+#include "base/synchronization/waitable_event.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier.h"
 #include "chrome/browser/autofill/personal_data_manager.h"
 #include "chrome/browser/background/background_contents_service_factory.h"
@@ -589,6 +590,12 @@ ProfileImpl::~ProfileImpl() {
       Source<Profile>(this),
       NotificationService::NoDetails());
 
+  if (appcache_service_ && prefs_->GetBoolean(prefs::kClearSiteDataOnExit)) {
+    // We don't own event
+    base::WaitableEvent* event = appcache_service_->ClearAppCache(NULL);
+    event->Wait();

[michaeln, 2011/07/02 00:29:09]

We should not block the UI thread for this as profiles can be deleted at times
other than shutdown.

I think a decent place to wait for completion would be in the vicinity of the
call to WaitForPluginDataRemoverToFinish() in ~BrowserProcessImpl(). The trick
is figuring out what to wait on exactly. 

We might be able to generalize this a bit. The code below is not correct since
it creates complex objects as statics (the WaitableEvent), but i'm just trying
to get across the general idea here. If this existed i imagine
WaitForPluginDataRemoverToFinish() could be implemented in terms of this class
too. For ChromeAppCacheService to utilize a class like this it would have to be
in the /content layer.

class PreventBrowserShutdown {
 public:
  // Created by classes like the ChromeAppCacheService prior to starting
  // its 'clearonexit' work, and then deleted by the ChromeAppCacheService when
  // that work is done.
  PreventBrowserShutdown();
  ~PreventBrowserShutdown();

  // Called by ~BrowserProcessImpl to wait for all instances to be
  // deleted prior to tearing down the background threads.
  static void Wait(int timeout_milliseconds);

 private:
  static base::AtomicRefCount instance_count_;
  static WaitableEvent event_;
};

PreventBrowserShutdown::PreventBrowserShutdown() {
  AtomicRefCountInc(&instance_count_);
  event_.Reset();
}

PreventBrowserShutdown::~PreventBrowserShutdown() {
  if (!AtomicRefCountDec(&instance_count_))
    event_.Signal();
}

void PreventBrowserShutdown::Wait(int timeout_millis) {
  // We can assume that no new instances will be created after
  // the time that this method is invoked.
  if (AtomicRefCountIsZero(&instance_count_))
    return;
  event_.TimedWait(timeout_millis);
}

[jochen (gone - plz use gerrit), 2011/07/06 09:41:45]

I agree that we shouldn't wait on the UI thread. OTOH, why wait for the deletion
to complete? 

At this point, we can be sure that the IO thread will be drained before the
browser shuts down, so we can just fire the task to the IO thread and forget
about it

[marja(google), 2011/07/06 11:44:39]

The appcache deletion procedure posts tasks across the IO and DB threads (so, a
task is posted to the IO thread, when that is executed, it posts a task to the
DB thread, when that is executed, it posts again to the IO thread, etc.) So it
could happen that there are no tasks in the IO thread, so the IO thread can shut
down, and when the task in the DB thread is executed, it's too late to post
anything to the IO thread any more. Or am I missing something?

+  }
+
   StopCreateSessionServiceTimer();
 
   // Remove pref observers
@@ -1322,8 +1329,7 @@ void ProfileImpl::CreateQuotaManagerAndClients() {
           IsOffTheRecord()
               ? FilePath() : GetPath().Append(chrome::kAppCacheDirname),
           &GetResourceContext(),
-          make_scoped_refptr(GetExtensionSpecialStoragePolicy()),
-          clear_local_state_on_exit_));
+          make_scoped_refptr(GetExtensionSpecialStoragePolicy())));
 }
 
 WebKitContext* ProfileImpl::GetWebKitContext() {
@@ -1377,10 +1383,6 @@ void ProfileImpl::Observe(NotificationType type,
           webkit_context_->set_clear_local_state_on_exit(
               clear_local_state_on_exit_);
         }
-        if (appcache_service_) {
-          appcache_service_->SetClearLocalStateOnExit(
-              clear_local_state_on_exit_);
-        }
       } else if (*pref_name_in == prefs::kGoogleServicesUsername) {
         ProfileManager* profile_manager = g_browser_process->profile_manager();
         profile_manager->RegisterProfileName(this);