[NativeFS] Remove some no longer needed thread hopping.

Now all code runs on the UI thread we no longer need to do thread hopping
in these places.

Bug: 1011534
Change-Id: I17107d20963f7f7d900294b13576c628fa24faa9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1860313
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Auto-Submit: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Olivier Yiptong <oyiptong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710161}

content/browser/native_file_system/file_system_chooser.cc

@@ -115,14 +115,11 @@ FileSystemChooser::Options::Options(
       file_types_(ConvertAcceptsToFileTypeInfo(accepts, include_accepts_all)) {}
 
 // static
-void FileSystemChooser::CreateAndShow(
-    WebContents* web_contents,
-    const Options& options,
-    ResultCallback callback,
-    scoped_refptr<base::TaskRunner> callback_runner) {
+void FileSystemChooser::CreateAndShow(WebContents* web_contents,
+                                      const Options& options,
+                                      ResultCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  auto* listener = new FileSystemChooser(options.type(), std::move(callback),
-                                         std::move(callback_runner));
+  auto* listener = new FileSystemChooser(options.type(), std::move(callback));
   listener->dialog_ = ui::SelectFileDialog::Create(
       listener,
       GetContentClient()->browser()->CreateSelectFilePolicy(web_contents));
@@ -157,11 +154,8 @@ void FileSystemChooser::CreateAndShow(
 
 FileSystemChooser::FileSystemChooser(
     blink::mojom::ChooseFileSystemEntryType type,
-    ResultCallback callback,
-    scoped_refptr<base::TaskRunner> callback_runner)
-    : callback_(std::move(callback)),
-      callback_runner_(std::move(callback_runner)),
-      type_(type) {}
+    ResultCallback callback)
+    : callback_(std::move(callback)), type_(type) {}
 
 FileSystemChooser::~FileSystemChooser() {
   if (dialog_)
@@ -181,22 +175,16 @@ void FileSystemChooser::MultiFilesSelected(
   DCHECK(isolated_context);
 
   RecordFileSelectionResult(type_, files.size());
-  callback_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(std::move(callback_), native_file_system_error::Ok(),
-                     std::move(files)));
+  std::move(callback_).Run(native_file_system_error::Ok(), std::move(files));
   delete this;
 }
 
 void FileSystemChooser::FileSelectionCanceled(void* params) {
   RecordFileSelectionResult(type_, 0);
-  callback_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(
-          std::move(callback_),
-          native_file_system_error::FromStatus(
-              blink::mojom::NativeFileSystemStatus::kOperationAborted),
-          std::vector<base::FilePath>()));
+  std::move(callback_).Run(
+      native_file_system_error::FromStatus(
+          blink::mojom::NativeFileSystemStatus::kOperationAborted),
+      std::vector<base::FilePath>());
   delete this;
 }
 

content/browser/native_file_system/file_system_chooser.h

@@ -49,12 +49,10 @@ class CONTENT_EXPORT FileSystemChooser : public ui::SelectFileDialog::Listener {
 
   static void CreateAndShow(WebContents* web_contents,
                             const Options& options,
-                            ResultCallback callback,
-                            scoped_refptr<base::TaskRunner> callback_runner);
+                            ResultCallback callback);
 
   FileSystemChooser(blink::mojom::ChooseFileSystemEntryType type,
-                    ResultCallback callback,
-                    scoped_refptr<base::TaskRunner> callback_runner);
+                    ResultCallback callback);
 
  private:
   ~FileSystemChooser() override;
@@ -68,7 +66,6 @@ class CONTENT_EXPORT FileSystemChooser : public ui::SelectFileDialog::Listener {
   void FileSelectionCanceled(void* params) override;
 
   ResultCallback callback_;
-  scoped_refptr<base::TaskRunner> callback_runner_;
   blink::mojom::ChooseFileSystemEntryType type_;
 
   scoped_refptr<ui::SelectFileDialog> dialog_;

content/browser/native_file_system/file_system_chooser_unittest.cc

@@ -34,8 +34,7 @@ class FileSystemChooserTest : public testing::Test {
             std::move(accepts), include_accepts_all),
         base::BindLambdaForTesting(
             [&](blink::mojom::NativeFileSystemErrorPtr,
-                std::vector<base::FilePath>) { loop.Quit(); }),
-        base::SequencedTaskRunnerHandle::Get());
+                std::vector<base::FilePath>) { loop.Quit(); }));
     loop.Run();
   }
 

content/browser/native_file_system/native_file_system_manager_impl.cc

@@ -44,21 +44,16 @@ namespace {
 void ShowFilePickerOnUIThread(const url::Origin& requesting_origin,
                               int render_process_id,
                               int frame_id,
-                              bool require_user_gesture,
                               const FileSystemChooser::Options& options,
-                              FileSystemChooser::ResultCallback callback,
-                              scoped_refptr<base::TaskRunner> callback_runner) {
+                              FileSystemChooser::ResultCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RenderFrameHost* rfh = RenderFrameHost::FromID(render_process_id, frame_id);
   WebContents* web_contents = WebContents::FromRenderFrameHost(rfh);
 
   if (!web_contents) {
-    callback_runner->PostTask(
-        FROM_HERE,
-        base::BindOnce(std::move(callback),
-                       native_file_system_error::FromStatus(
-                           NativeFileSystemStatus::kOperationAborted),
-                       std::vector<base::FilePath>()));
+    std::move(callback).Run(native_file_system_error::FromStatus(
+                                NativeFileSystemStatus::kOperationAborted),
+                            std::vector<base::FilePath>());
     return;
   }
 
@@ -66,48 +61,18 @@ void ShowFilePickerOnUIThread(const url::Origin& requesting_origin,
       url::Origin::Create(web_contents->GetLastCommittedURL());
   if (embedding_origin != requesting_origin) {
     // Third party iframes are not allowed to show a file picker.
-    callback_runner->PostTask(
-        FROM_HERE,
-        base::BindOnce(
-            std::move(callback),
-            native_file_system_error::FromStatus(
-                NativeFileSystemStatus::kPermissionDenied,
-                "Third party iframes are not allowed to show a file picker."),
-            std::vector<base::FilePath>()));
-    return;
-  }
-
-  // Renderer process should already check for user activation before sending
-  // IPC, but just to be sure double check here as well. This is not treated
-  // as a BadMessage because it is possible for the transient user activation
-  // to expire between the renderer side check and this check.
-  if (require_user_gesture && !rfh->HasTransientUserActivation()) {
-    callback_runner->PostTask(
-        FROM_HERE,
-        base::BindOnce(
-            std::move(callback),
-            native_file_system_error::FromStatus(
-                NativeFileSystemStatus::kPermissionDenied,
-                "User activation is required to show a file picker."),
-            std::vector<base::FilePath>()));
+    std::move(callback).Run(
+        native_file_system_error::FromStatus(
+            NativeFileSystemStatus::kPermissionDenied,
+            "Third party iframes are not allowed to show a file picker."),
+        std::vector<base::FilePath>());
     return;
   }
 
   // Drop fullscreen mode so that the user sees the URL bar.
   web_contents->ForSecurityDropFullscreen();
 
-  FileSystemChooser::CreateAndShow(web_contents, options, std::move(callback),
-                                   std::move(callback_runner));
-}
-
-bool HasTransientUserActivation(int render_process_id, int frame_id) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  RenderFrameHost* rfh = RenderFrameHost::FromID(render_process_id, frame_id);
-
-  if (!rfh)
-    return false;
-
-  return rfh->HasTransientUserActivation();
+  FileSystemChooser::CreateAndShow(web_contents, options, std::move(callback));
 }
 
 bool CreateOrTruncateFile(const base::FilePath& path) {
@@ -238,17 +203,36 @@ void NativeFileSystemManagerImpl::ChooseEntries(
     return;
   }
 
+  RenderFrameHost* rfh =
+      RenderFrameHost::FromID(context.process_id, context.frame_id);
+  if (!rfh) {
+    std::move(callback).Run(
+        native_file_system_error::FromStatus(
+            NativeFileSystemStatus::kOperationAborted),
+        std::vector<blink::mojom::NativeFileSystemEntryPtr>());
+    return;
+  }
+
+  // Renderer process should already check for user activation before sending
+  // IPC, but just to be sure double check here as well. This is not treated
+  // as a BadMessage because it is possible for the transient user activation
+  // to expire between the renderer side check and this check.
+  if (!rfh->HasTransientUserActivation()) {
+    std::move(callback).Run(
+        native_file_system_error::FromStatus(
+            NativeFileSystemStatus::kPermissionDenied,
+            "User activation is required to show a file picker."),
+        std::vector<blink::mojom::NativeFileSystemEntryPtr>());
+    return;
+  }
+
   FileSystemChooser::Options options(type, std::move(accepts),
                                      include_accepts_all);
-  base::PostTask(
-      FROM_HERE, {BrowserThread::UI},
-      base::BindOnce(
-          &ShowFilePickerOnUIThread, context.origin, context.process_id,
-          context.frame_id, /*require_user_gesture=*/true, options,
-          base::BindOnce(&NativeFileSystemManagerImpl::DidChooseEntries,
-                         weak_factory_.GetWeakPtr(), context, options,
-                         std::move(callback)),
-          base::SequencedTaskRunnerHandle::Get()));
+  ShowFilePickerOnUIThread(
+      context.origin, context.process_id, context.frame_id, options,
+      base::BindOnce(&NativeFileSystemManagerImpl::DidChooseEntries,
+                     weak_factory_.GetWeakPtr(), context, options,
+                     std::move(callback)));
 }
 
 void NativeFileSystemManagerImpl::GetFileHandleFromToken(
@@ -384,16 +368,14 @@ NativeFileSystemManagerImpl::CreateFileWriter(
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   mojo::PendingRemote<blink::mojom::NativeFileSystemFileWriter> result;
-  mojo::PendingReceiver<blink::mojom::NativeFileSystemFileWriter>
-      writer_receiver = result.InitWithNewPipeAndPassReceiver();
-
-  base::PostTaskAndReplyWithResult(
-      FROM_HERE, {BrowserThread::UI},
-      base::BindOnce(&HasTransientUserActivation, binding_context.process_id,
-                     binding_context.frame_id),
-      base::BindOnce(&NativeFileSystemManagerImpl::CreateFileWriterImpl,
-                     weak_factory_.GetWeakPtr(), binding_context, url, swap_url,
-                     handle_state, std::move(writer_receiver)));
+
+  RenderFrameHost* rfh = RenderFrameHost::FromID(binding_context.process_id,
+                                                 binding_context.frame_id);
+  bool has_transient_user_activation = rfh && rfh->HasTransientUserActivation();
+  writer_receivers_.Add(std::make_unique<NativeFileSystemFileWriterImpl>(
+                            this, binding_context, url, swap_url, handle_state,
+                            has_transient_user_activation),
+                        result.InitWithNewPipeAndPassReceiver());
   return result;
 }
 
@@ -566,16 +548,12 @@ void NativeFileSystemManagerImpl::DidVerifySensitiveDirectoryAccess(
     return;
   }
   if (result == SensitiveDirectoryResult::kTryAgain) {
-    base::PostTask(
-        FROM_HERE, {BrowserThread::UI},
-        base::BindOnce(
-            &ShowFilePickerOnUIThread, binding_context.origin,
-            binding_context.process_id, binding_context.frame_id,
-            /*require_user_gesture=*/false, options,
-            base::BindOnce(&NativeFileSystemManagerImpl::DidChooseEntries,
-                           weak_factory_.GetWeakPtr(), binding_context, options,
-                           std::move(callback)),
-            base::SequencedTaskRunnerHandle::Get()));
+    ShowFilePickerOnUIThread(
+        binding_context.origin, binding_context.process_id,
+        binding_context.frame_id, options,
+        base::BindOnce(&NativeFileSystemManagerImpl::DidChooseEntries,
+                       weak_factory_.GetWeakPtr(), binding_context, options,
+                       std::move(callback)));
     return;
   }
 
@@ -764,18 +742,4 @@ NativeFileSystemManagerImpl::CreateFileEntryFromPathImpl(
       url.base_name);
 }
 
-void NativeFileSystemManagerImpl::CreateFileWriterImpl(
-    const BindingContext& binding_context,
-    const storage::FileSystemURL& url,
-    const storage::FileSystemURL& swap_url,
-    const SharedHandleState& handle_state,
-    mojo::PendingReceiver<blink::mojom::NativeFileSystemFileWriter>
-        writer_receiver,
-    bool has_transient_user_activation) {
-  writer_receivers_.Add(std::make_unique<NativeFileSystemFileWriterImpl>(
-                            this, binding_context, url, swap_url, handle_state,
-                            has_transient_user_activation),
-                        std::move(writer_receiver));
-}
-
 }  // namespace content

content/browser/native_file_system/native_file_system_manager_impl.h

@@ -248,15 +248,6 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl
       const base::FilePath& file_path,
       NativeFileSystemPermissionContext::UserAction user_action);
 
-  void CreateFileWriterImpl(
-      const BindingContext& binding_context,
-      const storage::FileSystemURL& url,
-      const storage::FileSystemURL& swap_url,
-      const SharedHandleState& handle_state,
-      mojo::PendingReceiver<blink::mojom::NativeFileSystemFileWriter>
-          writer_receiver,
-      bool has_transient_user_activation);
-
   SEQUENCE_CHECKER(sequence_checker_);
 
   const scoped_refptr<storage::FileSystemContext> context_;