[Native File System] Change GetEntries to be asynchronous

This changes NativeFileSystemDirectoryHandle.GetEntries to be
asynchronous like the web-exposed API and the storage implementation.
This should make directory iteration more efficient.

Bug: 970059
Change-Id: I43bdd4950a70abc8d7cd8dcc890963655d49fdcd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1867150Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Auto-Submit: Eero Häkkinen <eero.hakkinen@intel.com>
Commit-Queue: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708885}

content/browser/native_file_system/native_file_system_directory_handle_impl.cc

@@ -49,11 +49,6 @@ bool IsCurrentOrParentDirectory(const std::string& name) {
 
 }  // namespace
 
-struct NativeFileSystemDirectoryHandleImpl::ReadDirectoryState {
-  GetEntriesCallback callback;
-  std::vector<NativeFileSystemEntryPtr> entries;
-};
-
 NativeFileSystemDirectoryHandleImpl::NativeFileSystemDirectoryHandleImpl(
     NativeFileSystemManagerImpl* manager,
     const BindingContext& context,
@@ -174,15 +169,20 @@ void NativeFileSystemDirectoryHandleImpl::GetDirectory(
 }
 
 void NativeFileSystemDirectoryHandleImpl::GetEntries(
-    GetEntriesCallback callback) {
+    mojo::PendingRemote<blink::mojom::NativeFileSystemDirectoryEntriesListener>
+        pending_listener) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  auto listener = std::make_unique<
+      mojo::Remote<blink::mojom::NativeFileSystemDirectoryEntriesListener>>(
+      std::move(pending_listener));
+  listener->reset_on_disconnect();
+
   DoFileSystemOperation(
       FROM_HERE, &FileSystemOperationRunner::ReadDirectory,
       base::BindRepeating(
           &NativeFileSystemDirectoryHandleImpl::DidReadDirectory,
-          weak_factory_.GetWeakPtr(),
-          base::Owned(new ReadDirectoryState{std::move(callback)})),
+          weak_factory_.GetWeakPtr(), base::Owned(std::move(listener))),
       url());
 }
 
@@ -284,19 +284,24 @@ void NativeFileSystemDirectoryHandleImpl::DidGetDirectory(
 }
 
 void NativeFileSystemDirectoryHandleImpl::DidReadDirectory(
-    ReadDirectoryState* state,
+    mojo::Remote<blink::mojom::NativeFileSystemDirectoryEntriesListener>*
+        listener,
     base::File::Error result,
     std::vector<filesystem::mojom::DirectoryEntry> file_list,
-    bool has_more) {
+    bool has_more_entries) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  if (!*listener)
+    return;
+
   if (result != base::File::FILE_OK) {
-    DCHECK(!has_more);
-    std::move(state->callback)
-        .Run(native_file_system_error::FromFileError(result), {});
+    DCHECK(!has_more_entries);
+    (*listener)->DidReadDirectory(
+        native_file_system_error::FromFileError(result), {}, false);
     return;
   }
 
+  std::vector<NativeFileSystemEntryPtr> entries;
   for (const auto& entry : file_list) {
     std::string basename = storage::FilePathToString(entry.name);
 
@@ -308,17 +313,12 @@ void NativeFileSystemDirectoryHandleImpl::DidReadDirectory(
     // |basename|.
     CHECK_EQ(get_child_url_result->status, NativeFileSystemStatus::kOk);
 
-    state->entries.push_back(
+    entries.push_back(
         CreateEntry(basename, child_url,
                     entry.type == filesystem::mojom::FsFileType::DIRECTORY));
   }
-
-  // TODO(mek): Change API so we can stream back entries as they come in, rather
-  // than waiting till we have retrieved them all.
-  if (!has_more) {
-    std::move(state->callback)
-        .Run(native_file_system_error::Ok(), std::move(state->entries));
-  }
+  (*listener)->DidReadDirectory(native_file_system_error::Ok(),
+                                std::move(entries), has_more_entries);
 }
 
 void NativeFileSystemDirectoryHandleImpl::RemoveEntryImpl(

content/browser/native_file_system/native_file_system_directory_handle_impl.h

@@ -43,7 +43,9 @@ class NativeFileSystemDirectoryHandleImpl
   void GetDirectory(const std::string& basename,
                     bool create,
                     GetDirectoryCallback callback) override;
-  void GetEntries(GetEntriesCallback callback) override;
+  void GetEntries(mojo::PendingRemote<
+                  blink::mojom::NativeFileSystemDirectoryEntriesListener>
+                      pending_listener) override;
   void RemoveEntry(const std::string& basename,
                    bool recurse,
                    RemoveEntryCallback callback) override;
@@ -52,9 +54,6 @@ class NativeFileSystemDirectoryHandleImpl
       override;
 
  private:
-  // State that is kept for the duration of a GetEntries/ReadDirectory call.
-  struct ReadDirectoryState;
-
   // This method creates the file if it does not currently exists. I.e. it is
   // the implementation for passing create=true to GetFile.
   void GetFileWithWritePermission(const storage::FileSystemURL& child_url,
@@ -70,10 +69,11 @@ class NativeFileSystemDirectoryHandleImpl
                        GetDirectoryCallback callback,
                        base::File::Error result);
   void DidReadDirectory(
-      ReadDirectoryState* state,
+      mojo::Remote<blink::mojom::NativeFileSystemDirectoryEntriesListener>*
+          listener,
       base::File::Error result,
       std::vector<filesystem::mojom::DirectoryEntry> file_list,
-      bool has_more);
+      bool has_more_entries);
 
   void RemoveEntryImpl(const storage::FileSystemURL& url,
                        bool recurse,

third_party/blink/public/mojom/native_file_system/native_file_system_directory_handle.mojom

@@ -21,6 +21,15 @@ struct NativeFileSystemEntry {
   string name;
 };
 
+interface NativeFileSystemDirectoryEntriesListener {
+  // Called by NativeFileSystemDirectoryHandle.GetEntries when some entries
+  // have been obtained. |has_more_entries| is false when all the entries have
+  // been obtained, and  indicates that the callback will not be called again.
+  DidReadDirectory(NativeFileSystemError result,
+                   array<NativeFileSystemEntry> entries,
+                   bool has_more_entries);
+};
+
 // This interface represents a handle to a directory in the Native File System
 // API.
 //
@@ -55,9 +64,7 @@ interface NativeFileSystemDirectoryHandle {
        pending_remote<NativeFileSystemDirectoryHandle>? directory);
 
   // Returns all the direct children of this directory.
-  // TODO(mek): Change this API to stream results in chunks rather than block
-  // until all entries have been retrieved.
-  GetEntries() => (NativeFileSystemError result, array<NativeFileSystemEntry> entries);
+  GetEntries(pending_remote<NativeFileSystemDirectoryEntriesListener> listener);
 
   // Deletes an entry which is a child of this directory.
   // To delete recursively, set |recurse| to true.

third_party/blink/renderer/modules/native_file_system/native_file_system_directory_iterator.cc

@@ -19,9 +19,7 @@ NativeFileSystemDirectoryIterator::NativeFileSystemDirectoryIterator(
     NativeFileSystemDirectoryHandle* directory,
     ExecutionContext* execution_context)
     : ContextLifecycleObserver(execution_context), directory_(directory) {
-  directory_->MojoHandle()->GetEntries(
-      WTF::Bind(&NativeFileSystemDirectoryIterator::OnGotEntries,
-                WrapWeakPersistent(this)));
+  directory_->MojoHandle()->GetEntries(receiver_.BindNewPipeAndPassRemote());
 }
 
 ScriptPromise NativeFileSystemDirectoryIterator::next(
@@ -60,9 +58,10 @@ void NativeFileSystemDirectoryIterator::Trace(Visitor* visitor) {
   visitor->Trace(directory_);
 }
 
-void NativeFileSystemDirectoryIterator::OnGotEntries(
+void NativeFileSystemDirectoryIterator::DidReadDirectory(
     mojom::blink::NativeFileSystemErrorPtr result,
-    Vector<mojom::blink::NativeFileSystemEntryPtr> entries) {
+    Vector<mojom::blink::NativeFileSystemEntryPtr> entries,
+    bool has_more_entries) {
   if (!GetExecutionContext())
     return;
   if (result->status != mojom::blink::NativeFileSystemStatus::kOk) {
@@ -77,7 +76,7 @@ void NativeFileSystemDirectoryIterator::OnGotEntries(
     entries_.push_back(NativeFileSystemHandle::CreateFromMojoEntry(
         std::move(e), GetExecutionContext()));
   }
-  waiting_for_more_entries_ = false;
+  waiting_for_more_entries_ = has_more_entries;
   if (pending_next_) {
     ScriptState::Scope scope(pending_next_->GetScriptState());
     pending_next_->Resolve(
@@ -86,4 +85,8 @@ void NativeFileSystemDirectoryIterator::OnGotEntries(
   }
 }
 
+void NativeFileSystemDirectoryIterator::Dispose() {
+  receiver_.reset();
+}
+
 }  // namespace blink

third_party/blink/renderer/modules/native_file_system/native_file_system_directory_iterator.h

@@ -6,7 +6,8 @@
 #define THIRD_PARTY_BLINK_RENDERER_MODULES_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_DIRECTORY_ITERATOR_H_
 
 #include "base/files/file.h"
-#include "third_party/blink/public/mojom/native_file_system/native_file_system_directory_handle.mojom-blink-forward.h"
+#include "mojo/public/cpp/bindings/receiver.h"
+#include "third_party/blink/public/mojom/native_file_system/native_file_system_directory_handle.mojom-blink.h"
 #include "third_party/blink/public/mojom/native_file_system/native_file_system_error.mojom-blink.h"
 #include "third_party/blink/renderer/core/execution_context/context_lifecycle_observer.h"
 #include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
@@ -20,9 +21,11 @@ class ScriptState;
 
 class NativeFileSystemDirectoryIterator final
     : public ScriptWrappable,
-      public ContextLifecycleObserver {
+      public ContextLifecycleObserver,
+      public mojom::blink::NativeFileSystemDirectoryEntriesListener {
   DEFINE_WRAPPERTYPEINFO();
   USING_GARBAGE_COLLECTED_MIXIN(NativeFileSystemDirectoryIterator);
+  USING_PRE_FINALIZER(NativeFileSystemDirectoryIterator, Dispose);
 
  public:
   NativeFileSystemDirectoryIterator(NativeFileSystemDirectoryHandle* directory,
@@ -33,14 +36,18 @@ class NativeFileSystemDirectoryIterator final
   void Trace(Visitor*) override;
 
  private:
-  void OnGotEntries(mojom::blink::NativeFileSystemErrorPtr result,
-                    Vector<mojom::blink::NativeFileSystemEntryPtr> entries);
+  void DidReadDirectory(mojom::blink::NativeFileSystemErrorPtr result,
+                        Vector<mojom::blink::NativeFileSystemEntryPtr> entries,
+                        bool has_more_entries) override;
+  void Dispose();
 
   mojom::blink::NativeFileSystemErrorPtr error_;
   bool waiting_for_more_entries_ = true;
   HeapDeque<Member<NativeFileSystemHandle>> entries_;
   Member<ScriptPromiseResolver> pending_next_;
   Member<NativeFileSystemDirectoryHandle> directory_;
+  mojo::Receiver<mojom::blink::NativeFileSystemDirectoryEntriesListener>
+      receiver_{this};
 };
 
 }  // namespace blink