From 23981fc4d3fa8cb2596bd2f13c1f1d97e58bfc1c Mon Sep 17 00:00:00 2001 From: Aaron Klotz Date: Mon, 6 Jun 2022 20:01:59 -0500 Subject: Bug 1677168 - nsFilePicker cleanup --- system/interface/windows/nsFilePicker.cpp | 350 +++++++++--------------------- system/interface/windows/nsFilePicker.h | 32 +-- 2 files changed, 107 insertions(+), 275 deletions(-) diff --git a/system/interface/windows/nsFilePicker.cpp b/system/interface/windows/nsFilePicker.cpp index 59ae152ec..3d733fcb7 100644 --- a/system/interface/windows/nsFilePicker.cpp +++ b/system/interface/windows/nsFilePicker.cpp @@ -35,12 +35,14 @@ using mozilla::mscom::EnsureMTA; using mozilla::UniquePtr; using namespace mozilla::widget; -char16_t *nsFilePicker::mLastUsedUnicodeDirectory; +UniquePtr + nsFilePicker::sLastUsedUnicodeDirectory; + char nsFilePicker::mLastUsedDirectory[MAX_PATH+1] = { 0 }; static const wchar_t kDialogPtrProp[] = L"DialogPtrProperty"; static const DWORD kDialogTimerID = 9999; -static const unsigned long kDialogTimerTimeout = 300; + #define MAX_EXTENSION_LENGTH 10 #define FILE_BUFFER_SIZE 4096 @@ -71,31 +73,6 @@ private: RefPtr mWindow; }; -// Manages the current working path. -class AutoRestoreWorkingPath -{ -public: - AutoRestoreWorkingPath() { - DWORD bufferLength = GetCurrentDirectoryW(0, nullptr); - mWorkingPath = MakeUnique(bufferLength); - if (GetCurrentDirectoryW(bufferLength, mWorkingPath.get()) == 0) { - mWorkingPath = nullptr; - } - } - - ~AutoRestoreWorkingPath() { - if (HasWorkingPath()) { - ::SetCurrentDirectoryW(mWorkingPath.get()); - } - } - - inline bool HasWorkingPath() const { - return mWorkingPath != nullptr; - } -private: - UniquePtr mWorkingPath; -}; - // Manages NS_NATIVE_TMP_WINDOW child windows. NS_NATIVE_TMP_WINDOWs are // temporary child windows of mParentWidget created to address RTL issues // in picker dialogs. We are responsible for destroying these. @@ -140,57 +117,11 @@ private: RefPtr mWindow; }; -// Manages a simple callback timer -class AutoTimerCallbackCancel -{ -public: - AutoTimerCallbackCancel(nsFilePicker* aTarget, - nsTimerCallbackFunc aCallbackFunc) { - Init(aTarget, aCallbackFunc); - } - - ~AutoTimerCallbackCancel() { - if (mPickerCallbackTimer) { - mPickerCallbackTimer->Cancel(); - } - } - -private: - void Init(nsFilePicker* aTarget, - nsTimerCallbackFunc aCallbackFunc) { - mPickerCallbackTimer = do_CreateInstance("@mozilla.org/timer;1"); - if (!mPickerCallbackTimer) { - NS_WARNING("do_CreateInstance for timer failed??"); - return; - } - mPickerCallbackTimer->InitWithFuncCallback(aCallbackFunc, - aTarget, - kDialogTimerTimeout, - nsITimer::TYPE_REPEATING_SLACK); - } - nsCOMPtr mPickerCallbackTimer; - -}; - /////////////////////////////////////////////////////////////////////////////// // nsIFilePicker -nsFilePicker::nsFilePicker() : - mSelectedType(1) - , mDlgWnd(nullptr) - , mFDECookie(0) -{ - CoInitialize(nullptr); -} - -nsFilePicker::~nsFilePicker() -{ - if (mLastUsedUnicodeDirectory) { - free(mLastUsedUnicodeDirectory); - mLastUsedUnicodeDirectory = nullptr; - } - CoUninitialize(); -} +nsFilePicker::nsFilePicker() + : mSelectedType(1) {} NS_IMPL_ISUPPORTS(nsFilePicker, nsIFilePicker) @@ -205,130 +136,6 @@ NS_IMETHODIMP nsFilePicker::Init(mozIDOMWindowProxy *aParent, const nsAString& a return nsBaseFilePicker::Init(aParent, aTitle, aMode); } -STDMETHODIMP nsFilePicker::QueryInterface(REFIID refiid, void** ppvResult) -{ - *ppvResult = nullptr; - if (IID_IUnknown == refiid || - refiid == IID_IFileDialogEvents) { - *ppvResult = this; - } - - if (nullptr != *ppvResult) { - ((LPUNKNOWN)*ppvResult)->AddRef(); - return S_OK; - } - - return E_NOINTERFACE; -} - - -/* - * Callbacks - */ - -HRESULT -nsFilePicker::OnFileOk(IFileDialog *pfd) -{ - return S_OK; -} - -HRESULT -nsFilePicker::OnFolderChanging(IFileDialog *pfd, - IShellItem *psiFolder) -{ - return S_OK; -} - -HRESULT -nsFilePicker::OnFolderChange(IFileDialog *pfd) -{ - return S_OK; -} - -HRESULT -nsFilePicker::OnSelectionChange(IFileDialog *pfd) -{ - return S_OK; -} - -HRESULT -nsFilePicker::OnShareViolation(IFileDialog *pfd, - IShellItem *psi, - FDE_SHAREVIOLATION_RESPONSE *pResponse) -{ - return S_OK; -} - -HRESULT -nsFilePicker::OnTypeChange(IFileDialog *pfd) -{ - // Failures here result in errors due to security concerns. - RefPtr win; - pfd->QueryInterface(IID_IOleWindow, getter_AddRefs(win)); - if (!win) { - NS_ERROR("Could not retrieve the IOleWindow interface for IFileDialog."); - return S_OK; - } - HWND hwnd = nullptr; - win->GetWindow(&hwnd); - if (!hwnd) { - NS_ERROR("Could not retrieve the HWND for IFileDialog."); - return S_OK; - } - - SetDialogHandle(hwnd); - return S_OK; -} - -HRESULT -nsFilePicker::OnOverwrite(IFileDialog *pfd, - IShellItem *psi, - FDE_OVERWRITE_RESPONSE *pResponse) -{ - return S_OK; -} - -/* - * Close on parent close logic - */ - -bool -nsFilePicker::ClosePickerIfNeeded() -{ - if (!mParentWidget || !mDlgWnd) - return false; - - nsWindow *win = static_cast(mParentWidget.get()); - if (IsWindow(mDlgWnd) && IsWindowVisible(mDlgWnd) && win->DestroyCalled()) { - wchar_t className[64]; - // Make sure we have the right window - if (GetClassNameW(mDlgWnd, className, mozilla::ArrayLength(className)) && - !wcscmp(className, L"#32770") && - DestroyWindow(mDlgWnd)) { - mDlgWnd = nullptr; - return true; - } - } - return false; -} - -void -nsFilePicker::PickerCallbackTimerFunc(nsITimer *aTimer, void *aCtx) -{ - nsFilePicker* picker = (nsFilePicker*)aCtx; - if (picker->ClosePickerIfNeeded()) { - aTimer->Cancel(); - } -} - -void -nsFilePicker::SetDialogHandle(HWND aWnd) -{ - if (!aWnd || mDlgWnd) - return; - mDlgWnd = aWnd; -} - /* * Folder picker invocation */ @@ -353,15 +160,14 @@ nsFilePicker::ShowFolderPicker(const nsString& aInitialDir) } RefPtr dialog; - if (FAILED(CoCreateInstance(CLSID_FileOpenDialog, nullptr, CLSCTX_INPROC, + if (FAILED(CoCreateInstance(CLSID_FileOpenDialog, + nullptr, + CLSCTX_INPROC_SERVER, IID_IFileOpenDialog, getter_AddRefs(dialog)))) { return false; } - // hook up event callbacks - dialog->Advise(this, &mFDECookie); - // options FILEOPENDIALOGOPTIONS fos = FOS_PICKFOLDERS; // Require interaction if the folder picker is triggered by an element that @@ -371,13 +177,22 @@ nsFilePicker::ShowFolderPicker(const nsString& aInitialDir) fos |= FOS_OKBUTTONNEEDSINTERACTION; } - dialog->SetOptions(fos); + HRESULT hr = dialog->SetOptions(fos); + if (FAILED(hr)) { + return false; + } // initial strings - dialog->SetTitle(mTitle.get()); + hr = dialog->SetTitle(mTitle.get()); + if (FAILED(hr)) { + return false; + } if (!mOkButtonLabel.IsEmpty()) { - dialog->SetOkButtonLabel(mOkButtonLabel.get()); + hr = dialog->SetOkButtonLabel(mOkButtonLabel.get()); + if (FAILED(hr)) { + return false; + } } if (!aInitialDir.IsEmpty()) { @@ -386,7 +201,10 @@ nsFilePicker::ShowFolderPicker(const nsString& aInitialDir) WinUtils::SHCreateItemFromParsingName(aInitialDir.get(), nullptr, IID_IShellItem, getter_AddRefs(folder)))) { - dialog->SetFolder(folder); + hr = dialog->SetFolder(folder); + if (FAILED(hr)) { + return false; + } } } @@ -398,10 +216,8 @@ nsFilePicker::ShowFolderPicker(const nsString& aInitialDir) if (FAILED(dialog->Show(adtw.get())) || FAILED(dialog->GetResult(getter_AddRefs(item))) || !item) { - dialog->Unadvise(mFDECookie); return false; } - dialog->Unadvise(mFDECookie); // results @@ -409,8 +225,14 @@ nsFilePicker::ShowFolderPicker(const nsString& aInitialDir) // default save folder. RefPtr folderPath; RefPtr shellLib; - CoCreateInstance(CLSID_ShellLibrary, nullptr, CLSCTX_INPROC, - IID_IShellLibrary, getter_AddRefs(shellLib)); + if (FAILED(CoCreateInstance(CLSID_ShellLibrary, + nullptr, + CLSCTX_INPROC_SERVER, + IID_IShellLibrary, + getter_AddRefs(shellLib)))) { + return false; + } + if (shellLib && SUCCEEDED(shellLib->LoadLibraryFromItem(item, STGM_READ)) && SUCCEEDED(shellLib->GetDefaultSaveFolder(DSFT_DETECT, IID_IShellItem, @@ -449,22 +271,23 @@ nsFilePicker::ShowFilePicker(const nsString& aInitialDir) RefPtr dialog; if (mMode != modeSave) { - if (FAILED(CoCreateInstance(CLSID_FileOpenDialog, nullptr, CLSCTX_INPROC, + if (FAILED(CoCreateInstance(CLSID_FileOpenDialog, + nullptr, + CLSCTX_INPROC_SERVER, IID_IFileOpenDialog, getter_AddRefs(dialog)))) { return false; } } else { - if (FAILED(CoCreateInstance(CLSID_FileSaveDialog, nullptr, CLSCTX_INPROC, + if (FAILED(CoCreateInstance(CLSID_FileSaveDialog, + nullptr, + CLSCTX_INPROC_SERVER, IID_IFileSaveDialog, getter_AddRefs(dialog)))) { return false; } } - // hook up event callbacks - dialog->Advise(this, &mFDECookie); - // options FILEOPENDIALOGOPTIONS fos = 0; @@ -483,10 +306,6 @@ nsFilePicker::ShowFilePicker(const nsString& aInitialDir) fos |= FOS_DONTADDTORECENT; } - // Msdn claims FOS_NOCHANGEDIR is not needed. We'll add this - // just in case. - AutoRestoreWorkingPath arw; - // mode specific switch(mMode) { case modeOpen: @@ -506,25 +325,45 @@ nsFilePicker::ShowFilePicker(const nsString& aInitialDir) break; } - dialog->SetOptions(fos); + HRESULT hr = dialog->SetOptions(fos); + if (FAILED(hr)) { + return false; + } // initial strings // title - dialog->SetTitle(mTitle.get()); + hr = dialog->SetTitle(mTitle.get()); + if (FAILED(hr)) { + return false; + } // default filename if (!mDefaultFilename.IsEmpty()) { - dialog->SetFileName(mDefaultFilename.get()); + // Prevent the shell from expanding environment variables by removing + // the % characters that are used to delimit them. + nsAutoString sanitizedFilename(mDefaultFilename); + sanitizedFilename.ReplaceChar('%', '_'); + + hr = dialog->SetFileName(sanitizedFilename.get()); + if (FAILED(hr)) { + return false; + } } NS_NAMED_LITERAL_STRING(htmExt, "html"); // default extension to append to new files if (!mDefaultExtension.IsEmpty()) { - dialog->SetDefaultExtension(mDefaultExtension.get()); + hr = dialog->SetDefaultExtension(mDefaultExtension.get()); + if (FAILED(hr)) { + return false; + } } else if (IsDefaultPathHtml()) { - dialog->SetDefaultExtension(htmExt.get()); + hr = dialog->SetDefaultExtension(htmExt.get()); + if (FAILED(hr)) { + return false; + } } // initial location @@ -534,14 +373,24 @@ nsFilePicker::ShowFilePicker(const nsString& aInitialDir) WinUtils::SHCreateItemFromParsingName(aInitialDir.get(), nullptr, IID_IShellItem, getter_AddRefs(folder)))) { - dialog->SetFolder(folder); + hr = dialog->SetFolder(folder); + if (FAILED(hr)) { + return false; + } } } // filter types and the default index if (!mComFilterList.IsEmpty()) { - dialog->SetFileTypes(mComFilterList.Length(), mComFilterList.get()); - dialog->SetFileTypeIndex(mSelectedType); + hr = dialog->SetFileTypes(mComFilterList.Length(), mComFilterList.get()); + if (FAILED(hr)) { + return false; + } + + hr = dialog->SetFileTypeIndex(mSelectedType); + if (FAILED(hr)) { + return false; + } } // display @@ -549,14 +398,11 @@ nsFilePicker::ShowFilePicker(const nsString& aInitialDir) { AutoDestroyTmpWindow adtw((HWND)(mParentWidget.get() ? mParentWidget->GetNativeData(NS_NATIVE_TMP_WINDOW) : nullptr)); - AutoTimerCallbackCancel atcc(this, PickerCallbackTimerFunc); AutoWidgetPickerState awps(mParentWidget); if (FAILED(dialog->Show(adtw.get()))) { - dialog->Unadvise(mFDECookie); return false; } - dialog->Unadvise(mFDECookie); } // results @@ -596,9 +442,10 @@ nsFilePicker::ShowFilePicker(const nsString& aInitialDir) if (SUCCEEDED(items->GetItemAt(idx, getter_AddRefs(item)))) { if (!WinUtils::GetShellItemPath(item, str)) continue; - nsCOMPtr file = do_CreateInstance("@mozilla.org/file/local;1"); - if (file && NS_SUCCEEDED(file->InitWithPath(str))) + nsCOMPtr file; + if (NS_SUCCEEDED(NS_NewLocalFile(str, false, getter_AddRefs(file)))) { mFiles.AppendObject(file); + } } } return true; @@ -623,7 +470,7 @@ nsFilePicker::ShowW(int16_t *aReturnVal) // If no display directory, re-use the last one. if(initialDir.IsEmpty()) { // Allocate copy of last used dir. - initialDir = mLastUsedUnicodeDirectory; + initialDir = sLastUsedUnicodeDirectory.get(); } // Clear previous file selections @@ -651,10 +498,11 @@ nsFilePicker::ShowW(int16_t *aReturnVal) if (mMode == modeSave) { // Windows does not return resultReplace, we must check if file // already exists. - nsCOMPtr file(do_CreateInstance("@mozilla.org/file/local;1")); + nsCOMPtr file; + nsresult rv = NS_NewLocalFile(mUnicodeFile, false, getter_AddRefs(file)); + bool flag = false; - if (file && NS_SUCCEEDED(file->InitWithPath(mUnicodeFile)) && - NS_SUCCEEDED(file->Exists(&flag)) && flag) { + if (NS_SUCCEEDED(rv) && NS_SUCCEEDED(file->Exists(&flag)) && flag) { retValue = returnReplace; } } @@ -678,14 +526,13 @@ nsFilePicker::GetFile(nsIFile **aFile) if (mUnicodeFile.IsEmpty()) return NS_OK; - nsCOMPtr file(do_CreateInstance("@mozilla.org/file/local;1")); - - NS_ENSURE_TRUE(file, NS_ERROR_FAILURE); - - file->InitWithPath(mUnicodeFile); - - NS_ADDREF(*aFile = file); + nsCOMPtr file; + nsresult rv = NS_NewLocalFile(mUnicodeFile, false, getter_AddRefs(file)); + if (NS_FAILED(rv)) { + return rv; + } + file.forget(aFile); return NS_OK; } @@ -800,8 +647,13 @@ nsFilePicker::AppendFilter(const nsAString& aTitle, const nsAString& aFilter) void nsFilePicker::RememberLastUsedDirectory() { - nsCOMPtr file(do_CreateInstance("@mozilla.org/file/local;1")); - if (!file || NS_FAILED(file->InitWithPath(mUnicodeFile))) { + if (IsPrivacyModeEnabled()) { + // Don't remember the directory if private browsing was in effect + return; + } + + nsCOMPtr file; + if (NS_FAILED(NS_NewLocalFile(mUnicodeFile, false, getter_AddRefs(file)))) { NS_WARNING("RememberLastUsedDirectory failed to init file path."); return; } @@ -816,11 +668,7 @@ nsFilePicker::RememberLastUsedDirectory() return; } - if (mLastUsedUnicodeDirectory) { - free(mLastUsedUnicodeDirectory); - mLastUsedUnicodeDirectory = nullptr; - } - mLastUsedUnicodeDirectory = ToNewUnicode(newDir); + sLastUsedUnicodeDirectory.reset(ToNewUnicode(newDir)); } bool diff --git a/system/interface/windows/nsFilePicker.h b/system/interface/windows/nsFilePicker.h index 7a3fbbe07..050b6acaf 100644 --- a/system/interface/windows/nsFilePicker.h +++ b/system/interface/windows/nsFilePicker.h @@ -10,7 +10,6 @@ #include #include "nsIFile.h" -#include "nsITimer.h" #include "nsISimpleEnumerator.h" #include "nsCOMArray.h" #include "nsBaseFilePicker.h" @@ -41,11 +40,9 @@ protected: * Native Windows FileSelector wrapper */ -class nsFilePicker : - public IFileDialogEvents, - public nsBaseWinFilePicker -{ - virtual ~nsFilePicker(); +class nsFilePicker : public nsBaseWinFilePicker { + virtual ~nsFilePicker() = default; + public: nsFilePicker(); @@ -54,9 +51,6 @@ public: NS_DECL_ISUPPORTS - // IUnknown's QueryInterface - STDMETHODIMP QueryInterface(REFIID refiid, void** ppvResult); - // nsIFilePicker (less what's in nsBaseFilePicker and nsBaseWinFilePicker) NS_IMETHOD GetFilterIndex(int32_t *aFilterIndex); NS_IMETHOD SetFilterIndex(int32_t aFilterIndex); @@ -67,15 +61,6 @@ public: NS_IMETHOD ShowW(int16_t *aReturnVal); NS_IMETHOD AppendFilter(const nsAString& aTitle, const nsAString& aFilter); - // IFileDialogEvents - HRESULT STDMETHODCALLTYPE OnFileOk(IFileDialog *pfd); - HRESULT STDMETHODCALLTYPE OnFolderChanging(IFileDialog *pfd, IShellItem *psiFolder); - HRESULT STDMETHODCALLTYPE OnFolderChange(IFileDialog *pfd); - HRESULT STDMETHODCALLTYPE OnSelectionChange(IFileDialog *pfd); - HRESULT STDMETHODCALLTYPE OnShareViolation(IFileDialog *pfd, IShellItem *psi, FDE_SHAREVIOLATION_RESPONSE *pResponse); - HRESULT STDMETHODCALLTYPE OnTypeChange(IFileDialog *pfd); - HRESULT STDMETHODCALLTYPE OnOverwrite(IFileDialog *pfd, IShellItem *psi, FDE_OVERWRITE_RESPONSE *pResponse); - protected: /* method from nsBaseFilePicker */ virtual void InitNative(nsIWidget *aParent, @@ -87,9 +72,6 @@ protected: bool IsPrivacyModeEnabled(); bool IsDefaultPathLink(); bool IsDefaultPathHtml(); - void SetDialogHandle(HWND aWnd); - bool ClosePickerIfNeeded(); - static void PickerCallbackTimerFunc(nsITimer *aTimer, void *aPicker); nsCOMPtr mLoadContext; nsCOMPtr mParentWidget; @@ -100,10 +82,13 @@ protected: nsCOMArray mFiles; static char mLastUsedDirectory[]; nsString mUnicodeFile; - static char16_t *mLastUsedUnicodeDirectory; - HWND mDlgWnd; bool mRequireInteraction; + struct FreeDeleter { + void operator()(void* aPtr) { ::free(aPtr); } + }; + static mozilla::UniquePtr sLastUsedUnicodeDirectory; + class ComDlgFilterSpec { public: @@ -129,7 +114,6 @@ protected: }; ComDlgFilterSpec mComFilterList; - DWORD mFDECookie; }; #endif // nsFilePicker_h__ -- cgit v1.2.3