Add ScopedFile class which supports scope-out deletion and/or callbacks

webkit/blob/shareable_file_reference.cc

Index: webkit/blob/shareable_file_reference.cc
diff --git a/webkit/blob/shareable_file_reference.cc b/webkit/blob/shareable_file_reference.cc
index 07acac3a55e5598c4fd4bc8f7a0946c454216009..448e2cfd9b881fb0aaa3267b2f1ac146571aa628 100644
--- a/webkit/blob/shareable_file_reference.cc
+++ b/webkit/blob/shareable_file_reference.cc
@@ -6,22 +6,17 @@
 
 #include <map>
 
-#include "base/file_util.h"
-#include "base/files/file_util_proxy.h"
 #include "base/lazy_instance.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "base/task_runner.h"
-#include "base/threading/thread_checker.h"
+#include "base/threading/non_thread_safe.h"
 
 namespace webkit_blob {
 
 namespace {
 
 // A shareable file map with enforcement of thread checker.
-// This map may get deleted on a different thread in AtExitManager at the
-// very end on the main thread (at the point all other threads must be
-// terminated), so we use ThreadChecker rather than NonThreadSafe and do not
-// check thread in the dtor.
-class ShareableFileMap {
+class ShareableFileMap : public base::NonThreadSafe {
  public:
   typedef std::map<base::FilePath, ShareableFileReference*> FileMap;
   typedef FileMap::iterator iterator;
@@ -30,6 +25,10 @@ class ShareableFileMap {
 
   ShareableFileMap() {}
 
+  ~ShareableFileMap() {
+    DetachFromThread();
+  }
+
   iterator Find(key_type key) {
     DCHECK(CalledOnValidThread());
     return file_map_.find(key);
@@ -50,13 +49,8 @@ class ShareableFileMap {
     file_map_.erase(key);
   }
 
-  bool CalledOnValidThread() const {
-    return thread_checker_.CalledOnValidThread();
-  }
-
  private:
   FileMap file_map_;
-  base::ThreadChecker thread_checker_;
   DISALLOW_COPY_AND_ASSIGN(ShareableFileMap);
 };
 
@@ -75,21 +69,43 @@ scoped_refptr<ShareableFileReference> ShareableFileReference::Get(
 
 // static
 scoped_refptr<ShareableFileReference> ShareableFileReference::GetOrCreate(
-    const base::FilePath& path, FinalReleasePolicy policy,
+    const base::FilePath& path,
+    FinalReleasePolicy policy,
     base::TaskRunner* file_task_runner) {
-  DCHECK(file_task_runner);
+  ScopedFile::ScopeOutPolicy scope_out_policy =
+      ScopedFile::DONT_DELETE_ON_SCOPE_OUT;
+  switch (policy) {
+    case DELETE_ON_FINAL_RELEASE:
+      scope_out_policy = ScopedFile::DELETE_ON_SCOPE_OUT;
+      break;
+    case DONT_DELETE_ON_FINAL_RELEASE:
+      scope_out_policy = ScopedFile::DONT_DELETE_ON_SCOPE_OUT;
+      break;
+    default:
+      NOTREACHED();
+  }
+  return GetOrCreate(ScopedFile(path, scope_out_policy, file_task_runner));
+}
+
+// static
+scoped_refptr<ShareableFileReference> ShareableFileReference::GetOrCreate(
+    ScopedFile scoped_file) {
+  if (scoped_file.empty())
+    return NULL;
   typedef std::pair<ShareableFileMap::iterator, bool> InsertResult;
 
   // Required for VS2010: http://connect.microsoft.com/VisualStudio/feedback/details/520043/error-converting-from-null-to-a-pointer-type-in-std-pair
   webkit_blob::ShareableFileReference* null_reference = NULL;
   InsertResult result = g_file_map.Get().Insert(
-      ShareableFileMap::value_type(path, null_reference));
-  if (result.second == false)
+      ShareableFileMap::value_type(scoped_file.path(), null_reference));
+  if (result.second == false) {
+    scoped_file.Release();

[michaeln, 2013/04/22 21:07:03]

if there are existing callbacks associated with scoped_file, what happens to
them? should they be moved over to the pre-existing scoped file for that path?

[kinuko, 2013/04/23 06:31:29]

I have a mixed feeling about how to do in such a case as we ignore the
scope-out-policy of the new scoped_file one anyway. For now I just added a
comment about the behavior in .h and also added TODO (so that we can revisit if
we find it doesn't work)

     return scoped_refptr<ShareableFileReference>(result.first->second);
+  }
 
   // Wasn't in the map, create a new reference and store the pointer.
   scoped_refptr<ShareableFileReference> reference(
-      new ShareableFileReference(path, policy, file_task_runner));
+      new ShareableFileReference(scoped_file.Pass()));
   result.first->second = reference.get();
   return reference;
 }
@@ -97,29 +113,18 @@ scoped_refptr<ShareableFileReference> ShareableFileReference::GetOrCreate(
 void ShareableFileReference::AddFinalReleaseCallback(
     const FinalReleaseCallback& callback) {
   DCHECK(g_file_map.Get().CalledOnValidThread());
-  final_release_callbacks_.push_back(callback);
+  scoped_file_.AddScopeOutCallback(
+      callback, base::MessageLoopProxy::current());
 }
 
-ShareableFileReference::ShareableFileReference(
-    const base::FilePath& path, FinalReleasePolicy policy,
-    base::TaskRunner* file_task_runner)
-    : path_(path),
-      final_release_policy_(policy),
-      file_task_runner_(file_task_runner) {
-  DCHECK(g_file_map.Get().Find(path_)->second == NULL);
+ShareableFileReference::ShareableFileReference(ScopedFile scoped_file)
+    : scoped_file_(scoped_file.Pass()) {
+  DCHECK(g_file_map.Get().Find(path())->second == NULL);
 }
 
 ShareableFileReference::~ShareableFileReference() {
-  DCHECK(g_file_map.Get().Find(path_)->second == this);
-  g_file_map.Get().Erase(path_);
-
-  for (size_t i = 0; i < final_release_callbacks_.size(); i++)
-    final_release_callbacks_[i].Run(path_);
-
-  if (final_release_policy_ == DELETE_ON_FINAL_RELEASE) {
-    base::FileUtilProxy::Delete(file_task_runner_, path_, false /* recursive */,
-                                base::FileUtilProxy::StatusCallback());
-  }
+  DCHECK(g_file_map.Get().Find(path())->second == this);
+  g_file_map.Get().Erase(path());
 }
 
 }  // namespace webkit_blob