Fixing MoveItemToTrash behavior in Windows

Modifying MoveItemToTrash for Windows to actually verify file can be
moved to a drive's recycle bin before attempting to delete it.

PR: #3337.

diff --git a/atom/common/platform_util_win.cc b/atom/common/platform_util_win.cc
index 09ac5ac..2ef8a05 100644 (file)
--- a/atom/common/platform_util_win.cc
+++ b/atom/common/platform_util_win.cc
@@ -5,7 +5,9 @@
 #include "atom/common/platform_util.h"
 
 #include <windows.h>
+#include <atlbase.h>
 #include <commdlg.h>
+#include <comdef.h>
 #include <dwmapi.h>
 #include <shellapi.h>
 #include <shlobj.h>
@@ -19,6 +21,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_co_mem.h"
+#include "base/win/scoped_com_initializer.h"
 #include "base/win/scoped_comptr.h"
 #include "base/win/windows_version.h"
 #include "url/gurl.h"
@@ -42,6 +45,159 @@ bool ValidateShellCommandForScheme(const std::string& scheme) {
   return true;
 }
 
+// Required COM implementation of IFileOperationProgressSink so we can
+// precheck files before deletion to make sure they can be move to the
+// Recycle Bin.
+class DeleteFileProgressSink : public IFileOperationProgressSink {
+ public:
+  DeleteFileProgressSink();
+
+ private:
+  ULONG STDMETHODCALLTYPE AddRef(void);
+  ULONG STDMETHODCALLTYPE Release(void);
+  HRESULT STDMETHODCALLTYPE QueryInterface(REFIID riid, LPVOID* ppvObj);
+  HRESULT STDMETHODCALLTYPE StartOperations(void);
+  HRESULT STDMETHODCALLTYPE FinishOperations(HRESULT);
+  HRESULT STDMETHODCALLTYPE PreRenameItem(
+      DWORD, IShellItem*, LPCWSTR);
+  HRESULT STDMETHODCALLTYPE PostRenameItem(
+      DWORD, IShellItem*, LPCWSTR, HRESULT, IShellItem*);
+  HRESULT STDMETHODCALLTYPE PreMoveItem(
+      DWORD, IShellItem*, IShellItem*, LPCWSTR);
+  HRESULT STDMETHODCALLTYPE PostMoveItem(
+      DWORD, IShellItem*, IShellItem*, LPCWSTR, HRESULT, IShellItem*);
+  HRESULT STDMETHODCALLTYPE PreCopyItem(
+      DWORD, IShellItem*, IShellItem*, LPCWSTR);
+  HRESULT STDMETHODCALLTYPE PostCopyItem(
+      DWORD, IShellItem*, IShellItem*, LPCWSTR, HRESULT, IShellItem*);
+  HRESULT STDMETHODCALLTYPE PreDeleteItem(DWORD, IShellItem*);
+  HRESULT STDMETHODCALLTYPE PostDeleteItem(
+      DWORD, IShellItem*, HRESULT, IShellItem*);
+  HRESULT STDMETHODCALLTYPE PreNewItem(
+      DWORD, IShellItem*, LPCWSTR);
+  HRESULT STDMETHODCALLTYPE PostNewItem(
+      DWORD, IShellItem*, LPCWSTR, LPCWSTR, DWORD, HRESULT, IShellItem*);
+  HRESULT STDMETHODCALLTYPE UpdateProgress(UINT, UINT);
+  HRESULT STDMETHODCALLTYPE ResetTimer(void);
+  HRESULT STDMETHODCALLTYPE PauseTimer(void);
+  HRESULT STDMETHODCALLTYPE ResumeTimer(void);
+
+  ULONG m_cRef;
+};
+
+DeleteFileProgressSink::DeleteFileProgressSink() {
+  m_cRef = 0;
+}
+
+HRESULT DeleteFileProgressSink::PreDeleteItem(DWORD dwFlags, IShellItem*) {
+  if (!(dwFlags & TSF_DELETE_RECYCLE_IF_POSSIBLE)) {
+    // TSF_DELETE_RECYCLE_IF_POSSIBLE will not be set for items that cannot be
+    // recycled.  In this case, we abort the delete operation.  This bubbles
+    // up and stops the Delete in IFileOperation.
+    return E_ABORT;
+  }
+  // Returns S_OK if successful, or an error value otherwise. In the case of an
+  // error value, the delete operation and all subsequent operations pending
+  // from the call to IFileOperation are canceled.
+  return S_OK;
+}
+
+HRESULT DeleteFileProgressSink::QueryInterface(REFIID riid, LPVOID* ppvObj) {
+  // Always set out parameter to NULL, validating it first.
+  if (!ppvObj)
+    return E_INVALIDARG;
+  *ppvObj = nullptr;
+  if (riid == IID_IUnknown || riid == IID_IFileOperationProgressSink) {
+    // Increment the reference count and return the pointer.
+    *ppvObj = reinterpret_cast<IUnknown*>(this);
+    AddRef();
+    return NOERROR;
+  }
+  return E_NOINTERFACE;
+}
+
+ULONG DeleteFileProgressSink::AddRef() {
+  InterlockedIncrement(&m_cRef);
+  return m_cRef;
+}
+
+ULONG DeleteFileProgressSink::Release() {
+  // Decrement the object's internal counter.
+  ULONG ulRefCount = InterlockedDecrement(&m_cRef);
+  if (0 == m_cRef) {
+    delete this;
+  }
+  return ulRefCount;
+}
+
+HRESULT DeleteFileProgressSink::StartOperations() {
+  return S_OK;
+}
+
+HRESULT DeleteFileProgressSink::FinishOperations(HRESULT) {
+  return S_OK;
+}
+
+HRESULT DeleteFileProgressSink::PreRenameItem(DWORD, IShellItem*, LPCWSTR) {
+  return S_OK;
+}
+
+HRESULT DeleteFileProgressSink::PostRenameItem(
+    DWORD, IShellItem*, __RPC__in_string LPCWSTR, HRESULT, IShellItem*) {
+  return E_NOTIMPL;
+}
+
+HRESULT DeleteFileProgressSink::PreMoveItem(
+    DWORD, IShellItem*, IShellItem*, LPCWSTR) {
+  return E_NOTIMPL;
+}
+
+HRESULT DeleteFileProgressSink::PostMoveItem(
+    DWORD, IShellItem*, IShellItem*, LPCWSTR, HRESULT, IShellItem*) {
+  return E_NOTIMPL;
+}
+
+HRESULT DeleteFileProgressSink::PreCopyItem(
+    DWORD, IShellItem*, IShellItem*, LPCWSTR) {
+  return E_NOTIMPL;
+}
+
+HRESULT DeleteFileProgressSink::PostCopyItem(
+    DWORD, IShellItem*, IShellItem*, LPCWSTR, HRESULT, IShellItem*) {
+  return E_NOTIMPL;
+}
+
+HRESULT DeleteFileProgressSink::PostDeleteItem(
+    DWORD, IShellItem*, HRESULT, IShellItem*) {
+  return S_OK;
+}
+
+HRESULT DeleteFileProgressSink::PreNewItem(
+    DWORD dwFlags, IShellItem*, LPCWSTR) {
+  return E_NOTIMPL;
+}
+
+HRESULT DeleteFileProgressSink::PostNewItem(
+    DWORD, IShellItem*, LPCWSTR, LPCWSTR, DWORD, HRESULT, IShellItem*) {
+  return E_NOTIMPL;
+}
+
+HRESULT DeleteFileProgressSink::UpdateProgress(UINT, UINT) {
+  return S_OK;
+}
+
+HRESULT DeleteFileProgressSink::ResetTimer() {
+  return S_OK;
+}
+
+HRESULT DeleteFileProgressSink::PauseTimer() {
+  return S_OK;
+}
+
+HRESULT DeleteFileProgressSink::ResumeTimer() {
+  return S_OK;
+}
+
 }  // namespace
 
 namespace platform_util {
@@ -170,32 +326,40 @@ bool OpenExternal(const GURL& url) {
 }
 
 bool MoveItemToTrash(const base::FilePath& path) {
-  // SHFILEOPSTRUCT wants the path to be terminated with two NULLs,
-  // so we have to use wcscpy because wcscpy_s writes non-NULLs
-  // into the rest of the buffer.
-  wchar_t double_terminated_path[MAX_PATH + 1] = {0};
-#pragma warning(suppress:4996)  // don't complain about wcscpy deprecation
-  wcscpy(double_terminated_path, path.value().c_str());
-
-  SHFILEOPSTRUCT file_operation = {0};
-  file_operation.wFunc = FO_DELETE;
-  file_operation.pFrom = double_terminated_path;
-  file_operation.fFlags = FOF_ALLOWUNDO | FOF_SILENT | FOF_NOCONFIRMATION;
-  int err = SHFileOperation(&file_operation);
-
-  // Since we're passing flags to the operation telling it to be silent,
-  // it's possible for the operation to be aborted/cancelled without err
-  // being set (although MSDN doesn't give any scenarios for how this can
-  // happen).  See MSDN for SHFileOperation and SHFILEOPTSTRUCT.
-  if (file_operation.fAnyOperationsAborted)
+  base::win::ScopedCOMInitializer com_initializer;
+  if (!com_initializer.succeeded())
+    return false;
+
+  base::win::ScopedComPtr<IFileOperation> pfo;
+  if (FAILED(pfo.CreateInstance(CLSID_FileOperation)))
+    return false;
+
+  // Elevation prompt enabled for UAC protected files.  This overrides the
+  // SILENT, NO_UI and NOERRORUI flags.
+  if (FAILED(pfo->SetOperationFlags(FOF_NO_UI |
+                                    FOF_ALLOWUNDO |
+                                    FOF_NOERRORUI |
+                                    FOF_SILENT |
+                                    FOFX_SHOWELEVATIONPROMPT |
+                                    FOFX_RECYCLEONDELETE)))
+    return false;
+
+  // Create an IShellItem from the supplied source path.
+  base::win::ScopedComPtr<IShellItem> delete_item;
+  if (FAILED(SHCreateItemFromParsingName(path.value().c_str(),
+                                         NULL,
+                                         IID_PPV_ARGS(delete_item.Receive()))))
+    return false;
+
+  base::win::ScopedComPtr<IFileOperationProgressSink> delete_sink(
+      new DeleteFileProgressSink);
+  if (!delete_sink)
     return false;
 
-  // Some versions of Windows return ERROR_FILE_NOT_FOUND (0x2) when deleting
-  // an empty directory and some return 0x402 when they should be returning
-  // ERROR_FILE_NOT_FOUND. MSDN says Vista and up won't return 0x402.  Windows 7
-  // can return DE_INVALIDFILES (0x7C) for nonexistent directories.
-  return (err == 0 || err == ERROR_FILE_NOT_FOUND || err == 0x402 ||
-          err == 0x7C);
+  // Processes the queued command DeleteItem. This will trigger
+  // the DeleteFileProgressSink to check for Recycle Bin.
+  return SUCCEEDED(pfo->DeleteItem(delete_item.get(), delete_sink.get())) &&
+         SUCCEEDED(pfo->PerformOperations());
 }
 
 void Beep() {