From 45409bb4e62fa583b8e4973cdd72f479af814174 Mon Sep 17 00:00:00 2001 From: reuk Date: Fri, 26 Feb 2021 23:42:47 +0000 Subject: [PATCH] FileChooser: Hide chooser when it leaves scope on Windows --- extras/Build/juceaide/Main.cpp | 2 +- .../components/juce_Component.h | 5 +- .../filebrowser/juce_FileChooser.cpp | 12 +- .../filebrowser/juce_FileChooser.h | 7 +- .../native/juce_android_FileChooser.cpp | 6 +- .../native/juce_ios_FileChooser.mm | 6 +- .../native/juce_linux_FileChooser.cpp | 4 +- .../native/juce_mac_FileChooser.mm | 6 +- .../native/juce_win32_FileChooser.cpp | 152 ++++++++++-------- 9 files changed, 105 insertions(+), 95 deletions(-) mode change 100644 => 100755 modules/juce_gui_basics/native/juce_win32_FileChooser.cpp diff --git a/extras/Build/juceaide/Main.cpp b/extras/Build/juceaide/Main.cpp index a2e2fc579e..25b4efc50f 100644 --- a/extras/Build/juceaide/Main.cpp +++ b/extras/Build/juceaide/Main.cpp @@ -495,7 +495,7 @@ int main (int argc, char** argv) std::transform (argv, argv + argc, std::back_inserter (arguments), getString); juce::ArgumentList argumentList { arguments.front(), - juce::StringArray (arguments.data() + 1, arguments.size() - 1) }; + juce::StringArray (arguments.data() + 1, (int) arguments.size() - 1) }; using Fn = typename std::add_lvalue_reference::type; diff --git a/modules/juce_gui_basics/components/juce_Component.h b/modules/juce_gui_basics/components/juce_Component.h index 6b2b0072b7..69dfcf11b4 100644 --- a/modules/juce_gui_basics/components/juce_Component.h +++ b/modules/juce_gui_basics/components/juce_Component.h @@ -2179,10 +2179,7 @@ public: operator ComponentType*() const noexcept { return getComponent(); } /** Returns the component that this pointer refers to, or null if the component no longer exists. */ - ComponentType* operator->() noexcept { return getComponent(); } - - /** Returns the component that this pointer refers to, or null if the component no longer exists. */ - const ComponentType* operator->() const noexcept { return getComponent(); } + ComponentType* operator->() const noexcept { return getComponent(); } /** If the component is valid, this deletes it and sets this pointer to null. */ void deleteAndZero() { delete getComponent(); } diff --git a/modules/juce_gui_basics/filebrowser/juce_FileChooser.cpp b/modules/juce_gui_basics/filebrowser/juce_FileChooser.cpp index 661c02a357..654be39835 100644 --- a/modules/juce_gui_basics/filebrowser/juce_FileChooser.cpp +++ b/modules/juce_gui_basics/filebrowser/juce_FileChooser.cpp @@ -158,7 +158,7 @@ bool FileChooser::showDialog (const int flags, FilePreviewComponent* const previ { FocusRestorer focusRestorer; - pimpl.reset (createPimpl (flags, previewComp)); + pimpl = createPimpl (flags, previewComp); pimpl->runModally(); // ensure that the finished function was invoked @@ -179,12 +179,12 @@ void FileChooser::launchAsync (int flags, std::functionlaunch(); } -FileChooser::Pimpl* FileChooser::createPimpl (int flags, FilePreviewComponent* previewComp) +std::unique_ptr FileChooser::createPimpl (int flags, FilePreviewComponent* previewComp) { results.clear(); @@ -214,10 +214,8 @@ FileChooser::Pimpl* FileChooser::createPimpl (int flags, FilePreviewComponent* p { return showPlatformDialog (*this, flags, previewComp); } - else - { - return new NonNative (*this, flags, previewComp); - } + + return std::make_unique (*this, flags, previewComp); } Array FileChooser::getResults() const noexcept diff --git a/modules/juce_gui_basics/filebrowser/juce_FileChooser.h b/modules/juce_gui_basics/filebrowser/juce_FileChooser.h index e1a50ee98d..c2b25f85aa 100644 --- a/modules/juce_gui_basics/filebrowser/juce_FileChooser.h +++ b/modules/juce_gui_basics/filebrowser/juce_FileChooser.h @@ -325,12 +325,11 @@ private: virtual void runModally() = 0; }; - std::unique_ptr pimpl; + std::shared_ptr pimpl; //============================================================================== - Pimpl* createPimpl (int, FilePreviewComponent*); - static Pimpl* showPlatformDialog (FileChooser&, int, - FilePreviewComponent*); + std::unique_ptr createPimpl (int, FilePreviewComponent*); + static std::unique_ptr showPlatformDialog (FileChooser&, int, FilePreviewComponent*); class NonNative; friend class NonNative; diff --git a/modules/juce_gui_basics/native/juce_android_FileChooser.cpp b/modules/juce_gui_basics/native/juce_android_FileChooser.cpp index 35d7f60e02..8d26cac2c0 100644 --- a/modules/juce_gui_basics/native/juce_android_FileChooser.cpp +++ b/modules/juce_gui_basics/native/juce_android_FileChooser.cpp @@ -219,11 +219,11 @@ private: FileChooser::Native* FileChooser::Native::currentFileChooser = nullptr; -FileChooser::Pimpl* FileChooser::showPlatformDialog (FileChooser& owner, int flags, - FilePreviewComponent*) +std::unique_ptr FileChooser::showPlatformDialog (FileChooser& owner, int flags, + FilePreviewComponent*) { if (FileChooser::Native::currentFileChooser == nullptr) - return new FileChooser::Native (owner, flags); + return std::make_unique (owner, flags); // there can only be one file chooser on Android at a once jassertfalse; diff --git a/modules/juce_gui_basics/native/juce_ios_FileChooser.mm b/modules/juce_gui_basics/native/juce_ios_FileChooser.mm index 9d56b55373..7a45fb90ab 100644 --- a/modules/juce_gui_basics/native/juce_ios_FileChooser.mm +++ b/modules/juce_gui_basics/native/juce_ios_FileChooser.mm @@ -379,10 +379,10 @@ bool FileChooser::isPlatformDialogAvailable() #endif } -FileChooser::Pimpl* FileChooser::showPlatformDialog (FileChooser& owner, int flags, - FilePreviewComponent*) +std::unique_ptr FileChooser::showPlatformDialog (FileChooser& owner, int flags, + FilePreviewComponent*) { - return new FileChooser::Native (owner, flags); + return std::make_unique (owner, flags); } #if JUCE_DEPRECATION_IGNORED diff --git a/modules/juce_gui_basics/native/juce_linux_FileChooser.cpp b/modules/juce_gui_basics/native/juce_linux_FileChooser.cpp index b799ee2e4c..6ead1dfe76 100644 --- a/modules/juce_gui_basics/native/juce_linux_FileChooser.cpp +++ b/modules/juce_gui_basics/native/juce_linux_FileChooser.cpp @@ -256,9 +256,9 @@ bool FileChooser::isPlatformDialogAvailable() #endif } -FileChooser::Pimpl* FileChooser::showPlatformDialog (FileChooser& owner, int flags, FilePreviewComponent*) +std::unique_ptr FileChooser::showPlatformDialog (FileChooser& owner, int flags, FilePreviewComponent*) { - return new Native (owner, flags); + return std::make_unique (owner, flags); } } // namespace juce diff --git a/modules/juce_gui_basics/native/juce_mac_FileChooser.mm b/modules/juce_gui_basics/native/juce_mac_FileChooser.mm index af5d96af71..6b980e335d 100644 --- a/modules/juce_gui_basics/native/juce_mac_FileChooser.mm +++ b/modules/juce_gui_basics/native/juce_mac_FileChooser.mm @@ -377,10 +377,10 @@ private: JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (Native) }; -FileChooser::Pimpl* FileChooser::showPlatformDialog (FileChooser& owner, int flags, - FilePreviewComponent* preview) +std::unique_ptr FileChooser::showPlatformDialog (FileChooser& owner, int flags, + FilePreviewComponent* preview) { - return new FileChooser::Native (owner, flags, preview); + return std::make_unique (owner, flags, preview); } bool FileChooser::isPlatformDialogAvailable() diff --git a/modules/juce_gui_basics/native/juce_win32_FileChooser.cpp b/modules/juce_gui_basics/native/juce_win32_FileChooser.cpp old mode 100644 new mode 100755 index 0c1138a2b0..26bfdc51fc --- a/modules/juce_gui_basics/native/juce_win32_FileChooser.cpp +++ b/modules/juce_gui_basics/native/juce_win32_FileChooser.cpp @@ -26,32 +26,23 @@ namespace juce { -// Win32NativeFileChooser needs to be a reference counted object as there -// is no way for the parent to know when the dialog HWND has actually been -// created without pumping the message thread (which is forbidden when modal -// loops are disabled). However, the HWND pointer is the only way to cancel -// the dialog box. This means that the actual native FileChooser HWND may -// not have been created yet when the user deletes JUCE's FileChooser class. If this -// occurs the Win32NativeFileChooser will still have a reference count of 1 and will -// simply delete itself immediately once the HWND will have been created a while later. -class Win32NativeFileChooser : public ReferenceCountedObject, +class Win32NativeFileChooser : public std::enable_shared_from_this, private Thread { public: - using Ptr = ReferenceCountedObjectPtr; - enum { charsAvailableForResult = 32768 }; Win32NativeFileChooser (Component* parent, int flags, FilePreviewComponent* previewComp, const File& startingFile, const String& titleToUse, const String& filtersToUse) : Thread ("Native Win32 FileChooser"), - owner (parent), title (titleToUse), filtersString (filtersToUse.replaceCharacter (',', ';')), + owner (parent), + title (titleToUse), + filtersString (filtersToUse.replaceCharacter (',', ';')), selectsDirectories ((flags & FileBrowserComponent::canSelectDirectories) != 0), isSave ((flags & FileBrowserComponent::saveMode) != 0), warnAboutOverwrite ((flags & FileBrowserComponent::warnAboutOverwriting) != 0), - selectMultiple ((flags & FileBrowserComponent::canSelectMultipleItems) != 0), - nativeDialogRef (nullptr), shouldCancel (0) + selectMultiple ((flags & FileBrowserComponent::canSelectMultipleItems) != 0) { auto parentDirectory = startingFile.getParentDirectory(); @@ -95,10 +86,7 @@ public: { jassert (! isThreadRunning()); - threadHasReference.reset(); startThread(); - - threadHasReference.wait (-1); } else { @@ -112,7 +100,7 @@ public: ScopedLock lock (deletingDialog); customComponent = nullptr; - shouldCancel.set (1); + shouldCancel = true; if (auto hwnd = nativeDialogRef.get()) EndDialog (hwnd, 0); @@ -151,12 +139,11 @@ private: }; //============================================================================== - Component::SafePointer owner; + const Component::SafePointer owner; String title, filtersString; std::unique_ptr customComponent; String initialPath, returnedString; - WaitableEvent threadHasReference; CriticalSection deletingDialog; bool selectsDirectories, isSave, warnAboutOverwrite, selectMultiple; @@ -164,15 +151,15 @@ private: HeapBlock files; HeapBlock filters; - Atomic nativeDialogRef; - Atomic shouldCancel; + Atomic nativeDialogRef { nullptr }; + bool shouldCancel = false; struct FreeLPWSTR { void operator() (LPWSTR ptr) const noexcept { CoTaskMemFree (ptr); } }; - bool showDialog (IFileDialog& dialog, bool async) const + bool showDialog (IFileDialog& dialog, bool async) { FILEOPENDIALOGOPTIONS flags = {}; @@ -235,7 +222,49 @@ private: if (! selectsDirectories && FAILED (dialog.SetFileTypes (numElementsInArray (spec), spec))) return false; - return dialog.Show (static_cast (async ? nullptr : owner->getWindowHandle())) == S_OK; + struct Events : public ComBaseClassHelper + { + explicit Events (Win32NativeFileChooser& o) : owner (o) {} + + JUCE_COMRESULT OnTypeChange (IFileDialog* d) override + { + HWND hwnd = nullptr; + IUnknown_GetWindow (d, &hwnd); + + ScopedLock lock (owner.deletingDialog); + + if (hwnd != nullptr) + owner.nativeDialogRef = hwnd; + + return owner.shouldCancel ? S_FALSE : S_OK; + } + + JUCE_COMRESULT OnFolderChanging (IFileDialog*, IShellItem*) override { return S_OK; } + JUCE_COMRESULT OnFileOk (IFileDialog*) override { return S_OK; } + JUCE_COMRESULT OnFolderChange (IFileDialog*) override { return S_OK; } + JUCE_COMRESULT OnSelectionChange (IFileDialog*) override { return S_OK; } + JUCE_COMRESULT OnShareViolation (IFileDialog*, IShellItem*, FDE_SHAREVIOLATION_RESPONSE*) override { return S_OK; } + JUCE_COMRESULT OnOverwrite (IFileDialog*, IShellItem*, FDE_OVERWRITE_RESPONSE*) override { return S_OK; } + + Win32NativeFileChooser& owner; + }; + + DWORD cookie = 0; + dialog.Advise (new Events { *this }, &cookie); + + { + ScopedLock lock (deletingDialog); + + if (shouldCancel) + return false; + } + + const auto result = dialog.Show (async ? nullptr : static_cast (owner->getWindowHandle())) == S_OK; + + ScopedLock lock (deletingDialog); + nativeDialogRef = nullptr; + + return result; } //============================================================================== @@ -447,33 +476,21 @@ private: void run() override { - // We use a functor rather than a lambda here because - // we want to move ownership of the Ptr into the function - // object, and C++11 doesn't support general lambda capture - struct AsyncCallback + // IUnknown_GetWindow will only succeed when instantiated in a single-thread apartment + CoInitializeEx (nullptr, COINIT_APARTMENTTHREADED); + + auto resultsCopy = openDialog (true); + auto safeOwner = owner; + std::weak_ptr weakThis = shared_from_this(); + + MessageManager::callAsync ([resultsCopy, safeOwner, weakThis] { - AsyncCallback (Ptr p, Array r) - : ptr (std::move (p)), - results (std::move (r)) {} + if (auto locked = weakThis.lock()) + locked->results = resultsCopy; - void operator()() - { - ptr->results = std::move (results); - - if (ptr->owner != nullptr) - ptr->owner->exitModalState (ptr->results.size() > 0 ? 1 : 0); - } - - Ptr ptr; - Array results; - }; - - // as long as the thread is running, don't delete this class - Ptr safeThis (this); - threadHasReference.signal(); - - auto r = openDialog (true); - MessageManager::callAsync (AsyncCallback (std::move (safeThis), std::move (r))); + if (safeOwner != nullptr) + safeOwner->exitModalState (resultsCopy.size() > 0 ? 1 : 0); + }); } static HashMap& getNativeDialogList() @@ -482,9 +499,9 @@ private: return dialogs; } - static Win32NativeFileChooser* getNativePointerForDialog (HWND hWnd) + static Win32NativeFileChooser* getNativePointerForDialog (HWND hwnd) { - return getNativeDialogList()[hWnd]; + return getNativeDialogList()[hwnd]; } //============================================================================== @@ -552,7 +569,7 @@ private: ScopedLock lock (deletingDialog); getNativeDialogList().set (hdlg, this); - if (shouldCancel.get() != 0) + if (shouldCancel) { EndDialog (hdlg, 0); } @@ -617,7 +634,7 @@ private: { ScopedLock lock (deletingDialog); - if (customComponent != nullptr && shouldCancel.get() == 0) + if (customComponent != nullptr && ! shouldCancel) { if (FilePreviewComponent* comp = dynamic_cast (customComponent->getChildComponent (0))) { @@ -715,14 +732,15 @@ private: JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (Win32NativeFileChooser) }; -class FileChooser::Native : public Component, +class FileChooser::Native : public std::enable_shared_from_this, + public Component, public FileChooser::Pimpl { public: Native (FileChooser& fileChooser, int flags, FilePreviewComponent* previewComp) : owner (fileChooser), - nativeFileChooser (new Win32NativeFileChooser (this, flags, previewComp, fileChooser.startingFile, - fileChooser.title, fileChooser.filters)) + nativeFileChooser (std::make_shared (this, flags, previewComp, fileChooser.startingFile, + fileChooser.title, fileChooser.filters)) { auto mainMon = Desktop::getInstance().getDisplays().getPrimaryDisplay()->userArea; @@ -739,19 +757,17 @@ public: { exitModalState (0); nativeFileChooser->cancel(); - nativeFileChooser = nullptr; } void launch() override { - SafePointer safeThis (this); + std::weak_ptr safeThis = shared_from_this(); - enterModalState (true, ModalCallbackFunction::create ( - [safeThis] (int) - { - if (safeThis != nullptr) - safeThis->owner.finished (safeThis->nativeFileChooser->results); - })); + enterModalState (true, ModalCallbackFunction::create ([safeThis] (int) + { + if (auto locked = safeThis.lock()) + locked->owner.finished (locked->nativeFileChooser->results); + })); nativeFileChooser->open (true); } @@ -783,7 +799,7 @@ public: private: FileChooser& owner; - Win32NativeFileChooser::Ptr nativeFileChooser; + std::shared_ptr nativeFileChooser; }; //============================================================================== @@ -796,10 +812,10 @@ bool FileChooser::isPlatformDialogAvailable() #endif } -FileChooser::Pimpl* FileChooser::showPlatformDialog (FileChooser& owner, int flags, - FilePreviewComponent* preview) +std::unique_ptr FileChooser::showPlatformDialog (FileChooser& owner, int flags, + FilePreviewComponent* preview) { - return new FileChooser::Native (owner, flags, preview); + return std::make_unique (owner, flags, preview); } } // namespace juce