WebView does not work with ARC enabled

[jdv85]

Hi there and thanks for the great work!

In experimenting with WebView, I found that it does not work when used from Objective-C++ with automatic reference counting (ARC) enabled.

There are two problems:

Converting Objective-C pointers to and from C++ needs to use __bridge type casts

This results in 4 compilation errors. As an example, line 361 in choc_VewView is:

        objc_setAssociatedObject (delegate, "choc_webview", (id) this, OBJC_ASSOCIATION_ASSIGN);

To work with ARC enabled it needs to be:

        objc_setAssociatedObject (delegate, "choc_webview", (__bridge id) this, OBJC_ASSOCIATION_ASSIGN);

-[NSAutoreleasePool new] cannot be used with ARC enabled

This causes an Objective-C exception to be thrown when constructing the WebView:

Exception NSException * "*** -[NSAutoreleasePool retain]: Cannot retain an autorelease pool" 0x0000600003ecb7e0

According to https://clang.llvm.org/docs/AutomaticReferenceCounting.html#autoreleasepool, when ARC is enabled, referencing the NSAutoreleasePool class is not allowed.

Making this work would require replacing the use of choc::objc::AutoReleasePool with @autorelease {} blocks. I don't think it's a massive undertaking but I understand if it clashes with the general CHOC style. As an example, the choc::ui::WebView::Pimpl constructor would change from:

    Pimpl (WebView& v, const Options& optionsToUse)
        : owner (v), options (optionsToUse)
    {
        using namespace choc::objc;
        AutoReleasePool autoreleasePool;

        id config = call<id> (getClass ("WKWebViewConfiguration"), "new");
...

To:

    Pimpl (WebView& v, const Options& optionsToUse)
        : owner (v), options (optionsToUse)
    @autoreleasepool {

        id config = call<id> (getClass ("WKWebViewConfiguration"), "new");
...

Alternative solution

I fully understand if this is not something you consider worth fixing. I can work around the problem by disabling ARC for the file that's using the WebView.

If ARC is not supported by CHOC, I would suggest documenting this, for example by adding a compile check along the lines of this:

static_assert(!__has_feature(objc_arc));

[julianstorer]

Thanks for the heads-up.. I've had a shot at making it support both ARC and non-ARC builds. Would appreciate you giving it a try on the develop branch and see if it works for you.

I'm not a fan of ARC because it all seems very hazy in terms of when ref counts are incremented, so I don't feel confident that this code is leak-free..

[jdv85]

@julianstorer, thanks for the quick fix!

In testing it out, I found that choc::ui::DesktopWindow also needs a few CHOC_OBJC_CAST_BRIDGED casts. (I wasn't using it in my previous tests).

I also got an EXC_BAD_ACCESS (code=1, address=0x0) in the WebView constructor. The problem is four lines like this one:

        call<id> (prefs, "setValue:forKey:", getNSNumberBool (true), getNSString ("fullScreenEnabled"));

-[setValue:forKey:] returnsvoid. Presumably the accidental cast to id makes ARC attempt to retain and autorelease something that isn't an Objective-C object.

Here's a diff that makes everything work for me when used in an .mm file:

diff --git a/gui/choc_DesktopWindow.h b/gui/choc_DesktopWindow.h
index 815550f..845d282 100644
--- a/gui/choc_DesktopWindow.h
+++ b/gui/choc_DesktopWindow.h
@@ -281,7 +281,7 @@ struct DesktopWindow::Pimpl
                            NSWindowStyleMaskTitled, NSBackingStoreBuffered, (int) 0);

         delegate = createDelegate();
-        objc_setAssociatedObject (delegate, "choc_window", (id) this, OBJC_ASSOCIATION_ASSIGN);
+        objc_setAssociatedObject (delegate, "choc_window", (CHOC_OBJC_CAST_BRIDGED id) this, OBJC_ASSOCIATION_ASSIGN);
         call<void> (window, "setDelegate:", delegate);
         CHOC_AUTORELEASE_END
     }
@@ -295,7 +295,7 @@ struct DesktopWindow::Pimpl
         CHOC_AUTORELEASE_END
     }

-    void* getWindowHandle() const     { return (void*) window; }
+    void* getWindowHandle() const     { return (CHOC_OBJC_CAST_BRIDGED void*) window; }

     void setWindowTitle (const std::string& newTitle)
     {
@@ -307,7 +307,7 @@ struct DesktopWindow::Pimpl
     void setContent (void* view)
     {
         CHOC_AUTORELEASE_BEGIN
-        objc::call<void> (window, "setContentView:", (id) view);
+        objc::call<void> (window, "setContentView:", (CHOC_OBJC_CAST_BRIDGED id) view);
         CHOC_AUTORELEASE_END
     }

@@ -361,7 +361,7 @@ struct DesktopWindow::Pimpl

     static Pimpl& getPimplFromContext (id self)
     {
-        auto view = (Pimpl*) objc_getAssociatedObject (self, "choc_window");
+        auto view = (CHOC_OBJC_CAST_BRIDGED Pimpl*) objc_getAssociatedObject (self, "choc_window");
         CHOC_ASSERT (view != nullptr);
         return *view;
     }
diff --git a/gui/choc_WebView.h b/gui/choc_WebView.h
index 343686a..5abf98b 100644
--- a/gui/choc_WebView.h
+++ b/gui/choc_WebView.h
@@ -351,12 +351,12 @@ struct choc::ui::WebView::Pimpl
         id config = call<id> (getClass ("WKWebViewConfiguration"), "new");

         id prefs = call<id> (config, "preferences");
-        call<id> (prefs, "setValue:forKey:", getNSNumberBool (true), getNSString ("fullScreenEnabled"));
-        call<id> (prefs, "setValue:forKey:", getNSNumberBool (true), getNSString ("DOMPasteAllowed"));
-        call<id> (prefs, "setValue:forKey:", getNSNumberBool (true), getNSString ("javaScriptCanAccessClipboard"));
+        call<void> (prefs, "setValue:forKey:", getNSNumberBool (true), getNSString ("fullScreenEnabled"));
+        call<void> (prefs, "setValue:forKey:", getNSNumberBool (true), getNSString ("DOMPasteAllowed"));
+        call<void> (prefs, "setValue:forKey:", getNSNumberBool (true), getNSString ("javaScriptCanAccessClipboard"));

         if (options.enableDebugMode)
-            call<id> (prefs, "setValue:forKey:", getNSNumberBool (true), getNSString ("developerExtrasEnabled"));
+            call<void> (prefs, "setValue:forKey:", getNSNumberBool (true), getNSString ("developerExtrasEnabled"));

         delegate = createDelegate();
         objc_setAssociatedObject (delegate, "choc_webview", (CHOC_OBJC_CAST_BRIDGED id) this, OBJC_ASSOCIATION_ASSIGN);

I have tested for leaks with ARC enabled by creating a DesktopWindow and a WebView and tearing it all down on window close. The "Leaks" Instrument did not find any leaked Objective-C objects. To double check my method, I added an extra retain on the WebView instance variable, and then did see a leaked WKUserContentController instance.

Let me know if you need me to do any additional testing.

[julianstorer]

That's great, thanks for those changes, I'll get that in there. (I'm a bit puzzled about why it didn't complain about those lines when I tried it with ARC, but hey ho..)