Drive: Removes unused cache files after the initial feed fetch.

chrome/browser/chromeos/gdata/gdata_file_system.h

Index: chrome/browser/chromeos/gdata/gdata_file_system.h
diff --git a/chrome/browser/chromeos/gdata/gdata_file_system.h b/chrome/browser/chromeos/gdata/gdata_file_system.h
index 0c82a6b385be193206198156d03090aa445120f3..cd4c4efe86bde1d01aa58803ce2597cbb93022c4 100644
--- a/chrome/browser/chromeos/gdata/gdata_file_system.h
+++ b/chrome/browser/chromeos/gdata/gdata_file_system.h
@@ -34,6 +34,7 @@ class DocumentsServiceInterface;
 class DriveWebAppsRegistryInterface;
 class GDataUploaderInterface;
 class GDataWapiFeedLoader;
+class StaleCacheFilesRemover;
 struct UploadFileInfo;
 
 // The production implementation of GDataFileSystemInterface.
@@ -809,10 +810,6 @@ class GDataFileSystem : public GDataFileSystemInterface,
       base::PlatformFileInfo* file_info,
       bool* get_file_info_result);
 
-  // All members should be accessed only on UI thread. Do not post tasks to
-  // other threads with base::Unretained(this).
-  scoped_ptr<GDataDirectoryService> directory_service_;
-
   // The profile hosts the GDataFileSystem via GDataSystemService.
   Profile* profile_;
 
@@ -837,12 +834,22 @@ class GDataFileSystem : public GDataFileSystemInterface,
   // The set of paths opened by OpenFile but not yet closed by CloseFile.
   std::set<FilePath> open_files_;
 
+  // Note: |observers_| should be placed before all the objects this class owns,
+  // because some of them may add themselves into the observers, so this should
+  // be constructed before them and be destructed after them.

[satorux1, 2012/08/21 20:57:06]

Wow, that's subtle. If so, why don't you move this to the very end before
WeakPtr stuff? StaleCacheFilesRemover is owned by this class, but placed below.
why?

[satorux1, 2012/08/21 21:29:06]

Sorry, I misunderstood. So the observer list has to be created *before*
|stale_cache_files_remover_| and destructed *after*
|stale_cache_files_remover_|, right?

I think the issue we were hit was not a memory leak but a use-after-free.
file_system_->RemoveObserver(this) was called after the observer list was
destructed, right?

What about moving the ownership of |stale_cache_files_remover_| to
GDataSystemService?

+  ObserverList<GDataFileSystemInterface::Observer> observers_;
+
+  // All members should be accessed only on UI thread. Do not post tasks to
+  // other threads with base::Unretained(this).
+  scoped_ptr<GDataDirectoryService> directory_service_;
+
   scoped_ptr<PrefChangeRegistrar> pref_registrar_;
 
   // The loader is used to load the feeds.
   scoped_ptr<GDataWapiFeedLoader> feed_loader_;
 
-  ObserverList<GDataFileSystemInterface::Observer> observers_;
+  // The remover is used to remove stale cache files.
+  scoped_ptr<StaleCacheFilesRemover> stale_cache_files_remover_;
 
   scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;