refactor: make shell.ShowItemInFolder asynchronous (#17121)

* fix: add scoped_blocking_calls to platform_win

1191582

* feat: make ShowItemInFolder async

* address feedback from review

* fix build

atom/common/platform_util.h

@@ -23,7 +23,7 @@ typedef base::OnceCallback<void(const std::string&)> OpenExternalCallback;
 
 // Show the given file in a file manager. If possible, select the file.
 // Must be called from the UI thread.
-bool ShowItemInFolder(const base::FilePath& full_path);
+void ShowItemInFolder(const base::FilePath& full_path);
 
 // Open the given file in the desktop's default manner.
 // Must be called from the UI thread.

atom/common/platform_util_linux.cc

@@ -69,12 +69,12 @@ namespace platform_util {
 // TODO(estade): It would be nice to be able to select the file in the file
 // manager, but that probably requires extending xdg-open. For now just
 // show the folder.
-bool ShowItemInFolder(const base::FilePath& full_path) {
+void ShowItemInFolder(const base::FilePath& full_path) {
   base::FilePath dir = full_path.DirName();
   if (!base::DirectoryExists(dir))
-    return false;
+    return;
 
-  return XDGOpen(dir.value(), false);
+  XDGOpen(dir.value(), false);
 }
 
 bool OpenItem(const base::FilePath& full_path) {

atom/common/platform_util_mac.mm

@@ -59,7 +59,7 @@ NSString* GetLoginHelperBundleIdentifier() {
 
 namespace platform_util {
 
-bool ShowItemInFolder(const base::FilePath& path) {
+void ShowItemInFolder(const base::FilePath& path) {
   // The API only takes absolute path.
   base::FilePath full_path =
       path.IsAbsolute() ? path : base::MakeAbsoluteFilePath(path);
@@ -69,9 +69,7 @@ bool ShowItemInFolder(const base::FilePath& path) {
   if (!path_string || ![[NSWorkspace sharedWorkspace] selectFile:path_string
                                         inFileViewerRootedAtPath:@""]) {
     LOG(WARNING) << "NSWorkspace failed to select file " << full_path.value();
-    return false;
   }
-  return true;
 }
 
 bool OpenItem(const base::FilePath& full_path) {

atom/common/platform_util_win.cc

@@ -23,10 +23,13 @@
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/task/post_task.h"
+#include "base/threading/scoped_blocking_call.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_co_mem.h"
 #include "base/win/scoped_com_initializer.h"
 #include "base/win/windows_version.h"
+#include "content/public/browser/browser_task_traits.h"
 #include "ui/base/win/shell.h"
 #include "url/gurl.h"
 
@@ -227,31 +230,29 @@ HRESULT DeleteFileProgressSink::ResumeTimer() {
   return S_OK;
 }
 
-}  // namespace
+void ShowItemInFolderOnWorkerThread(const base::FilePath& full_path) {
-
+  base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
-namespace platform_util {
-
-bool ShowItemInFolder(const base::FilePath& full_path) {
   base::win::ScopedCOMInitializer com_initializer;
   if (!com_initializer.Succeeded())
-    return false;
+    return;
 
   base::FilePath dir = full_path.DirName().AsEndingWithSeparator();
   // ParseDisplayName will fail if the directory is "C:", it must be "C:\\".
   if (dir.empty())
-    return false;
+    return;
 
   Microsoft::WRL::ComPtr<IShellFolder> desktop;
   HRESULT hr = SHGetDesktopFolder(desktop.GetAddressOf());
   if (FAILED(hr))
-    return false;
+    return;
 
   base::win::ScopedCoMem<ITEMIDLIST> dir_item;
   hr = desktop->ParseDisplayName(NULL, NULL,
                                  const_cast<wchar_t*>(dir.value().c_str()),
                                  NULL, &dir_item, NULL);
   if (FAILED(hr)) {
-    return ui::win::OpenFolderViaShell(dir);
+    ui::win::OpenFolderViaShell(dir);
+    return;
   }
 
   base::win::ScopedCoMem<ITEMIDLIST> file_item;
@@ -259,35 +260,39 @@ bool ShowItemInFolder(const base::FilePath& full_path) {
       NULL, NULL, const_cast<wchar_t*>(full_path.value().c_str()), NULL,
       &file_item, NULL);
   if (FAILED(hr)) {
-    return ui::win::OpenFolderViaShell(dir);
+    ui::win::OpenFolderViaShell(dir);
+    return;
   }
 
   const ITEMIDLIST* highlight[] = {file_item};
-
   hr = SHOpenFolderAndSelectItems(dir_item, base::size(highlight), highlight,
                                   NULL);
-  if (!FAILED(hr))
+  if (FAILED(hr)) {
-    return true;
+    // On some systems, the above call mysteriously fails with "file not
-
+    // found" even though the file is there.  In these cases, ShellExecute()
-  // On some systems, the above call mysteriously fails with "file not
+    // seems to work as a fallback (although it won't select the file).
-  // found" even though the file is there.  In these cases, ShellExecute()
+    if (hr == ERROR_FILE_NOT_FOUND) {
-  // seems to work as a fallback (although it won't select the file).
+      ShellExecute(NULL, L"open", dir.value().c_str(), NULL, NULL, SW_SHOW);
-  if (hr == ERROR_FILE_NOT_FOUND) {
+    } else {
-    return ui::win::OpenFolderViaShell(dir);
+      LOG(WARNING) << " " << __func__ << "(): Can't open full_path = \""
-  } else {
+                   << full_path.value() << "\""
-    LPTSTR message = NULL;
+                   << " hr = " << logging::SystemErrorCodeToString(hr);
-    FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM,
+      ui::win::OpenFolderViaShell(dir);
-                  0, hr, 0, reinterpret_cast<LPTSTR>(&message), 0, NULL);
+    }
-    LOG(WARNING) << " " << __FUNCTION__ << "(): Can't open full_path = \""
-                 << full_path.value() << "\""
-                 << " hr = " << hr << " " << reinterpret_cast<LPTSTR>(&message);
-    if (message)
-      LocalFree(message);
-
-    return ui::win::OpenFolderViaShell(dir);
   }
 }
 
+}  // namespace
+
+namespace platform_util {
+
+void ShowItemInFolder(const base::FilePath& full_path) {
+  base::CreateCOMSTATaskRunnerWithTraits(
+      {base::MayBlock(), base::TaskPriority::USER_BLOCKING})
+      ->PostTask(FROM_HERE,
+                 base::BindOnce(&ShowItemInFolderOnWorkerThread, full_path));
+}
+
 bool OpenItem(const base::FilePath& full_path) {
   if (base::DirectoryExists(full_path))
     return ui::win::OpenFolderViaShell(full_path);

docs/api/shell.md

@@ -22,8 +22,6 @@ The `shell` module has the following methods:
 
 * `fullPath` String
 
-Returns `Boolean` - Whether the item was successfully shown.
-
 Show the given file in a file manager. If possible, select the file.
 
 ### `shell.openItem(fullPath)`