Bug 1258137 - OSFileSystem should not be kept alive by more than 1 Directory, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Sun, 20 Mar 2016 11:58:01 +0100
changeset 289606 00fcb0aaf4e0d8e0d7ec083e8e7d779a2fc7256a
parent 289605 7f0b6d057313534b899bcc0c8ed154bc42fa2904
child 289607 06e41d2080a2a0736c320aaa7f8319e51515ce85
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1258137
milestone48.0a1
Bug 1258137 - OSFileSystem should not be kept alive by more than 1 Directory, r=smaug
dom/filesystem/DeviceStorageFileSystem.cpp
dom/filesystem/DeviceStorageFileSystem.h
dom/filesystem/Directory.cpp
dom/filesystem/FileSystemBase.h
dom/filesystem/OSFileSystem.cpp
dom/filesystem/OSFileSystem.h
--- a/dom/filesystem/DeviceStorageFileSystem.cpp
+++ b/dom/filesystem/DeviceStorageFileSystem.cpp
@@ -57,16 +57,27 @@ DeviceStorageFileSystem::DeviceStorageFi
     = DeviceStorageTypeChecker::CreateOrGet();
   MOZ_ASSERT(typeChecker);
 }
 
 DeviceStorageFileSystem::~DeviceStorageFileSystem()
 {
 }
 
+already_AddRefed<FileSystemBase>
+DeviceStorageFileSystem::Clone()
+{
+  RefPtr<DeviceStorageFileSystem> fs =
+    new DeviceStorageFileSystem(mStorageType, mStorageName);
+
+  fs->mWindowId = mWindowId;
+
+  return fs.forget();
+}
+
 void
 DeviceStorageFileSystem::Init(nsDOMDeviceStorage* aDeviceStorage)
 {
   MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread!");
   MOZ_ASSERT(aDeviceStorage);
   nsCOMPtr<nsPIDOMWindowInner> window = aDeviceStorage->GetOwner();
   MOZ_ASSERT(window->IsInnerWindow());
   mWindowId = window->WindowID();
--- a/dom/filesystem/DeviceStorageFileSystem.h
+++ b/dom/filesystem/DeviceStorageFileSystem.h
@@ -22,16 +22,19 @@ public:
   DeviceStorageFileSystem(const nsAString& aStorageType,
                           const nsAString& aStorageName);
 
   void
   Init(nsDOMDeviceStorage* aDeviceStorage);
 
   // Overrides FileSystemBase
 
+  virtual already_AddRefed<FileSystemBase>
+  Clone() override;
+
   virtual void
   Shutdown() override;
 
   virtual nsISupports*
   GetParentObject() const override;
 
   virtual void
   GetRootName(nsAString& aRetval) const override;
--- a/dom/filesystem/Directory.cpp
+++ b/dom/filesystem/Directory.cpp
@@ -153,24 +153,30 @@ Directory::Create(nsISupports* aParent, 
   return directory.forget();
 }
 
 Directory::Directory(nsISupports* aParent,
                      nsIFile* aFile,
                      DirectoryType aType,
                      FileSystemBase* aFileSystem)
   : mParent(aParent)
-  , mFileSystem(aFileSystem)
   , mFile(aFile)
   , mType(aType)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aFile);
 
   // aFileSystem can be null. In this case we create a OSFileSystem when needed.
+  if (aFileSystem) {
+    // More likely, this is a OSFileSystem. This object keeps a reference of
+    // mParent but it's not cycle collectable and to avoid manual
+    // addref/release, it's better to have 1 object per directory. For this
+    // reason we clone it here.
+    mFileSystem = aFileSystem->Clone();
+  }
 }
 
 Directory::~Directory()
 {
 }
 
 nsISupports*
 Directory::GetParentObject() const
--- a/dom/filesystem/FileSystemBase.h
+++ b/dom/filesystem/FileSystemBase.h
@@ -29,16 +29,19 @@ public:
 
   virtual void
   Shutdown();
 
   // SerializeDOMPath the FileSystem to string.
   virtual void
   SerializeDOMPath(nsAString& aOutput) const = 0;
 
+  virtual already_AddRefed<FileSystemBase>
+  Clone() = 0;
+
   virtual nsISupports*
   GetParentObject() const;
 
   /*
    * Get the virtual name of the root directory. This name will be exposed to
    * the content page.
    */
   virtual void
--- a/dom/filesystem/OSFileSystem.cpp
+++ b/dom/filesystem/OSFileSystem.cpp
@@ -25,16 +25,27 @@ OSFileSystem::OSFileSystem(const nsAStri
   // access different parts of devices storage like Pictures, or Videos, etc.
   mRequiresPermissionChecks = false;
 
 #ifdef DEBUG
   mPermission.AssignLiteral("never-used");
 #endif
 }
 
+already_AddRefed<FileSystemBase>
+OSFileSystem::Clone()
+{
+  RefPtr<OSFileSystem> fs = new OSFileSystem(mLocalRootPath);
+  if (mParent) {
+    fs->Init(mParent);
+  }
+
+  return fs.forget();
+}
+
 void
 OSFileSystem::Init(nsISupports* aParent)
 {
   MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread!");
   MOZ_ASSERT(!mParent, "No duple Init() calls");
   MOZ_ASSERT(aParent);
   mParent = aParent;
 
--- a/dom/filesystem/OSFileSystem.h
+++ b/dom/filesystem/OSFileSystem.h
@@ -17,16 +17,19 @@ class OSFileSystem final : public FileSy
 public:
   explicit OSFileSystem(const nsAString& aRootDir);
 
   void
   Init(nsISupports* aParent);
 
   // Overrides FileSystemBase
 
+  virtual already_AddRefed<FileSystemBase>
+  Clone() override;
+
   virtual nsISupports*
   GetParentObject() const override;
 
   virtual void
   GetRootName(nsAString& aRetval) const override;
 
   virtual bool
   IsSafeFile(nsIFile* aFile) const override;