Bug 1677168: nsFilePicker cleanup; r=mhowell

* We replace `mLastUsedUnicodeDirectory` and its manual memory management with
  a `UniquePtr` named `sLastUsedUnicodeDirectory`
* We remove `AutoRestoreWorkingPath`, as it never was actually necessary and
  was left in, "just in case."
* We remove the timeout stuff, as that never actually worked, at least with
  the current implementation of the file picker; perhaps it worked with the
  legacy file picker APIs, but it doesn't work with the Vista+, COM-based stuff.
* We also get rid of `IFileDialogEvents`, as all of that stuff was needed for
  timeouts. It also adds additional complexity that impairs our ability to
  implement out-of-process file pickers in a future bug.
* We replace `do_CreateInstance("@mozilla.org/file/local;1")` with
  `NS_NewLocalFile`.
* We add `HRESULT` error handling to all of the method invocations on COM
  interfaces.
* We replace `CLSCTX_INPROC` with `CLSCTX_INPROC_SERVER` in `CoCreateInstance`
  calls.
* We remove `CoInitialize` and `CoUninitialize` calls. Those have been
  unnecessary ever since we added `mscom::ProcessRuntime` instantiation during
  startup.
* We add a missing check for private browsing mode in
  `nsFilePicker::RememberLastUsedDirectory`.

Differential Revision: https://phabricator.services.mozilla.com/D97045
This commit is contained in:
Aaron Klotz 2020-11-16 19:37:09 +00:00
parent 673de157fa
commit be59e29275
2 changed files with 96 additions and 243 deletions

View File

@ -29,11 +29,11 @@ using mozilla::IsWin8OrLater;
using mozilla::MakeUnique;
using mozilla::UniquePtr;
using mozilla::mscom::EnsureMTA;
using namespace mozilla::widget;
char16_t* nsFilePicker::mLastUsedUnicodeDirectory;
static const unsigned long kDialogTimerTimeout = 300;
UniquePtr<char16_t[], nsFilePicker::FreeDeleter>
nsFilePicker::sLastUsedUnicodeDirectory;
#define MAX_EXTENSION_LENGTH 10
#define FILE_BUFFER_SIZE 4096
@ -43,29 +43,6 @@ typedef DWORD FILEOPENDIALOGOPTIONS;
///////////////////////////////////////////////////////////////////////////////
// Helper classes
// Manages the current working path.
class AutoRestoreWorkingPath {
public:
AutoRestoreWorkingPath() {
DWORD bufferLength = GetCurrentDirectoryW(0, nullptr);
mWorkingPath = MakeUnique<wchar_t[]>(bufferLength);
if (GetCurrentDirectoryW(bufferLength, mWorkingPath.get()) == 0) {
mWorkingPath = nullptr;
}
}
~AutoRestoreWorkingPath() {
if (HasWorkingPath()) {
::SetCurrentDirectoryW(mWorkingPath.get());
}
}
inline bool HasWorkingPath() const { return mWorkingPath != nullptr; }
private:
UniquePtr<wchar_t[]> 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.
@ -105,49 +82,10 @@ class AutoWidgetPickerState {
RefPtr<nsWindow> mWindow;
};
// Manages a simple callback timer
class AutoTimerCallbackCancel {
public:
AutoTimerCallbackCancel(nsFilePicker* aTarget,
nsTimerCallbackFunc aCallbackFunc,
const char* aName) {
Init(aTarget, aCallbackFunc, aName);
}
~AutoTimerCallbackCancel() {
if (mPickerCallbackTimer) {
mPickerCallbackTimer->Cancel();
}
}
private:
void Init(nsFilePicker* aTarget, nsTimerCallbackFunc aCallbackFunc,
const char* aName) {
NS_NewTimerWithFuncCallback(getter_AddRefs(mPickerCallbackTimer),
aCallbackFunc, aTarget, kDialogTimerTimeout,
nsITimer::TYPE_REPEATING_SLACK, aName);
if (!mPickerCallbackTimer) {
NS_WARNING("do_CreateInstance for timer failed??");
}
}
nsCOMPtr<nsITimer> 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)
@ -160,102 +98,6 @@ NS_IMETHODIMP nsFilePicker::Init(mozIDOMWindowProxy* aParent,
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;
}
/*
* Vista+ 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<IOleWindow> 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<nsWindow*>(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
*/
@ -278,23 +120,30 @@ bool nsFilePicker::ShowFolderPicker(const nsString& aInitialDir) {
}
RefPtr<IFileOpenDialog> dialog;
if (FAILED(CoCreateInstance(CLSID_FileOpenDialog, nullptr, CLSCTX_INPROC,
IID_IFileOpenDialog, getter_AddRefs(dialog)))) {
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;
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()) {
@ -302,7 +151,10 @@ bool nsFilePicker::ShowFolderPicker(const nsString& aInitialDir) {
if (SUCCEEDED(SHCreateItemFromParsingName(aInitialDir.get(), nullptr,
IID_IShellItem,
getter_AddRefs(folder)))) {
dialog->SetFolder(folder);
hr = dialog->SetFolder(folder);
if (FAILED(hr)) {
return false;
}
}
}
@ -315,10 +167,8 @@ bool nsFilePicker::ShowFolderPicker(const nsString& aInitialDir) {
RefPtr<IShellItem> item;
if (FAILED(dialog->Show(adtw.get())) ||
FAILED(dialog->GetResult(getter_AddRefs(item))) || !item) {
dialog->Unadvise(mFDECookie);
return false;
}
dialog->Unadvise(mFDECookie);
// results
@ -326,8 +176,11 @@ bool nsFilePicker::ShowFolderPicker(const nsString& aInitialDir) {
// default save folder.
RefPtr<IShellItem> folderPath;
RefPtr<IShellLibrary> 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,
getter_AddRefs(folderPath)))) {
@ -363,20 +216,19 @@ bool nsFilePicker::ShowFilePicker(const nsString& aInitialDir) {
RefPtr<IFileDialog> dialog;
if (mMode != modeSave) {
if (FAILED(CoCreateInstance(CLSID_FileOpenDialog, nullptr, CLSCTX_INPROC,
IID_IFileOpenDialog, getter_AddRefs(dialog)))) {
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,
IID_IFileSaveDialog, getter_AddRefs(dialog)))) {
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;
@ -387,10 +239,6 @@ bool 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:
@ -409,23 +257,38 @@ bool 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());
hr = dialog->SetFileName(mDefaultFilename.get());
if (FAILED(hr)) {
return false;
}
}
// 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(L"html");
hr = dialog->SetDefaultExtension(L"html");
if (FAILED(hr)) {
return false;
}
}
// initial location
@ -434,14 +297,24 @@ bool nsFilePicker::ShowFilePicker(const nsString& aInitialDir) {
if (SUCCEEDED(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
@ -450,16 +323,12 @@ bool nsFilePicker::ShowFilePicker(const nsString& aInitialDir) {
AutoDestroyTmpWindow adtw((HWND)(
mParentWidget.get() ? mParentWidget->GetNativeData(NS_NATIVE_TMP_WINDOW)
: nullptr));
AutoTimerCallbackCancel atcc(this, PickerCallbackTimerFunc,
"nsFilePicker::PickerCallbackTimerFunc");
AutoWidgetPickerState awps(mParentWidget);
mozilla::BackgroundHangMonitor().NotifyWait();
if (FAILED(dialog->Show(adtw.get()))) {
dialog->Unadvise(mFDECookie);
return false;
}
dialog->Unadvise(mFDECookie);
}
// results
@ -497,9 +366,10 @@ bool nsFilePicker::ShowFilePicker(const nsString& aInitialDir) {
nsAutoString str;
if (SUCCEEDED(items->GetItemAt(idx, getter_AddRefs(item)))) {
if (!WinUtils::GetShellItemPath(item, str)) continue;
nsCOMPtr<nsIFile> file = do_CreateInstance("@mozilla.org/file/local;1");
if (file && NS_SUCCEEDED(file->InitWithPath(str)))
nsCOMPtr<nsIFile> file;
if (NS_SUCCEEDED(NS_NewLocalFile(str, false, getter_AddRefs(file)))) {
mFiles.AppendObject(file);
}
}
}
return true;
@ -519,7 +389,7 @@ nsresult 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
@ -546,10 +416,11 @@ nsresult nsFilePicker::ShowW(int16_t* aReturnVal) {
if (mMode == modeSave) {
// Windows does not return resultReplace, we must check if file
// already exists.
nsCOMPtr<nsIFile> file(do_CreateInstance("@mozilla.org/file/local;1"));
nsCOMPtr<nsIFile> 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;
}
}
@ -567,14 +438,13 @@ nsFilePicker::GetFile(nsIFile** aFile) {
if (mUnicodeFile.IsEmpty()) return NS_OK;
nsCOMPtr<nsIFile> file(do_CreateInstance("@mozilla.org/file/local;1"));
NS_ENSURE_TRUE(file, NS_ERROR_FAILURE);
file->InitWithPath(mUnicodeFile);
NS_ADDREF(*aFile = file);
nsCOMPtr<nsIFile> file;
nsresult rv = NS_NewLocalFile(mUnicodeFile, false, getter_AddRefs(file));
if (NS_FAILED(rv)) {
return rv;
}
file.forget(aFile);
return NS_OK;
}
@ -673,8 +543,13 @@ nsFilePicker::AppendFilter(const nsAString& aTitle, const nsAString& aFilter) {
}
void nsFilePicker::RememberLastUsedDirectory() {
nsCOMPtr<nsIFile> 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<nsIFile> file;
if (NS_FAILED(NS_NewLocalFile(mUnicodeFile, false, getter_AddRefs(file)))) {
NS_WARNING("RememberLastUsedDirectory failed to init file path.");
return;
}
@ -688,11 +563,7 @@ void nsFilePicker::RememberLastUsedDirectory() {
return;
}
if (mLastUsedUnicodeDirectory) {
free(mLastUsedUnicodeDirectory);
mLastUsedUnicodeDirectory = nullptr;
}
mLastUsedUnicodeDirectory = ToNewUnicode(newDir);
sLastUsedUnicodeDirectory.reset(ToNewUnicode(newDir));
}
bool nsFilePicker::IsPrivacyModeEnabled() {

View File

@ -10,7 +10,6 @@
#include <windows.h>
#include "nsIFile.h"
#include "nsITimer.h"
#include "nsISimpleEnumerator.h"
#include "nsCOMArray.h"
#include "nsBaseFilePicker.h"
@ -39,8 +38,8 @@ class nsBaseWinFilePicker : public nsBaseFilePicker {
* Native Windows FileSelector wrapper
*/
class nsFilePicker : public IFileDialogEvents, public nsBaseWinFilePicker {
virtual ~nsFilePicker();
class nsFilePicker : public nsBaseWinFilePicker {
virtual ~nsFilePicker() = default;
public:
nsFilePicker();
@ -50,9 +49,6 @@ class nsFilePicker : public IFileDialogEvents, public nsBaseWinFilePicker {
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) override;
NS_IMETHOD SetFilterIndex(int32_t aFilterIndex) override;
@ -62,19 +58,6 @@ class nsFilePicker : public IFileDialogEvents, public nsBaseWinFilePicker {
NS_IMETHOD AppendFilter(const nsAString& aTitle,
const nsAString& aFilter) override;
// 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, const nsAString& aTitle) override;
@ -87,9 +70,6 @@ class nsFilePicker : public IFileDialogEvents, public nsBaseWinFilePicker {
bool IsPrivacyModeEnabled();
bool IsDefaultPathLink();
bool IsDefaultPathHtml();
void SetDialogHandle(HWND aWnd);
bool ClosePickerIfNeeded();
static void PickerCallbackTimerFunc(nsITimer* aTimer, void* aPicker);
nsCOMPtr<nsILoadContext> mLoadContext;
nsCOMPtr<nsIWidget> mParentWidget;
@ -99,8 +79,11 @@ class nsFilePicker : public IFileDialogEvents, public nsBaseWinFilePicker {
int16_t mSelectedType;
nsCOMArray<nsIFile> mFiles;
nsString mUnicodeFile;
static char16_t* mLastUsedUnicodeDirectory;
HWND mDlgWnd;
struct FreeDeleter {
void operator()(void* aPtr) { ::free(aPtr); }
};
static mozilla::UniquePtr<char16_t[], FreeDeleter> sLastUsedUnicodeDirectory;
class ComDlgFilterSpec {
public:
@ -121,7 +104,6 @@ class nsFilePicker : public IFileDialogEvents, public nsBaseWinFilePicker {
};
ComDlgFilterSpec mComFilterList;
DWORD mFDECookie;
};
#endif // nsFilePicker_h__