From 17b870885230e7af04a8b34cc0d51b8086340dbf Mon Sep 17 00:00:00 2001 From: reuk Date: Thu, 21 Oct 2021 17:44:07 +0100 Subject: [PATCH] WebBrowserComponent: Avoid leaking browsers on macOS --- .../juce_core/native/juce_mac_ObjCHelpers.h | 60 ++++++++++ .../native/juce_mac_WebBrowserComponent.mm | 110 ++++++++---------- 2 files changed, 108 insertions(+), 62 deletions(-) diff --git a/modules/juce_core/native/juce_mac_ObjCHelpers.h b/modules/juce_core/native/juce_mac_ObjCHelpers.h index fd91a6a7a1..1847e8c7c0 100644 --- a/modules/juce_core/native/juce_mac_ObjCHelpers.h +++ b/modules/juce_core/native/juce_mac_ObjCHelpers.h @@ -252,6 +252,66 @@ struct NSObjectDeleter template using NSUniquePtr = std::unique_ptr; +/* This has very similar semantics to NSUniquePtr, with the main difference that it doesn't + automatically add a pointer to the managed type. This makes it possible to declare + scoped handles to id or block types. +*/ +template +class ObjCObjectHandle +{ +public: + ObjCObjectHandle() = default; + + // Note that this does *not* retain the argument. + explicit ObjCObjectHandle (T ptr) : item (ptr) {} + + ~ObjCObjectHandle() noexcept { reset(); } + + ObjCObjectHandle (const ObjCObjectHandle& other) + : item (other.item) + { + if (item != nullptr) + [item retain]; + } + + ObjCObjectHandle& operator= (const ObjCObjectHandle& other) + { + auto copy = other; + swap (copy); + return *this; + } + + ObjCObjectHandle (ObjCObjectHandle&& other) noexcept { swap (other); } + + ObjCObjectHandle& operator= (ObjCObjectHandle&& other) noexcept + { + reset(); + swap (other); + return *this; + } + + // Note that this does *not* retain the argument. + void reset (T ptr) { *this = ObjCObjectHandle { ptr }; } + + T get() const { return item; } + + void reset() + { + if (item != nullptr) + [item release]; + + item = {}; + } + + bool operator== (const ObjCObjectHandle& other) const { return item == other.item; } + bool operator!= (const ObjCObjectHandle& other) const { return ! (*this == other); } + +private: + void swap (ObjCObjectHandle& other) noexcept { std::swap (other.item, item); } + + T item{}; +}; + //============================================================================== template inline Type getIvar (id self, const char* name) diff --git a/modules/juce_gui_extra/native/juce_mac_WebBrowserComponent.mm b/modules/juce_gui_extra/native/juce_mac_WebBrowserComponent.mm index bd179c928a..bef333f3a5 100644 --- a/modules/juce_gui_extra/native/juce_mac_WebBrowserComponent.mm +++ b/modules/juce_gui_extra/native/juce_mac_WebBrowserComponent.mm @@ -239,16 +239,11 @@ private: DeletedFileChooserWrapper (std::unique_ptr fc, id rl) : chooser (std::move (fc)), listener (rl) { - [listener retain]; - } - - ~DeletedFileChooserWrapper() - { - [listener release]; + [listener.get() retain]; } std::unique_ptr chooser; - id listener; + ObjCObjectHandle> listener; }; auto chooser = std::make_unique (TRANS("Select the file you want to upload..."), @@ -261,7 +256,7 @@ private: wrapper->chooser->launchAsync (flags, [wrapper] (const FileChooser&) { for (auto& f : wrapper->chooser->getResults()) - [wrapper->listener chooseFilename: juceStringToNS (f.getFullPathName())]; + [wrapper->listener.get() chooseFilename: juceStringToNS (f.getFullPathName())]; delete wrapper; }); @@ -358,13 +353,12 @@ private: DeletedFileChooserWrapper (std::unique_ptr fc, CompletionHandlerType h) : chooser (std::move (fc)), handler (h) { - [handler retain]; + [handler.get() retain]; } ~DeletedFileChooserWrapper() { callHandler (nullptr); - [handler release]; } void callHandler (NSArray* urls) @@ -372,14 +366,14 @@ private: if (handlerCalled) return; - handler (urls); + handler.get() (urls); handlerCalled = true; } std::unique_ptr chooser; private: - CompletionHandlerType handler; + ObjCObjectHandle handler; bool handlerCalled = false; }; @@ -436,28 +430,25 @@ public: WebViewImpl (WebBrowserComponent* owner) { static WebViewKeyEquivalentResponder webviewClass; - webView = (WebView*) webviewClass.createInstance(); - webView = [webView initWithFrame: NSMakeRect (0, 0, 100.0f, 100.0f) - frameName: nsEmptyString() - groupName: nsEmptyString()]; + webView.reset ([webviewClass.createInstance() initWithFrame: NSMakeRect (0, 0, 100.0f, 100.0f) + frameName: nsEmptyString() + groupName: nsEmptyString()]); static DownloadClickDetectorClass cls; - clickListener = [cls.createInstance() init]; - DownloadClickDetectorClass::setOwner (clickListener, owner); + clickListener.reset ([cls.createInstance() init]); + DownloadClickDetectorClass::setOwner (clickListener.get(), owner); - [webView setPolicyDelegate: clickListener]; - [webView setFrameLoadDelegate: clickListener]; - [webView setUIDelegate: clickListener]; + [webView.get() setPolicyDelegate: clickListener.get()]; + [webView.get() setFrameLoadDelegate: clickListener.get()]; + [webView.get() setUIDelegate: clickListener.get()]; } ~WebViewImpl() override { - [webView setPolicyDelegate: nil]; - [webView setFrameLoadDelegate: nil]; - [webView setUIDelegate: nil]; - - [clickListener release]; + [webView.get() setPolicyDelegate: nil]; + [webView.get() setFrameLoadDelegate: nil]; + [webView.get() setUIDelegate: nil]; } void goToURL (const String& url, @@ -466,7 +457,7 @@ public: { if (url.trimStart().startsWithIgnoreCase ("javascript:")) { - [webView stringByEvaluatingJavaScriptFromString: juceStringToNS (url.fromFirstOccurrenceOf (":", false, false))]; + [webView.get() stringByEvaluatingJavaScriptFromString: juceStringToNS (url.fromFirstOccurrenceOf (":", false, false))]; return; } @@ -490,30 +481,30 @@ public: }; if (NSMutableURLRequest* request = getRequest()) - [[webView mainFrame] loadRequest: request]; + [[webView.get() mainFrame] loadRequest: request]; } - void goBack() override { [webView goBack]; } - void goForward() override { [webView goForward]; } + void goBack() override { [webView.get() goBack]; } + void goForward() override { [webView.get() goForward]; } - void stop() override { [webView stopLoading: nil]; } - void refresh() override { [webView reload: nil]; } + void stop() override { [webView.get() stopLoading: nil]; } + void refresh() override { [webView.get() reload: nil]; } - id getWebView() override { return webView; } + id getWebView() override { return webView.get(); } void mouseMove (const MouseEvent&) { JUCE_BEGIN_IGNORE_WARNINGS_GCC_LIKE ("-Wundeclared-selector") // WebKit doesn't capture mouse-moves itself, so it seems the only way to make // them work is to push them via this non-public method.. - if ([webView respondsToSelector: @selector (_updateMouseoverWithFakeEvent)]) - [webView performSelector: @selector (_updateMouseoverWithFakeEvent)]; + if ([webView.get() respondsToSelector: @selector (_updateMouseoverWithFakeEvent)]) + [webView.get() performSelector: @selector (_updateMouseoverWithFakeEvent)]; JUCE_END_IGNORE_WARNINGS_GCC_LIKE } private: - WebView* webView = nil; - id clickListener; + ObjCObjectHandle webView; + ObjCObjectHandle clickListener; }; JUCE_END_IGNORE_WARNINGS_GCC_LIKE #endif @@ -525,27 +516,24 @@ public: { #if JUCE_MAC static WebViewKeyEquivalentResponder webviewClass; - webView = (WKWebView*) webviewClass.createInstance(); - webView = [webView initWithFrame: NSMakeRect (0, 0, 100.0f, 100.0f)]; + webView.reset ([webviewClass.createInstance() initWithFrame: NSMakeRect (0, 0, 100.0f, 100.0f)]); #else - webView = [[WKWebView alloc] initWithFrame: CGRectMake (0, 0, 100.0f, 100.0f)]; + webView.reset ([[WKWebView alloc] initWithFrame: CGRectMake (0, 0, 100.0f, 100.0f)]); #endif static WebViewDelegateClass cls; - webViewDelegate = [cls.createInstance() init]; - WebViewDelegateClass::setOwner (webViewDelegate, owner); + webViewDelegate.reset ([cls.createInstance() init]); + WebViewDelegateClass::setOwner (webViewDelegate.get(), owner); - [webView setNavigationDelegate: webViewDelegate]; - [webView setUIDelegate: webViewDelegate]; + [webView.get() setNavigationDelegate: webViewDelegate.get()]; + [webView.get() setUIDelegate: webViewDelegate.get()]; } ~WKWebViewImpl() override { - [webView setNavigationDelegate: nil]; - [webView setUIDelegate: nil]; - - [webViewDelegate release]; + [webView.get() setNavigationDelegate: nil]; + [webView.get() setUIDelegate: nil]; } void goToURL (const String& url, @@ -556,8 +544,8 @@ public: if (trimmed.startsWithIgnoreCase ("javascript:")) { - [webView evaluateJavaScript: juceStringToNS (url.fromFirstOccurrenceOf (":", false, false)) - completionHandler: nil]; + [webView.get() evaluateJavaScript: juceStringToNS (url.fromFirstOccurrenceOf (":", false, false)) + completionHandler: nil]; return; } @@ -569,25 +557,25 @@ public: auto file = URL (url).getLocalFile(); if (NSURL* nsUrl = [NSURL fileURLWithPath: juceStringToNS (file.getFullPathName())]) - [webView loadFileURL: appendParametersToFileURL (url, nsUrl) allowingReadAccessToURL: nsUrl]; + [webView.get() loadFileURL: appendParametersToFileURL (url, nsUrl) allowingReadAccessToURL: nsUrl]; } else if (NSMutableURLRequest* request = getRequestForURL (url, headers, postData)) { - [webView loadRequest: request]; + [webView.get() loadRequest: request]; } } - void goBack() override { [webView goBack]; } - void goForward() override { [webView goForward]; } + void goBack() override { [webView.get() goBack]; } + void goForward() override { [webView.get() goForward]; } - void stop() override { [webView stopLoading]; } - void refresh() override { [webView reload]; } + void stop() override { [webView.get() stopLoading]; } + void refresh() override { [webView.get() reload]; } - id getWebView() override { return webView; } + id getWebView() override { return webView.get(); } private: - WKWebView* webView = nil; - id webViewDelegate; + ObjCObjectHandle webView; + ObjCObjectHandle webViewDelegate; }; //============================================================================== @@ -643,9 +631,7 @@ WebBrowserComponent::WebBrowserComponent (bool unloadWhenHidden) addAndMakeVisible (browser.get()); } -WebBrowserComponent::~WebBrowserComponent() -{ -} +WebBrowserComponent::~WebBrowserComponent() = default; //============================================================================== void WebBrowserComponent::goToURL (const String& url,