fix: shell.trashItem crash when called in renderer (#28788)

* fix: shell.trashItem crash when called in renderer * Update api-shell-spec.ts * Update spec-main/api-shell-spec.ts Co-authored-by: Jeremy Rose <nornagon@nornagon.net> Co-authored-by: Jeremy Rose <jeremya@chromium.org>
electron · Apr 26, 2021 · eca4a5c · eca4a5c
1 parent 56fc2bc
commit eca4a5c
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 11 deletions.
diff --git a/shell/common/platform_util.cc b/shell/common/platform_util.cc
@@ -14,24 +14,32 @@
 
 namespace platform_util {
 
-void TrashItemOnBlockingThread(
-    const base::FilePath& full_path,
-    base::OnceCallback<void(bool, const std::string&)> callback) {
+struct TrashItemResult {
+  bool success;
+  std::string error;
+};
+
+TrashItemResult TrashItemOnBlockingThread(const base::FilePath& full_path) {
   std::string error;
   bool success = internal::PlatformTrashItem(full_path, &error);
-  content::GetUIThreadTaskRunner({})->PostTask(
-      FROM_HERE, base::BindOnce(std::move(callback), success, error));
+  return {success, error};
 }
 
 void TrashItem(const base::FilePath& full_path,
                base::OnceCallback<void(bool, const std::string&)> callback) {
   // XXX: is continue_on_shutdown right?
-  base::ThreadPool::PostTask(FROM_HERE,
-                             {base::MayBlock(), base::WithBaseSyncPrimitives(),
-                              base::TaskPriority::USER_BLOCKING,
-                              base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
-                             base::BindOnce(&TrashItemOnBlockingThread,
-                                            full_path, std::move(callback)));
+  base::ThreadPool::PostTaskAndReplyWithResult(
+      FROM_HERE,
+      {base::MayBlock(), base::WithBaseSyncPrimitives(),
+       base::TaskPriority::USER_BLOCKING,
+       base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
+      base::BindOnce(&TrashItemOnBlockingThread, full_path),
+      base::BindOnce(
+          [](base::OnceCallback<void(bool, const std::string&)> callback,
+             TrashItemResult result) {
+            std::move(callback).Run(result.success, result.error);
+          },
+          std::move(callback)));
 }
 
 }  // namespace platform_util
diff --git a/spec-main/api-shell-spec.ts b/spec-main/api-shell-spec.ts
@@ -62,6 +62,8 @@ describe('shell module', () => {
   });
 
   describe('shell.trashItem()', () => {
+    afterEach(closeAllWindows);
+
     it('moves an item to the trash', async () => {
       const dir = await fs.mkdtemp(path.resolve(app.getPath('temp'), 'electron-shell-spec-'));
       const filename = path.join(dir, 'temp-to-be-deleted');
@@ -74,5 +76,11 @@ describe('shell module', () => {
       const filename = path.join(app.getPath('temp'), 'does-not-exist');
       await expect(shell.trashItem(filename)).to.eventually.be.rejected();
     });
+
+    it('works in the renderer process', async () => {
+      const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true, contextIsolation: false } });
+      w.loadURL('about:blank');
+      await expect(w.webContents.executeJavaScript('require(\'electron\').shell.trashItem(\'does-not-exist\')')).to.be.rejectedWith(/does-not-exist|Failed to move item|Failed to create FileOperation/);
+    });
   });
 });