Merge pull request #67 from ryanmcgrath/feat/change-objc-classdec

Feat/change objc classdec

examples/browser/main.rs

@@ -91,7 +91,9 @@ impl Dispatcher for BasicApp {
 #[derive(Default)]
 pub struct WebViewInstance;
 
-impl WebViewDelegate for WebViewInstance {}
+impl WebViewDelegate for WebViewInstance {
+    const NAME: &'static str = "BrowserWebViewDelegate";
+}
 
 struct AppWindow {
     toolbar: Toolbar<BrowserToolbar>,

examples/webview_custom_protocol.rs

@@ -23,6 +23,8 @@ impl AppDelegate for BasicApp {
 pub struct WebViewInstance;
 
 impl WebViewDelegate for WebViewInstance {
+    const NAME: &'static str = "CustomWebViewDelegate";
+
     fn on_custom_protocol_request(&self, path: &str) -> Option<Vec<u8>> {
         let requested_asset_path = path.replace("cacao://", "");
 

src/foundation/class.rs

@@ -1,9 +1,13 @@
+use std::cell::Cell;
+use std::collections::hash_map::DefaultHasher;
 use std::collections::HashMap;
 use std::ffi::CString;
+use std::hash::{Hash, Hasher};
 use std::sync::{Arc, RwLock};
+use std::thread;
+use std::time::Instant;
 
 use lazy_static::lazy_static;
-
 use objc::declare::ClassDecl;
 use objc::runtime::{objc_getClass, Class};
 
@@ -11,98 +15,98 @@ lazy_static! {
     static ref CLASSES: ClassMap = ClassMap::new();
 }
 
+thread_local! {
+    /// A very simple RNG seed that we use in constructing unique subclass names.
+    ///
+    /// Why are we doing this? Mainly because I just don't want to bring in another
+    /// crate for something that can be done like this; we don't need cryptographically
+    /// secure generation or anything fancy, as we're just after a unique dangling bit
+    /// for class names.
+    static RNG_SEED: Cell<u64> = Cell::new({
+        let mut hasher = DefaultHasher::new();
+        Instant::now().hash(&mut hasher);
+        thread::current().id().hash(&mut hasher);
+        let hash = hasher.finish();
+        (hash << 1) | 1
+    });
+}
+
+/// Represents an entry in a `ClassMap`. We store an optional superclass_name for debugging
+/// purposes; it's an `Option` to make the logic of loading a class type where we don't need to
+/// care about the superclass type simpler.
+#[derive(Debug)]
+struct ClassEntry {
+    pub superclass_name: Option<&'static str>,
+    pub ptr: usize
+}
+
+/// Represents a key in a `ClassMap`.
+type ClassKey = (&'static str, Option<&'static str>);
+
 /// A ClassMap is a general cache for our Objective-C class lookup and registration. Rather than
 /// constantly calling into the runtime, we store pointers to Class types here after first lookup
-/// and/or creation. The general store format is (roughly speaking) as follows:
-///
-/// ```ignore
-/// {
-///     "subclass_type": {
-///         "superclass_type": *const Class as usize
-///     }
-/// }
-/// ```
-///
-/// The reasoning behind the double map is that it allows for lookup without allocating a `String`
-/// on each hit; allocations are only required when creating a Class to inject, purely for naming
-/// and debugging reasons.
+/// and/or creation.
 ///
 /// There may be a way to do this without using HashMaps and avoiding the heap, but working and
 /// usable beats ideal for now. Open to suggestions.
 #[derive(Debug)]
-pub(crate) struct ClassMap(RwLock<HashMap<&'static str, HashMap<&'static str, usize>>>);
+pub(crate) struct ClassMap(RwLock<HashMap<ClassKey, ClassEntry>>);
 
 impl ClassMap {
     /// Returns a new ClassMap.
     pub fn new() -> Self {
-        ClassMap(RwLock::new({
-            let mut map = HashMap::new();
-
-            // Top-level classes, like `NSView`, we cache here. The reasoning is that if a subclass
-            // is being created, we can avoid querying the runtime for the superclass - i.e, many
-            // subclasses will have `NSView` as their superclass.
-            map.insert("_supers", HashMap::new());
-
-            map
-        }))
+        ClassMap(RwLock::new(HashMap::new()))
     }
 
-    /// Attempts to load a previously registered subclass.
-    pub fn load_subclass(&self, subclass_name: &'static str, superclass_name: &'static str) -> Option<*const Class> {
-        let reader = self.0.read().unwrap();
-
-        if let Some(inner) = (*reader).get(subclass_name) {
-            if let Some(class) = inner.get(superclass_name) {
-                return Some(*class as *const Class);
-            }
-        }
-
-        None
-    }
-
-    /// Store a newly created subclass type.
-    pub fn store_subclass(&self, subclass_name: &'static str, superclass_name: &'static str, class: *const Class) {
-        let mut writer = self.0.write().unwrap();
-
-        if let Some(map) = (*writer).get_mut(subclass_name) {
-            map.insert(superclass_name, class as usize);
-        } else {
-            let mut map = HashMap::new();
-            map.insert(superclass_name, class as usize);
-            (*writer).insert(subclass_name, map);
-        }
-    }
-
-    /// Attempts to load a Superclass. This first checks for the cached pointer; if not present, it
-    /// will load the superclass from the Objective-C runtime and cache it for future lookup. This
-    /// assumes that the class is one that should *already* and *always* exist in the runtime, and
-    /// by design will panic if it can't load the correct superclass, as that would lead to very
-    /// invalid behavior.
-    pub fn load_superclass(&self, name: &'static str) -> Option<*const Class> {
+    /// Attempts to load a previously registered class.
+    ///
+    /// This checks our internal map first, and then calls out to the Objective-C runtime to ensure
+    /// we're not missing anything.
+    pub fn load(&self, class_name: &'static str, superclass_name: Option<&'static str>) -> Option<*const Class> {
         {
             let reader = self.0.read().unwrap();
-            if let Some(superclass) = (*reader)["_supers"].get(name) {
-                return Some(*superclass as *const Class);
+            if let Some(entry) = (*reader).get(&(class_name, superclass_name)) {
+                let ptr = &entry.ptr;
+                return Some(*ptr as *const Class);
             }
         }
 
-        let objc_superclass_name = CString::new(name).unwrap();
-        let superclass = unsafe { objc_getClass(objc_superclass_name.as_ptr() as *const _) };
+        // If we don't have an entry for the class_name in our internal map, we should still check
+        // if we can load it from the Objective-C runtime directly. The reason we need to do this
+        // is that there's a use-case where someone might have multiple bundles attempting to
+        // use or register the same subclass; Rust doesn't see the same pointers unlike the Objective-C
+        // runtime, and we can wind up in a situation where we're attempting to register a Class
+        // that already exists but we can't see.
+        let objc_class_name = CString::new(class_name).unwrap();
+        let class = unsafe { objc_getClass(objc_class_name.as_ptr() as *const _) };
 
-        // This should not happen, for our use-cases, but it's conceivable that this could actually
+        // This should not happen for our use-cases, but it's conceivable that this could actually
         // be expected, so just return None and let the caller panic if so desired.
-        if superclass.is_null() {
+        if class.is_null() {
             return None;
         }
 
+        // If we got here, then this class exists in the Objective-C runtime but is not known to
+        // us. For consistency's sake, we'll add this to our store and return that.
         {
             let mut writer = self.0.write().unwrap();
-            if let Some(supers) = (*writer).get_mut("_supers") {
-                supers.insert(name, superclass as usize);
-            }
+            writer.insert((class_name, superclass_name), ClassEntry {
+                superclass_name,
+                ptr: class as usize
+            });
         }
 
-        Some(superclass)
+        Some(class)
+    }
+
+    /// Store a newly created subclass type.
+    pub fn store(&self, class_name: &'static str, superclass_name: Option<&'static str>, class: *const Class) {
+        let mut writer = self.0.write().unwrap();
+
+        writer.insert((class_name, superclass_name), ClassEntry {
+            superclass_name,
+            ptr: class as usize
+        });
     }
 }
 
@@ -124,19 +128,37 @@ pub fn load_or_register_class<F>(superclass_name: &'static str, subclass_name: &
 where
     F: Fn(&mut ClassDecl) + 'static
 {
-    if let Some(subclass) = CLASSES.load_subclass(subclass_name, superclass_name) {
+    if let Some(subclass) = CLASSES.load(subclass_name, Some(superclass_name)) {
         return subclass;
     }
 
-    if let Some(superclass) = CLASSES.load_superclass(superclass_name) {
-        let objc_subclass_name = format!("{}_{}", subclass_name, superclass_name);
+    // If we can't find the class anywhere, then we'll attempt to load the superclass and register
+    // our new class type.
+    if let Some(superclass) = CLASSES.load(superclass_name, None) {
+        // When we're generating a new Subclass name, we need to append a random-ish component
+        // due to some oddities that can come up in certain scenarios (e.g, various bundler
+        // situations appear to have odd rules about subclass name usage/registration, this simply
+        // guarantees that we almost always have a unique name to register with the ObjC runtime).
+        //
+        // For more context, see: https://github.com/ryanmcgrath/cacao/issues/63
+        let objc_subclass_name = format!(
+            "{}_{}_{}",
+            subclass_name,
+            superclass_name,
+            RNG_SEED.with(|rng| {
+                rng.set(rng.get().wrapping_add(0xa0761d6478bd642f));
+                let s = rng.get();
+                let t = u128::from(s) * (u128::from(s ^ 0xe7037ed1a0b428db));
+                ((t >> 64) as u64) ^ (t as u64)
+            })
+        );
 
         match ClassDecl::new(&objc_subclass_name, unsafe { &*superclass }) {
             Some(mut decl) => {
                 config(&mut decl);
 
                 let class = decl.register();
-                CLASSES.store_subclass(subclass_name, superclass_name, class);
+                CLASSES.store(subclass_name, Some(superclass_name), class);
                 return class;
             },
 

src/webview/class.rs

@@ -12,7 +12,7 @@ use objc::declare::ClassDecl;
 use objc::runtime::{Class, Object, Sel};
 use objc::{class, msg_send, sel, sel_impl};
 
-use crate::foundation::{id, nil, NSArray, NSInteger, NSString, NO, YES};
+use crate::foundation::{id, load_or_register_class, nil, NSArray, NSInteger, NSString, NO, YES};
 use crate::webview::actions::{NavigationAction, NavigationResponse};
 use crate::webview::{mimetype::MimeType, WebViewDelegate, WEBVIEW_DELEGATE_PTR}; //, OpenPanelParameters};
                                                                                  //use crate::webview::enums::{NavigationPolicy, NavigationResponsePolicy};
@@ -170,29 +170,14 @@ extern "C" fn handle_download<T: WebViewDelegate>(this: &Object, _: Sel, downloa
 /// both a subclass of `NSViewController` and a delegate of the held `WKWebView` (for the various
 /// varieties of delegates needed there).
 pub fn register_webview_class() -> *const Class {
-    static mut VIEW_CLASS: *const Class = 0 as *const Class;
-    static INIT: Once = Once::new();
-
-    INIT.call_once(|| unsafe {
-        let superclass = class!(WKWebView);
-        let decl = ClassDecl::new("RSTWebView", superclass).unwrap();
-        VIEW_CLASS = decl.register();
-    });
-
-    unsafe { VIEW_CLASS }
+    load_or_register_class("WKWebView", "CacaoWebView", |decl| unsafe {})
 }
 
 /// Registers an `NSViewController` that we effectively turn into a `WebViewController`. Acts as
 /// both a subclass of `NSViewController` and a delegate of the held `WKWebView` (for the various
 /// varieties of delegates needed there).
-pub fn register_webview_delegate_class<T: WebViewDelegate>() -> *const Class {
-    static mut VIEW_CLASS: *const Class = 0 as *const Class;
-    static INIT: Once = Once::new();
-
-    INIT.call_once(|| unsafe {
-        let superclass = class!(NSObject);
-        let mut decl = ClassDecl::new("RSTWebViewDelegate", superclass).unwrap();
-
+pub fn register_webview_delegate_class<T: WebViewDelegate>(instance: &T) -> *const Class {
+    load_or_register_class("NSObject", instance.subclass_name(), |decl| unsafe {
         decl.add_ivar::<usize>(WEBVIEW_DELEGATE_PTR);
 
         // WKNavigationDelegate
@@ -239,9 +224,5 @@ pub fn register_webview_delegate_class<T: WebViewDelegate>() -> *const Class {
             sel!(_download:decideDestinationWithSuggestedFilename:completionHandler:),
             handle_download::<T> as extern "C" fn(&Object, _, id, id, usize)
         );
-
-        VIEW_CLASS = decl.register();
-    });
-
-    unsafe { VIEW_CLASS }
+    })
 }

src/webview/mod.rs

@@ -230,10 +230,11 @@ where
     /// Initializes a new WebView with a given `WebViewDelegate`. This enables you to respond to events
     /// and customize the view as a module, similar to class-based systems.
     pub fn with(config: WebViewConfig, delegate: T) -> WebView<T> {
+        let delegate_class = register_webview_delegate_class(&delegate);
         let mut delegate = Box::new(delegate);
 
         let objc_delegate = unsafe {
-            let objc_delegate: id = msg_send![register_webview_delegate_class::<T>(), new];
+            let objc_delegate: id = msg_send![delegate_class, new];
             let ptr: *const T = &*delegate;
             (&mut *objc_delegate).set_ivar(WEBVIEW_DELEGATE_PTR, ptr as usize);
             ShareId::from_ptr(objc_delegate)

src/webview/traits.rs

@@ -8,6 +8,17 @@ use crate::webview::WebView;
 
 /// You can implement this on structs to handle callbacks from the underlying `WKWebView`.
 pub trait WebViewDelegate {
+    /// Used to cache subclass creations on the Objective-C side.
+    /// You can just set this to be the name of your delegate type. This
+    /// value *must* be unique per-type.
+    const NAME: &'static str;
+
+    /// You should rarely (read: probably never) need to implement this yourself.
+    /// It simply acts as a getter for the associated `NAME` const on this trait.
+    fn subclass_name(&self) -> &'static str {
+        Self::NAME
+    }
+
     /// Called when the View is ready to work with. You're passed a `ViewHandle` - this is safe to
     /// store and use repeatedly, but it's not thread safe - any UI calls must be made from the
     /// main thread!