Bug 1758324 - Implement file system directory iterator; r=dom-storage-reviewers,jesup,janv,smaug

Differential Revision: https://phabricator.services.mozilla.com/D140300

dom/fs/api/FileSystemDirectoryHandle.cpp

@@ -5,8 +5,9 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "FileSystemDirectoryHandle.h"
-#include "fs/FileSystemRequestHandler.h"
 
+#include "FileSystemDirectoryIteratorFactory.h"
+#include "fs/FileSystemRequestHandler.h"
 #include "mozilla/ErrorResult.h"
 #include "mozilla/dom/FileSystemDirectoryHandleBinding.h"
 #include "mozilla/dom/FileSystemDirectoryIterator.h"
@@ -47,30 +48,42 @@ FileSystemHandleKind FileSystemDirectoryHandle::Kind() const {
   return FileSystemHandleKind::Directory;
 }
 
-already_AddRefed<FileSystemDirectoryIterator>
-FileSystemDirectoryHandle::Entries() {
-  return MakeRefPtr<FileSystemDirectoryIterator>(GetParentObject()).forget();
+void FileSystemDirectoryHandle::InitAsyncIterator(
+    FileSystemDirectoryHandle::iterator_t* aIterator, ErrorResult& aError) {
+  aIterator->SetData(
+      static_cast<void*>(fs::FileSystemDirectoryIteratorFactory::Create(
+                             mMetadata, aIterator->GetIteratorType())
+                             .release()));
 }
 
-already_AddRefed<FileSystemDirectoryIterator>
-FileSystemDirectoryHandle::Keys() {
-  return MakeRefPtr<FileSystemDirectoryIterator>(GetParentObject()).forget();
+void FileSystemDirectoryHandle::DestroyAsyncIterator(
+    FileSystemDirectoryHandle::iterator_t* aIterator) {
+  auto* it =
+      static_cast<FileSystemDirectoryIterator::Impl*>(aIterator->GetData());
+  delete it;
+  aIterator->SetData(nullptr);
 }
 
-already_AddRefed<FileSystemDirectoryIterator>
-FileSystemDirectoryHandle::Values() {
-  return MakeRefPtr<FileSystemDirectoryIterator>(GetParentObject()).forget();
+already_AddRefed<Promise> FileSystemDirectoryHandle::GetNextPromise(
+    JSContext* /* aCx */, FileSystemDirectoryHandle::iterator_t* aIterator,
+    ErrorResult& aError) {
+  return static_cast<FileSystemDirectoryIterator::Impl*>(aIterator->GetData())
+      ->Next(mGlobal, mManager, aError);
 }
 
 already_AddRefed<Promise> FileSystemDirectoryHandle::GetFileHandle(
     const nsAString& aName, const FileSystemGetFileOptions& aOptions,
     ErrorResult& aError) {
+  MOZ_ASSERT(!mMetadata.entryId().IsEmpty());
+
   RefPtr<Promise> promise = Promise::Create(GetParentObject(), aError);
   if (aError.Failed()) {
     return nullptr;
   }
 
-  promise->MaybeReject(NS_ERROR_NOT_IMPLEMENTED);
+  fs::Name name(aName);
+  fs::FileSystemChildMetadata metadata(mMetadata.entryId(), name);
+  mRequestHandler->GetFileHandle(mManager, metadata, aOptions.mCreate, promise);
 
   return promise.forget();
 }
@@ -78,12 +91,17 @@ already_AddRefed<Promise> FileSystemDirectoryHandle::GetFileHandle(
 already_AddRefed<Promise> FileSystemDirectoryHandle::GetDirectoryHandle(
     const nsAString& aName, const FileSystemGetDirectoryOptions& aOptions,
     ErrorResult& aError) {
+  MOZ_ASSERT(!mMetadata.entryId().IsEmpty());
+
   RefPtr<Promise> promise = Promise::Create(GetParentObject(), aError);
   if (aError.Failed()) {
     return nullptr;
   }
 
-  promise->MaybeReject(NS_ERROR_NOT_IMPLEMENTED);
+  fs::Name name(aName);
+  fs::FileSystemChildMetadata metadata(mMetadata.entryId(), name);
+  mRequestHandler->GetDirectoryHandle(mManager, metadata, aOptions.mCreate,
+                                      promise);
 
   return promise.forget();
 }
@@ -91,12 +109,18 @@ already_AddRefed<Promise> FileSystemDirectoryHandle::GetDirectoryHandle(
 already_AddRefed<Promise> FileSystemDirectoryHandle::RemoveEntry(
     const nsAString& aName, const FileSystemRemoveOptions& aOptions,
     ErrorResult& aError) {
+  MOZ_ASSERT(!mMetadata.entryId().IsEmpty());
+
   RefPtr<Promise> promise = Promise::Create(GetParentObject(), aError);
   if (aError.Failed()) {
     return nullptr;
   }
 
-  promise->MaybeReject(NS_ERROR_NOT_IMPLEMENTED);
+  fs::Name name(aName);
+  fs::FileSystemChildMetadata metadata(mMetadata.entryId(), name);
+
+  mRequestHandler->RemoveEntry(mManager, metadata, aOptions.mRecursive,
+                               promise);
 
   return promise.forget();
 }

dom/fs/api/FileSystemDirectoryHandle.h

@@ -8,6 +8,8 @@
 #define DOM_FS_FILESYSTEMDIRECTORYHANDLE_H_
 
 #include "mozilla/dom/FileSystemHandle.h"
+#include "mozilla/dom/IterableIterator.h"
+#include "mozilla/dom/FileSystemDirectoryIterator.h"
 
 namespace mozilla {
 
@@ -22,6 +24,8 @@ struct FileSystemRemoveOptions;
 
 class FileSystemDirectoryHandle final : public FileSystemHandle {
  public:
+  using iterator_t = AsyncIterableIterator<FileSystemDirectoryHandle>;
+
   FileSystemDirectoryHandle(nsIGlobalObject* aGlobal,
                             RefPtr<FileSystemManager>& aManager,
                             const fs::FileSystemEntryMetadata& aMetadata,
@@ -42,11 +46,13 @@ class FileSystemDirectoryHandle final : public FileSystemHandle {
   // WebIDL Interface
   FileSystemHandleKind Kind() const override;
 
-  [[nodiscard]] already_AddRefed<FileSystemDirectoryIterator> Entries();
+  void InitAsyncIterator(iterator_t* aIterator, ErrorResult& aError);
 
-  [[nodiscard]] already_AddRefed<FileSystemDirectoryIterator> Keys();
+  void DestroyAsyncIterator(iterator_t* aIterator);
 
-  [[nodiscard]] already_AddRefed<FileSystemDirectoryIterator> Values();
+  [[nodiscard]] already_AddRefed<Promise> GetNextPromise(JSContext* aCx,
+                                                         iterator_t* aIterator,
+                                                         ErrorResult& aError);
 
   already_AddRefed<Promise> GetFileHandle(
       const nsAString& aName, const FileSystemGetFileOptions& aOptions,

dom/fs/api/FileSystemDirectoryIterator.cpp

@@ -8,14 +8,11 @@
 
 #include "mozilla/ErrorResult.h"
 #include "mozilla/dom/FileSystemDirectoryIteratorBinding.h"
+#include "mozilla/dom/FileSystemManager.h"
 #include "mozilla/dom/Promise.h"
 
 namespace mozilla::dom {
 
-FileSystemDirectoryIterator::FileSystemDirectoryIterator(
-    nsIGlobalObject* aGlobal)
-    : mGlobal(aGlobal) {}
-
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(FileSystemDirectoryIterator)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   NS_INTERFACE_MAP_ENTRY(nsISupports)
@@ -24,6 +21,11 @@ NS_IMPL_CYCLE_COLLECTING_ADDREF(FileSystemDirectoryIterator);
 NS_IMPL_CYCLE_COLLECTING_RELEASE(FileSystemDirectoryIterator);
 NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(FileSystemDirectoryIterator, mGlobal);
 
+FileSystemDirectoryIterator::FileSystemDirectoryIterator(
+    nsIGlobalObject* aGlobal, RefPtr<FileSystemManager>& aManager,
+    UniquePtr<Impl> aImpl)
+    : mGlobal(aGlobal), mManager(aManager), mImpl(std::move(aImpl)) {}
+
 // WebIDL Boilerplate
 
 nsIGlobalObject* FileSystemDirectoryIterator::GetParentObject() const {
@@ -44,9 +46,8 @@ already_AddRefed<Promise> FileSystemDirectoryIterator::Next(
     return nullptr;
   }
 
-  promise->MaybeReject(NS_ERROR_NOT_IMPLEMENTED);
-
-  return promise.forget();
+  MOZ_ASSERT(mImpl);
+  return mImpl->Next(mGlobal, mManager, aError);
 }
 
 }  // namespace mozilla::dom

dom/fs/api/FileSystemDirectoryIterator.h

@@ -10,6 +10,7 @@
 #include "nsCOMPtr.h"
 #include "nsISupports.h"
 #include "nsWrapperCache.h"
+#include "mozilla/dom/IterableIterator.h"
 
 class nsIGlobalObject;
 
@@ -19,15 +20,30 @@ class ErrorResult;
 
 namespace dom {
 
+class FileSystemManager;
+class IterableIteratorBase;
 class Promise;
 
+// XXX This class isn't used to support iteration anymore. `Impl` should be
+// extracted elsewhere and `FileSystemDirectoryIterator` should be removed
+// completely
 class FileSystemDirectoryIterator : public nsISupports, public nsWrapperCache {
  public:
-  explicit FileSystemDirectoryIterator(nsIGlobalObject* aGlobal);
+  class Impl {
+   public:
+    virtual already_AddRefed<Promise> Next(nsIGlobalObject* aGlobal,
+                                           RefPtr<FileSystemManager>& aManager,
+                                           ErrorResult& aError) = 0;
+    virtual ~Impl() = default;
+  };
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(FileSystemDirectoryIterator)
 
+  explicit FileSystemDirectoryIterator(nsIGlobalObject* aGlobal,
+                                       RefPtr<FileSystemManager>& aManager,
+                                       UniquePtr<Impl> aImpl);
+
   // WebIDL Boilerplate
   nsIGlobalObject* GetParentObject() const;
 
@@ -41,6 +57,11 @@ class FileSystemDirectoryIterator : public nsISupports, public nsWrapperCache {
   virtual ~FileSystemDirectoryIterator() = default;
 
   nsCOMPtr<nsIGlobalObject> mGlobal;
+
+  RefPtr<FileSystemManager> mManager;
+
+ private:
+  UniquePtr<Impl> mImpl;
 };
 
 }  // namespace dom

dom/fs/api/FileSystemHandle.h

@@ -28,7 +28,6 @@ class ErrorResult;
 
 namespace dom {
 
-class DOMString;
 enum class FileSystemHandleKind : uint8_t;
 class FileSystemManager;
 class FileSystemManagerChild;

dom/fs/child/ArrayAppendable.h

@@ -0,0 +1,35 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef DOM_FS_ARRAYAPPENDABLE_H_
+#define DOM_FS_ARRAYAPPENDABLE_H_
+
+#include "nsTArray.h"
+
+class nsIGlobalObject;
+
+namespace mozilla::dom {
+
+class FileSystemManager;
+
+namespace fs {
+
+class FileSystemEntryMetadata;
+
+class ArrayAppendable {
+ public:
+  virtual void append(nsIGlobalObject* aGlobal,
+                      RefPtr<FileSystemManager>& aManager,
+                      const nsTArray<FileSystemEntryMetadata>& aBatch) = 0;
+
+ protected:
+  virtual ~ArrayAppendable() = default;
+};
+
+}  // namespace fs
+}  // namespace mozilla::dom
+
+#endif  // DOM_FS_ARRAYAPPENDABLE_H_

dom/fs/child/FileSystemDirectoryIteratorFactory.cpp

@@ -0,0 +1,296 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "FileSystemDirectoryIteratorFactory.h"
+
+#include "ArrayAppendable.h"
+#include "fs/FileSystemRequestHandler.h"
+#include "jsapi.h"
+#include "mozilla/dom/FileSystemDirectoryHandle.h"
+#include "mozilla/dom/FileSystemDirectoryIterator.h"
+#include "mozilla/dom/FileSystemFileHandle.h"
+#include "mozilla/dom/FileSystemHandle.h"
+#include "mozilla/dom/FileSystemManager.h"
+#include "mozilla/dom/IterableIterator.h"
+#include "mozilla/dom/Promise.h"
+
+namespace mozilla::dom::fs {
+
+namespace {
+
+inline JSContext* GetContext(const RefPtr<Promise>& aPromise) {
+  AutoJSAPI jsapi;
+  if (NS_WARN_IF(!jsapi.Init(aPromise->GetGlobalObject()))) {
+    return nullptr;
+  }
+  return jsapi.cx();
+}
+
+template <IterableIteratorBase::IteratorType Type>
+struct ValueResolver;
+
+template <>
+struct ValueResolver<IterableIteratorBase::Keys> {
+  nsresult operator()(nsIGlobalObject* aGlobal,
+                      RefPtr<FileSystemManager>& aManager,
+                      const FileSystemEntryMetadata& aValue,
+                      const RefPtr<Promise>& aPromise, ErrorResult& aError) {
+    JSContext* cx = GetContext(aPromise);
+    if (!cx) {
+      return NS_ERROR_UNEXPECTED;
+    }
+
+    JS::Rooted<JS::Value> key(cx);
+
+    if (!ToJSValue(cx, aValue.entryName(), &key)) {
+      return NS_ERROR_INVALID_ARG;
+    }
+
+    iterator_utils::ResolvePromiseWithKeyOrValue(cx, aPromise.get(), key,
+                                                 aError);
+
+    return NS_OK;
+  }
+};
+
+template <>
+struct ValueResolver<IterableIteratorBase::Values> {
+  nsresult operator()(nsIGlobalObject* aGlobal,
+                      RefPtr<FileSystemManager>& aManager,
+                      const FileSystemEntryMetadata& aValue,
+                      const RefPtr<Promise>& aPromise, ErrorResult& aError) {
+    JSContext* cx = GetContext(aPromise);
+    if (!cx) {
+      return NS_ERROR_UNEXPECTED;
+    }
+
+    RefPtr<FileSystemHandle> handle;
+
+    if (aValue.directory()) {
+      handle = new FileSystemDirectoryHandle(aGlobal, aManager, aValue);
+    } else {
+      handle = new FileSystemFileHandle(aGlobal, aManager, aValue);
+    }
+
+    JS::Rooted<JS::Value> value(cx);
+    if (!ToJSValue(cx, handle, &value)) {
+      return NS_ERROR_INVALID_ARG;
+    }
+
+    iterator_utils::ResolvePromiseWithKeyOrValue(cx, aPromise.get(), value,
+                                                 aError);
+
+    return NS_OK;
+  }
+};
+
+template <>
+struct ValueResolver<IterableIteratorBase::Entries> {
+  nsresult operator()(nsIGlobalObject* aGlobal,
+                      RefPtr<FileSystemManager>& aManager,
+                      const FileSystemEntryMetadata& aValue,
+                      const RefPtr<Promise>& aPromise, ErrorResult& aError) {
+    JSContext* cx = GetContext(aPromise);
+    if (!cx) {
+      return NS_ERROR_UNEXPECTED;
+    }
+
+    JS::Rooted<JS::Value> key(cx);
+    if (!ToJSValue(cx, aValue.entryName(), &key)) {
+      return NS_ERROR_INVALID_ARG;
+    }
+
+    RefPtr<FileSystemHandle> handle;
+
+    if (aValue.directory()) {
+      handle = new FileSystemDirectoryHandle(aGlobal, aManager, aValue);
+    } else {
+      handle = new FileSystemFileHandle(aGlobal, aManager, aValue);
+    }
+
+    JS::Rooted<JS::Value> value(cx);
+    if (!ToJSValue(cx, handle, &value)) {
+      return NS_ERROR_INVALID_ARG;
+    }
+
+    iterator_utils::ResolvePromiseWithKeyAndValue(cx, aPromise.get(), key,
+                                                  value, aError);
+
+    return NS_OK;
+  }
+};
+
+// TODO: PageSize could be compile-time shared between content and parent
+template <class ValueResolver, size_t PageSize = 1024u>
+class DoubleBufferQueueImpl
+    : public ArrayAppendable,
+      public mozilla::dom::FileSystemDirectoryIterator::Impl {
+  static_assert(PageSize > 0u);
+
+ public:
+  using DataType = FileSystemEntryMetadata;
+  explicit DoubleBufferQueueImpl(const FileSystemEntryMetadata& aMetadata)
+      : mEntryId(aMetadata.entryId()),
+        mData(),
+        mWithinPageEnd(0u),
+        mWithinPageIndex(0u),
+        mCurrentPageIsLastPage(true),
+        mPageNumber(0u) {}
+
+  // XXX This doesn't have to be public
+  void ResolveValue(nsIGlobalObject* aGlobal,
+                    RefPtr<FileSystemManager>& aManager,
+                    const Maybe<DataType>& aValue, RefPtr<Promise> aPromise,
+                    ErrorResult& aError) {
+    MOZ_ASSERT(aPromise);
+    MOZ_ASSERT(aPromise.get());
+
+    AutoJSAPI jsapi;
+    if (NS_WARN_IF(!jsapi.Init(aPromise->GetGlobalObject()))) {
+      aPromise->MaybeReject(NS_ERROR_DOM_UNKNOWN_ERR);
+      aError = NS_ERROR_DOM_UNKNOWN_ERR;
+      return;
+    }
+
+    JSContext* aCx = jsapi.cx();
+    MOZ_ASSERT(aCx);
+
+    if (!aValue) {
+      iterator_utils::ResolvePromiseForFinished(aCx, aPromise.get(), aError);
+      return;
+    }
+
+    ValueResolver{}(aGlobal, aManager, *aValue, aPromise, aError);
+  }
+
+  void append(nsIGlobalObject* aGlobal, RefPtr<FileSystemManager>& aManager,
+              const nsTArray<FileSystemEntryMetadata>& aNewPage) override {
+    MOZ_ASSERT(0u == mWithinPageIndex);
+
+    nsTArray<DataType> batch;
+    for (const auto& it : aNewPage) {
+      batch.AppendElement(it);
+    }
+
+    const size_t batchSize = std::min(PageSize, aNewPage.Length());
+    mData.InsertElementsAt(
+        PageSize * static_cast<size_t>(!mCurrentPageIsLastPage),
+        batch.Elements(), batchSize);
+    mWithinPageEnd += batchSize;
+
+    if (!mPendingPromise) {
+      return;
+    }
+
+    Maybe<DataType> value;
+    if (0 != aNewPage.Length()) {
+      nextInternal(value);
+    }
+
+    IgnoredErrorResult rv;
+    ResolveValue(aGlobal, aManager, value, mPendingPromise, rv);
+
+    mPendingPromise = nullptr;
+  }
+
+  already_AddRefed<Promise> Next(nsIGlobalObject* aGlobal,
+                                 RefPtr<FileSystemManager>& aManager,
+                                 ErrorResult& aError) override {
+    RefPtr<Promise> promise = Promise::Create(aGlobal, aError);
+    if (aError.Failed()) {
+      return nullptr;
+    }
+
+    next(aGlobal, aManager, promise, aError);
+
+    return promise.forget();
+  }
+
+  ~DoubleBufferQueueImpl() = default;
+
+ protected:
+  void next(nsIGlobalObject* aGlobal, RefPtr<FileSystemManager>& aManager,
+            RefPtr<Promise> aResult, ErrorResult& aError) {
+    MOZ_ASSERT(aResult);
+    if (mPendingPromise) {
+      aResult->MaybeRejectWithNotFoundError("Result was not awaited");
+      return;
+    }
+
+    Maybe<DataType> rawValue;
+
+    // TODO: Would it be better to prefetch the items before
+    // we hit the end of a page?
+    // How likely it is that it would that lead to wasted fetch operations?
+    if (0u == mWithinPageIndex) {
+      mPendingPromise = aResult;
+      FileSystemRequestHandler{}.GetEntries(aManager, mEntryId, mPageNumber,
+                                            mPendingPromise, *this);
+      ++mPageNumber;
+      return;
+    }
+
+    nextInternal(rawValue);
+
+    ResolveValue(aGlobal, aManager, rawValue, aResult, aError);
+  }
+
+  bool nextInternal(Maybe<DataType>& aNext) {
+    if (mWithinPageIndex >= mWithinPageEnd) {
+      return false;
+    }
+
+    const auto previous =
+        static_cast<size_t>(!mCurrentPageIsLastPage) * PageSize +
+        mWithinPageIndex;
+    MOZ_ASSERT(2u * PageSize > previous);
+    MOZ_ASSERT(previous < mData.Length());
+
+    ++mWithinPageIndex;
+
+    if (mWithinPageIndex == PageSize) {
+      // Page end reached
+      mWithinPageIndex = 0u;
+      mCurrentPageIsLastPage = !mCurrentPageIsLastPage;
+    }
+
+    aNext = Some(mData[previous]);
+    return true;
+  }
+
+  const EntryId mEntryId;
+
+  nsTArray<DataType> mData;  // TODO: Fixed size above one page?
+
+  size_t mWithinPageEnd = 0u;
+  size_t mWithinPageIndex = 0u;
+  bool mCurrentPageIsLastPage = true;  // In the beginning, first page is free
+  PageNumber mPageNumber = 0u;
+  RefPtr<Promise> mPendingPromise;
+};
+
+template <IterableIteratorBase::IteratorType Type>
+using UnderlyingQueue = DoubleBufferQueueImpl<ValueResolver<Type>>;
+
+}  // namespace
+
+UniquePtr<mozilla::dom::FileSystemDirectoryIterator::Impl>
+FileSystemDirectoryIteratorFactory::Create(
+    const FileSystemEntryMetadata& aMetadata,
+    IterableIteratorBase::IteratorType aType) {
+  if (IterableIteratorBase::Entries == aType) {
+    return MakeUnique<UnderlyingQueue<IterableIteratorBase::Entries>>(
+        aMetadata);
+  }
+
+  if (IterableIteratorBase::Values == aType) {
+    return MakeUnique<UnderlyingQueue<IterableIteratorBase::Values>>(aMetadata);
+  }
+
+  return MakeUnique<UnderlyingQueue<IterableIteratorBase::Keys>>(aMetadata);
+}
+
+}  // namespace mozilla::dom::fs

dom/fs/child/FileSystemDirectoryIteratorFactory.h

@@ -0,0 +1,24 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef DOM_FS_CHILD_FILESYSTEMDIRECTORYITERATORFACTORY_H_
+#define DOM_FS_CHILD_FILESYSTEMDIRECTORYITERATORFACTORY_H_
+
+#include "mozilla/dom/FileSystemDirectoryIterator.h"
+
+namespace mozilla::dom::fs {
+
+class FileSystemEntryMetadata;
+
+struct FileSystemDirectoryIteratorFactory {
+  static UniquePtr<mozilla::dom::FileSystemDirectoryIterator::Impl> Create(
+      const FileSystemEntryMetadata& aMetadata,
+      IterableIteratorBase::IteratorType aType);
+};
+
+}  // namespace mozilla::dom::fs
+
+#endif  // DOM_FS_CHILD_FILESYSTEMDIRECTORYITERATORFACTORY_H_

dom/fs/child/FileSystemRequestHandler.cpp

@@ -6,6 +6,7 @@
 
 #include "fs/FileSystemRequestHandler.h"
 
+#include "ArrayAppendable.h"
 #include "fs/FileSystemConstants.h"
 #include "mozilla/dom/File.h"
 #include "mozilla/dom/FileSystemDirectoryHandle.h"
@@ -42,21 +43,17 @@ void GetDirectoryContentsResponseHandler(
   // TODO: Add page size to FileSystemConstants, preallocate and handle overflow
   const auto& listing = aResponse.get_FileSystemDirectoryListing();
 
-  nsTArray<RefPtr<FileSystemHandle>> batch;
+  nsTArray<FileSystemEntryMetadata> batch;
 
   for (const auto& it : listing.files()) {
-    RefPtr<FileSystemHandle> handle =
-        new FileSystemFileHandle(aGlobal, aManager, it);
-    batch.AppendElement(handle);
+    batch.AppendElement(it);
   }
 
   for (const auto& it : listing.directories()) {
-    RefPtr<FileSystemHandle> handle =
-        new FileSystemDirectoryHandle(aGlobal, aManager, it);
-    batch.AppendElement(handle);
+    batch.AppendElement(it);
   }
 
-  aSink.append(batch);
+  aSink.append(aGlobal, aManager, batch);
 }
 
 RefPtr<FileSystemDirectoryHandle> MakeResolution(

dom/fs/child/moz.build

@@ -11,6 +11,7 @@ EXPORTS.mozilla.dom += [
 UNIFIED_SOURCES += [
     "FileSystemBackgroundRequestHandler.cpp",
     "FileSystemChildFactory.cpp",
+    "FileSystemDirectoryIteratorFactory.cpp",
     "FileSystemRequestHandler.cpp",
 ]
 

dom/fs/include/fs/FileSystemRequestHandler.h

@@ -20,15 +20,11 @@ class Promise;
 
 namespace fs {
 
+class ArrayAppendable;
 class FileSystemChildMetadata;
 class FileSystemEntryMetadata;
 class FileSystemEntryPair;
 
-class ArrayAppendable {
- public:
-  void append(const nsTArray<RefPtr<FileSystemHandle>>& /* aBatch */) {}
-};
-
 class FileSystemRequestHandler {
  public:
   virtual void GetRootHandle(RefPtr<FileSystemManager> aManager,

dom/fs/parent/FileSystemManagerParent.cpp

@@ -141,7 +141,20 @@ IPCResult FileSystemManagerParent::RecvResolveMsg(
 
 IPCResult FileSystemManagerParent::RecvGetEntriesMsg(
     FileSystemGetEntriesRequest&& aRequest, GetEntriesMsgResolver&& aResolver) {
-  FileSystemGetEntriesResponse response(NS_ERROR_NOT_IMPLEMENTED);
+  MOZ_ASSERT(!aRequest.parentId().IsEmpty());
+  MOZ_ASSERT(mDataManager);
+
+  auto reportError = [&aResolver](const QMResult& aRv) {
+    FileSystemGetEntriesResponse response(ToNSResult(aRv));
+    aResolver(response);
+  };
+
+  QM_TRY_UNWRAP(FileSystemDirectoryListing entries,
+                mDataManager->MutableDatabaseManagerPtr()->GetDirectoryEntries(
+                    aRequest.parentId(), aRequest.page()),
+                IPC_OK(), reportError);
+
+  FileSystemGetEntriesResponse response(entries);
   aResolver(response);
 
   return IPC_OK();

dom/fs/test/common/test_basics.js

@@ -50,17 +50,8 @@ exported_symbols.testKeysIteratorNextIsCallable = async function() {
   const it = await root.keys();
   Assert.ok(!!it, "Does root support keys iterator?");
 
-  try {
-    await it.next();
-    Assert.ok(false, "Should have thrown");
-  } catch (ex) {
-    Assert.ok(true, "Should have thrown");
-    Assert.equal(
-      ex.result,
-      Cr.NS_ERROR_NOT_IMPLEMENTED,
-      "Threw the right result code"
-    );
-  }
+  const item = await it.next();
+  Assert.ok(!!item, "Should return an item");
 };
 
 exported_symbols.testDirectoryHandleSupportsValuesIterator = async function() {
@@ -76,17 +67,8 @@ exported_symbols.testValuesIteratorNextIsCallable = async function() {
   const it = await root.values();
   Assert.ok(!!it, "Does root support values iterator?");
 
-  try {
-    await it.next();
-    Assert.ok(false, "Should have thrown");
-  } catch (ex) {
-    Assert.ok(true, "Should have thrown");
-    Assert.equal(
-      ex.result,
-      Cr.NS_ERROR_NOT_IMPLEMENTED,
-      "Threw the right result code"
-    );
-  }
+  const item = await it.next();
+  Assert.ok(!!item, "Should return an item");
 };
 
 exported_symbols.testDirectoryHandleSupportsEntriesIterator = async function() {
@@ -102,69 +84,41 @@ exported_symbols.testEntriesIteratorNextIsCallable = async function() {
   const it = await root.entries();
   Assert.ok(!!it, "Does root support entries iterator?");
 
-  try {
-    await it.next();
-    Assert.ok(false, "Should have thrown");
-  } catch (ex) {
-    Assert.ok(true, "Should have thrown");
-    Assert.equal(
-      ex.result,
-      Cr.NS_ERROR_NOT_IMPLEMENTED,
-      "Threw the right result code"
-    );
-  }
+  const item = await it.next();
+  Assert.ok(!!item, "Should return an item");
 };
 
 exported_symbols.testGetFileHandleIsCallable = async function() {
   const root = await navigator.storage.getDirectory();
   const allowCreate = { create: true };
 
-  try {
-    await root.getFileHandle("name", allowCreate);
-
-    Assert.ok(false, "Should have thrown");
-  } catch (ex) {
-    Assert.ok(true, "Should have thrown");
-    Assert.equal(
-      ex.result,
-      Cr.NS_ERROR_NOT_IMPLEMENTED,
-      "Threw the right result code"
-    );
-  }
+  const item = await root.getFileHandle("fileName", allowCreate);
+  Assert.ok(!!item, "Should return an item");
 };
 
 exported_symbols.testGetDirectoryHandleIsCallable = async function() {
   const root = await navigator.storage.getDirectory();
   const allowCreate = { create: true };
 
-  try {
-    await root.getDirectoryHandle("name", allowCreate);
-
-    Assert.ok(false, "Should have thrown");
-  } catch (ex) {
-    Assert.ok(true, "Should have thrown");
-    Assert.equal(
-      ex.result,
-      Cr.NS_ERROR_NOT_IMPLEMENTED,
-      "Threw the right result code"
-    );
-  }
+  const item = await root.getDirectoryHandle("dirName", allowCreate);
+  Assert.ok(!!item, "Should return an item");
 };
 
 exported_symbols.testRemoveEntryIsCallable = async function() {
   const root = await navigator.storage.getDirectory();
   const removeOptions = { recursive: true };
 
+  await root.removeEntry("fileName", removeOptions);
+  await root.removeEntry("dirName", removeOptions);
   try {
-    await root.removeEntry("root", removeOptions);
-
+    await root.removeEntry("doesNotExist", removeOptions);
     Assert.ok(false, "Should have thrown");
   } catch (ex) {
     Assert.ok(true, "Should have thrown");
     Assert.equal(
-      ex.result,
-      Cr.NS_ERROR_NOT_IMPLEMENTED,
-      "Threw the right result code"
+      ex.message,
+      "Unknown failure",
+      "Threw the right error message"
     );
   }
 };

dom/fs/test/common/test_fileSystemDirectoryHandle.js

@@ -4,12 +4,6 @@
  */
 
 exported_symbols.smokeTest = async function smokeTest() {
-  const step = async aIter => {
-    return aIter.next().catch(err => {
-      /* console.log(err.message); */
-    });
-  };
-
   const storage = navigator.storage;
   const subdirectoryNames = new Set(["Documents", "Downloads", "Music"]);
   const allowCreate = { create: true };
@@ -18,24 +12,15 @@ exported_symbols.smokeTest = async function smokeTest() {
     let root = await storage.getDirectory();
     Assert.ok(root, "Can we access the root directory?");
 
-    let it = root.values();
+    let it = await root.values();
     Assert.ok(!!it, "Does root have values iterator?");
 
-    let elem = await step(it);
-    Assert.ok(!elem, "Is root directory empty?");
+    let elem = await it.next();
+    Assert.ok(elem.done, "Is root directory empty?");
 
     for (let dirName of subdirectoryNames) {
-      try {
-        await root.getDirectoryHandle(dirName, allowCreate);
-      } catch (err) {
-        Assert.equal(
-          Cr.NS_ERROR_NOT_IMPLEMENTED,
-          err.result,
-          "Iterator not implemented yet"
-        );
-      }
-      // TODO: Implement iterator
-      // Assert.ok(true, "Was it possible to add subdirectory " + dirName + "?");
+      await root.getDirectoryHandle(dirName, allowCreate);
+      Assert.ok(true, "Was it possible to add subdirectory " + dirName + "?");
     }
   }
 
@@ -43,18 +28,19 @@ exported_symbols.smokeTest = async function smokeTest() {
     let root = await storage.getDirectory();
     Assert.ok(root, "Can we refresh the root directory?");
 
-    let it = root.values();
+    let it = await root.values();
     Assert.ok(!!it, "Does root have values iterator?");
 
     let hasElements = false;
     let hangGuard = 0;
-    for (let elem = await step(it); elem; elem = await step(it)) {
-      Assert.ok(!!elem, "Is element not non-empty?");
+    for await (let [key, elem] of root.entries()) {
+      Assert.ok(elem, "Is element not non-empty?");
       Assert.equal("directory", elem.kind, "Is found item a directory?");
       Assert.ok(
         elem.name.length >= 1 && elem.name.match("^[A-Za-z]{1,64}"),
         "Are names of the elements strings?"
       );
+      Assert.equal(key, elem.name);
       Assert.ok(subdirectoryNames.has(elem.name), "Is name among known names?");
       hasElements = true;
       ++hangGuard;
@@ -63,42 +49,31 @@ exported_symbols.smokeTest = async function smokeTest() {
       }
     }
 
-    Assert.ok(!hasElements, "Iterator not implemented yet");
-    // TODO: Implement iterator
-    // Assert.ok(hasElements, "Is values container now non-empty?");
-    // Assert.equal(3, hangGuard, "Do we only have three elements?");
+    Assert.ok(hasElements, "Is values container now non-empty?");
+    Assert.equal(3, hangGuard, "Do we only have three elements?");
 
     {
-      it = root.values();
+      it = await root.values();
       Assert.ok(!!it, "Does root have values iterator?");
-      let elem = await step(it);
-      Assert.ok(!elem, "Iterator not yet implemented");
-      // TODO: Implement iterator
-      // Assert.ok(!!elem, "Is element not non-empty?");
-      // await elem.getDirectoryHandle("Trash", allowCreate);
-      // let subit = elem.values();
-      // let subdir = await step(subit);
-      // Assert.ok(!!subdir, "Is element not non-empty?");
-      // Assert.equal("directory", subdir.kind, "Is found item a directory?");
-      // Assert.equal("Trash", subdir.name, "Is found item a directory?");
+      let elem = await it.next();
+
+      await elem.value.getDirectoryHandle("Trash", allowCreate);
+      let subit = elem.value.values();
+      Assert.ok(!!elem, "Is element not non-empty?");
+      let subdirResult = await subit.next();
+      let subdir = subdirResult.value;
+      Assert.ok(!!subdir, "Is element not non-empty?");
+      Assert.equal("directory", subdir.kind, "Is found item a directory?");
+      Assert.equal("Trash", subdir.name, "Is found item a directory?");
     }
 
     const wipeEverything = { recursive: true };
     for (let dirName of subdirectoryNames) {
-      try {
-        await root.removeEntry(dirName, wipeEverything);
-      } catch (err) {
-        Assert.equal(
-          Cr.NS_ERROR_NOT_IMPLEMENTED,
-          err.result,
-          "Iterator not implemented yet"
-        );
-      }
-      // TODO: Implement iterator
-      // Assert.ok(
-      //   true,
-      //   "Was it possible to remove subdirectory " + dirName + "?"
-      // );
+      await root.removeEntry(dirName, wipeEverything);
+      Assert.ok(
+        true,
+        "Was it possible to remove subdirectory " + dirName + "?"
+      );
     }
   }
 
@@ -109,8 +84,8 @@ exported_symbols.smokeTest = async function smokeTest() {
     let it = root.values();
     Assert.ok(!!it, "Does root have values iterator?");
 
-    let elem = await step(it);
-    Assert.ok(!elem, "Is root directory empty?");
+    let elem = await it.next();
+    Assert.ok(elem.done, "Is root directory empty?");
   }
 };
 

dom/fs/test/gtest/api/TestFileSystemDirectoryHandle.cpp

@@ -33,7 +33,15 @@ class TestFileSystemDirectoryHandle : public ::testing::Test {
     mManager = MakeAndAddRef<FileSystemManager>(mGlobal, nullptr);
   }
 
+  FileSystemDirectoryHandle::iterator_t::WrapFunc GetWrapFunc() const {
+    return [](JSContext*, AsyncIterableIterator<FileSystemDirectoryHandle>*,
+              JS::Handle<JSObject*>,
+              JS::MutableHandle<JSObject*>) -> bool { return true; };
+  }
+
   nsIGlobalObject* mGlobal = GetGlobal();
+  const IterableIteratorBase::IteratorType mIteratorType =
+      IterableIteratorBase::IteratorType::Keys;
   UniquePtr<MockFileSystemRequestHandler> mRequestHandler;
   FileSystemEntryMetadata mMetadata;
   nsString mName;
@@ -47,37 +55,61 @@ TEST_F(TestFileSystemDirectoryHandle, constructDirectoryHandleRefPointer) {
   ASSERT_TRUE(dirHandle);
 }
 
-TEST_F(TestFileSystemDirectoryHandle, areEntriesReturned) {
+TEST_F(TestFileSystemDirectoryHandle, initAndDestroyIterator) {
   RefPtr<FileSystemDirectoryHandle> dirHandle =
       MakeAndAddRef<FileSystemDirectoryHandle>(mGlobal, mManager, mMetadata,
                                                mRequestHandler.release());
 
   ASSERT_TRUE(dirHandle);
 
-  RefPtr<FileSystemDirectoryIterator> entries = dirHandle->Entries();
-  ASSERT_TRUE(entries);
+  RefPtr<FileSystemDirectoryHandle::iterator_t> iterator =
+      new FileSystemDirectoryHandle::iterator_t(dirHandle.get(), mIteratorType,
+                                                GetWrapFunc());
+  IgnoredErrorResult rv;
+  dirHandle->InitAsyncIterator(iterator, rv);
+  ASSERT_TRUE(iterator->GetData());
+
+  dirHandle->DestroyAsyncIterator(iterator);
+  ASSERT_EQ(nullptr, iterator->GetData());
 }
 
-TEST_F(TestFileSystemDirectoryHandle, areKeysReturned) {
+class MockFileSystemDirectoryIteratorImpl final
+    : public FileSystemDirectoryIterator::Impl {
+ public:
+  NS_INLINE_DECL_REFCOUNTING(MockFileSystemDirectoryIteratorImpl)
+
+  MOCK_METHOD(already_AddRefed<Promise>, Next,
+              (nsIGlobalObject * aGlobal, RefPtr<FileSystemManager>& aManager,
+               ErrorResult& aError),
+              (override));
+
+ protected:
+  ~MockFileSystemDirectoryIteratorImpl() = default;
+};
+
+TEST_F(TestFileSystemDirectoryHandle, isNextPromiseReturned) {
   RefPtr<FileSystemDirectoryHandle> dirHandle =
       MakeAndAddRef<FileSystemDirectoryHandle>(mGlobal, mManager, mMetadata,
                                                mRequestHandler.release());
 
   ASSERT_TRUE(dirHandle);
 
-  RefPtr<FileSystemDirectoryIterator> keys = dirHandle->Keys();
-  ASSERT_TRUE(keys);
-}
+  auto* mockIter = new MockFileSystemDirectoryIteratorImpl();
+  IgnoredErrorResult error;
+  EXPECT_CALL(*mockIter, Next(_, _, _))
+      .WillOnce(::testing::Return(Promise::Create(mGlobal, error)));
 
-TEST_F(TestFileSystemDirectoryHandle, areValuesReturned) {
-  RefPtr<FileSystemDirectoryHandle> dirHandle =
-      MakeAndAddRef<FileSystemDirectoryHandle>(mGlobal, mManager, mMetadata,
-                                               mRequestHandler.release());
+  RefPtr<FileSystemDirectoryHandle::iterator_t> iterator =
+      MakeAndAddRef<FileSystemDirectoryHandle::iterator_t>(
+          dirHandle.get(), mIteratorType, GetWrapFunc());
+  iterator->SetData(static_cast<void*>(mockIter));
 
-  ASSERT_TRUE(dirHandle);
+  IgnoredErrorResult rv;
+  RefPtr<Promise> promise =
+      dirHandle->GetNextPromise(nullptr, iterator.get(), rv);
+  ASSERT_TRUE(promise);
 
-  RefPtr<FileSystemDirectoryIterator> values = dirHandle->Values();
-  ASSERT_TRUE(values);
+  dirHandle->DestroyAsyncIterator(iterator.get());
 }
 
 TEST_F(TestFileSystemDirectoryHandle, isHandleKindDirectory) {
@@ -91,6 +123,8 @@ TEST_F(TestFileSystemDirectoryHandle, isHandleKindDirectory) {
 }
 
 TEST_F(TestFileSystemDirectoryHandle, isFileHandleReturned) {
+  EXPECT_CALL(*mRequestHandler, GetFileHandle(_, _, _, _))
+      .WillOnce(::testing::ReturnArg<3>());
   RefPtr<FileSystemDirectoryHandle> dirHandle =
       MakeAndAddRef<FileSystemDirectoryHandle>(mGlobal, mManager, mMetadata,
                                                mRequestHandler.release());
@@ -119,6 +153,8 @@ TEST_F(TestFileSystemDirectoryHandle, doesGetFileHandleFailOnNullGlobal) {
 }
 
 TEST_F(TestFileSystemDirectoryHandle, isDirectoryHandleReturned) {
+  EXPECT_CALL(*mRequestHandler, GetDirectoryHandle(_, _, _, _))
+      .WillOnce(::testing::ReturnArg<3>());
   RefPtr<FileSystemDirectoryHandle> dirHandle =
       MakeAndAddRef<FileSystemDirectoryHandle>(mGlobal, mManager, mMetadata,
                                                mRequestHandler.release());
@@ -147,6 +183,8 @@ TEST_F(TestFileSystemDirectoryHandle, doesGetDirectoryHandleFailOnNullGlobal) {
 }
 
 TEST_F(TestFileSystemDirectoryHandle, isRemoveEntrySuccessful) {
+  EXPECT_CALL(*mRequestHandler, RemoveEntry(_, _, _, _))
+      .WillOnce(::testing::ReturnArg<3>());
   RefPtr<FileSystemDirectoryHandle> dirHandle =
       MakeAndAddRef<FileSystemDirectoryHandle>(mGlobal, mManager, mMetadata,
                                                mRequestHandler.release());

dom/fs/test/gtest/child/TestFileSystemRequestHandler.cpp

@@ -4,12 +4,11 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#include "gtest/gtest.h"
-
+#include "ArrayAppendable.h"
 #include "FileSystemBackgroundRequestHandler.h"
 #include "FileSystemMocks.h"
 #include "fs/FileSystemRequestHandler.h"
-
+#include "gtest/gtest.h"
 #include "mozilla/dom/IPCBlob.h"
 #include "mozilla/dom/FileSystemManager.h"
 #include "mozilla/dom/FileSystemManagerChild.h"
@@ -174,6 +173,14 @@ TEST_F(TestFileSystemRequestHandler, isGetFileSuccessful) {
                      [this]() { return mListener->IsDone(); });
 }
 
+class TestArrayAppendable : public ArrayAppendable {
+ public:
+  MOCK_METHOD(void, append,
+              (nsIGlobalObject * aGlobal, RefPtr<FileSystemManager>& aManager,
+               const nsTArray<FileSystemEntryMetadata>&),
+              (override));
+};
+
 TEST_F(TestFileSystemRequestHandler, isGetEntriesSuccessful) {
   auto fakeResponse = [](const auto& /* aRequest */, auto&& aResolve,
                          auto&& /* aReject */) {
@@ -200,7 +207,9 @@ TEST_F(TestFileSystemRequestHandler, isGetEntriesSuccessful) {
       });
 
   auto testable = GetFileSystemRequestHandler();
-  ArrayAppendable sink;
+  TestArrayAppendable sink;
+  EXPECT_CALL(sink, append(_, _, _)).Times(1);
+
   testable->GetEntries(mManager, mEntry.entryId(), /* page */ 0, promise, sink);
   SpinEventLoopUntil("Promise is fulfilled or timeout"_ns,
                      [listener]() { return listener->IsDone(); });

dom/quota/StorageManager.cpp

@@ -5,6 +5,7 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "StorageManager.h"
+#include "fs/FileSystemRequestHandler.h"
 
 #include <cstdint>
 #include <cstdlib>

dom/webidl/FileSystemDirectoryHandle.webidl

@@ -15,19 +15,15 @@ dictionary FileSystemRemoveOptions {
   boolean recursive = false;
 };
 
-// TODO: Add Serializzable
+// TODO: Add Serializable
 [Exposed=(Window,Worker), SecureContext, Pref="dom.fs.enabled"]
 interface FileSystemDirectoryHandle : FileSystemHandle {
-  // This interface defines an async iterable, however that isn't supported yet
-  // by the bindings. So for now just explicitly define what an async iterable
-  // definition implies.
-  //async iterable<USVString, FileSystemHandle>;
-  FileSystemDirectoryIterator entries();
-  FileSystemDirectoryIterator keys();
-  FileSystemDirectoryIterator values();
+
+  async iterable<USVString, FileSystemHandle>;
 
   [NewObject]
   Promise<FileSystemFileHandle> getFileHandle(USVString name, optional FileSystemGetFileOptions options = {});
+
   [NewObject]
   Promise<FileSystemDirectoryHandle> getDirectoryHandle(USVString name, optional FileSystemGetDirectoryOptions options = {});