Warnings/imports/unused code cleanup, rework some enums that seemed a bit wonky in hindsight, set WKWebView download delegate pieces as a feature flag since it's technically a private API

.gitignore

@@ -1,2 +1,4 @@
 target
 Cargo.lock
+placeholder
+placeholder2

appkit/Cargo.toml

@@ -16,3 +16,6 @@ lazy_static = "1"
 objc = "0.2.7"
 objc_id = "0.1.1"
 uuid = { version = "0.8", features = ["v4"] }
+
+[features]
+enable-webview-downloading = []

appkit/src/app/events.rs

@@ -2,25 +2,13 @@
 //!
 //! Now, I know what you're thinking: this is dumb.
 //!
-//! And sure, maybe. But if you've ever opened Xcode and wondered why the hell
-//! you have a xib/nib in your macOS project, it's (partly) because *that* handles
-//! the NSMenu architecture for you... an architecture that, supposedly, is one of the
-//! last Carbon pieces still laying around.
-//!
-//! And I gotta be honest, I ain't about the xib/nib life. SwiftUI will hopefully clear
-//! that mess up one day, but in the meantime, we'll do this.
-//!
-//! Now, what we're *actually* doing here is relatively plain - on certain key events,
-//! we want to make sure cut/copy/paste/etc are sent down the event chain. Usually, the
-//! xib/nib stuff handles this for you... but this'll mostly do the same.
+//! However, there are rare cases where this is beneficial, and by default we're doing nothing...
+//! so consider this a placeholder that we might use in the future for certain things.
 
 use std::sync::Once;
 
-use cocoa::base::{id, nil};
-
 use objc::declare::ClassDecl;
 use objc::runtime::Class;
-use objc::{class, msg_send, sel, sel_impl};
 
 /// Used for injecting a custom NSApplication. Currently does nothing.
 pub(crate) fn register_app_class() -> *const Class {

appkit/src/app/mod.rs

@@ -4,7 +4,6 @@
 use std::sync::Once;
 
 use cocoa::base::{id, nil};
-use cocoa::foundation::NSString;
 use cocoa::appkit::{NSRunningApplication};
 
 use objc_id::Id;
@@ -25,7 +24,7 @@ pub trait AppDelegate {
     fn did_finish_launching(&self) {}
     fn did_become_active(&self) {}
 
-    fn on_message(&self, message: Self::Message) {}
+    fn on_message(&self, _message: Self::Message) {}
 }
 
 /// A wrapper for `NSApplication`. It holds (retains) pointers for the Objective-C runtime, 

appkit/src/events.rs

@@ -1,27 +1,41 @@
 //! Hoists some type definitions in a way that I personally find cleaner than what's in the Servo
 //! code.
 
-#[allow(non_upper_case_globals, non_snake_case)]
-pub mod NSEventModifierFlag {
 use cocoa::foundation::NSUInteger;
 
-    /// Indicates the Caps Lock key has been pressed.
-    pub const CapsLock: NSUInteger = 1 << 16;
-
-    /// Indicates the Control key has been pressed.
-    pub const Control: NSUInteger = 1 << 18;
-
-    /// Indicates the Option key has been pressed.
-    pub const Option: NSUInteger = 1 << 19;
-
-    /// Indicates the Command key has been pressed.
-    pub const Command: NSUInteger = 1 << 20;
-
-    /// Indicates device-independent modifier flags are in play.
-    pub const DeviceIndependentFlagsMask: NSUInteger = 0xffff0000;
+#[derive(Clone, Copy, Debug)]
+pub enum EventModifierFlag {
+    CapsLock,
+    Control,
+    Option,
+    Command,
+    DeviceIndependentFlagsMask
 }
 
-#[allow(non_upper_case_globals, non_snake_case)]
-mod NSEventType {
-    pub const KeyDown: usize = 10;
+impl From<EventModifierFlag> for NSUInteger {
+    fn from(flag: EventModifierFlag) -> NSUInteger {
+        match flag {
+            EventModifierFlag::CapsLock => 1 << 16,
+            EventModifierFlag::Control => 1 << 18,
+            EventModifierFlag::Option => 1 << 19,
+            EventModifierFlag::Command => 1 << 20,
+            EventModifierFlag::DeviceIndependentFlagsMask => 0xffff0000
+        }
+    }
+}
+
+impl From<&EventModifierFlag> for NSUInteger {
+    fn from(flag: &EventModifierFlag) -> NSUInteger {
+        match flag {
+            EventModifierFlag::CapsLock => 1 << 16,
+            EventModifierFlag::Control => 1 << 18,
+            EventModifierFlag::Option => 1 << 19,
+            EventModifierFlag::Command => 1 << 20,
+            EventModifierFlag::DeviceIndependentFlagsMask => 0xffff0000
+        }
+    }
+}
+
+pub enum EventType {
+    KeyDown
 }

appkit/src/file_panel/save.rs

@@ -4,14 +4,13 @@
 
 use block::ConcreteBlock;
 
-use cocoa::base::{id, nil, YES, NO, BOOL};
-use cocoa::foundation::{NSInteger, NSUInteger, NSString};
+use cocoa::base::{id, nil, YES, NO};
+use cocoa::foundation::{NSInteger, NSString};
 
 use objc::{class, msg_send, sel, sel_impl};
 use objc::runtime::Object;
 use objc_id::ShareId;
 
-use crate::file_panel::enums::ModalResponse;
 use crate::utils::str_from;
 
 #[derive(Debug)]

appkit/src/file_panel/select.rs

@@ -4,8 +4,8 @@
 
 use block::ConcreteBlock;
 
-use cocoa::base::{id, nil, YES, NO, BOOL};
-use cocoa::foundation::{NSInteger, NSUInteger};
+use cocoa::base::{id, YES, NO};
+use cocoa::foundation::NSInteger;
 
 use objc::{class, msg_send, sel, sel_impl};
 use objc::runtime::Object;

appkit/src/file_panel/traits.rs

@@ -4,18 +4,18 @@
 pub trait OpenSaveController {
     /// Called when the user has entered a filename (typically, during saving). `confirmed`
     /// indicates whether or not they hit the save button.
-    fn user_entered_filename(&self, filename: &str, confirmed: bool) {}
+    fn user_entered_filename(&self, _filename: &str, _confirmed: bool) {}
 
     /// Notifies you that the panel selection changed.
     fn panel_selection_did_change(&self) {}
 
     /// Notifies you that the user changed directories.
-    fn did_change_to_directory(&self, url: &str) {}
+    fn did_change_to_directory(&self, _url: &str) {}
 
     /// Notifies you that the Save panel is about to expand or collapse because the user 
     /// clicked the disclosure triangle that displays or hides the file browser.
-    fn will_expand(&self, expanding: bool) {}
+    fn will_expand(&self, _expanding: bool) {}
 
     /// Determine whether the specified URL should be enabled in the Open panel.
-    fn should_enable_url(&self, url: &str) -> bool { true }
+    fn should_enable_url(&self, _url: &str) -> bool { true }
 }

appkit/src/menu/item.rs

@@ -9,10 +9,15 @@ use objc::{class, msg_send, sel, sel_impl};
 use objc::runtime::{Object, Sel};
 use objc_id::ShareId;
 
-use crate::events::NSEventModifierFlag;
+use crate::events::EventModifierFlag;
 
 /// Internal method (shorthand) for generating `NSMenuItem` holders.
-fn make_menu_item(title: &str, key: Option<&str>, action: Option<Sel>, modifier: Option<NSUInteger>) -> MenuItem {
+fn make_menu_item(
+    title: &str,
+    key: Option<&str>,
+    action: Option<Sel>,
+    modifiers: Option<&[EventModifierFlag]>
+) -> MenuItem {
     unsafe {
         let cls = class!(NSMenuItem);
         let alloc: id = msg_send![cls, alloc];
@@ -29,9 +34,16 @@ fn make_menu_item(title: &str, key: Option<&str>, action: Option<Sel>, modifier:
             None => msg_send![alloc, initWithTitle:title action:nil keyEquivalent:key]
         });
 
-        if let Some(modifier) = modifier {
-            let _: () = msg_send![&*item, setKeyEquivalentModifierMask:modifier];
-        };
+        if let Some(modifiers) = modifiers {
+            let mut key_mask: NSUInteger = 0;
+
+            for modifier in modifiers {
+                let y: NSUInteger = modifier.into();
+                key_mask = key_mask | y;
+            }
+
+            let _: () = msg_send![&*item, setKeyEquivalentModifierMask:key_mask];
+        }
 
         MenuItem::Action(item)
     }
@@ -108,7 +120,7 @@ impl MenuItem {
             "Hide Others",
             Some("h"),
             Some(sel!(hide:)),
-            Some(NSEventModifierFlag::Command | NSEventModifierFlag::Option)
+            Some(&[EventModifierFlag::Command, EventModifierFlag::Option])
         )
     }
 
@@ -143,7 +155,7 @@ impl MenuItem {
             "Enter Full Screen",
             Some("f"),
             Some(sel!(toggleFullScreen:)),
-            Some(NSEventModifierFlag::Command | NSEventModifierFlag::Control)
+            Some(&[EventModifierFlag::Command, EventModifierFlag::Control])
         )
     }
 

appkit/src/menu/menu.rs

@@ -1,6 +1,6 @@
 //! Wraps NSMenu and handles instrumenting necessary delegate pieces.
 
-use cocoa::base::{id, nil, YES};
+use cocoa::base::{id, nil};
 use cocoa::foundation::NSString;
 
 use objc_id::Id;

appkit/src/networking/mod.rs

@@ -4,7 +4,7 @@
 use cocoa::base::id;
 use objc_id::Id;
 
-use objc::{class, msg_send, sel, sel_impl};
+use objc::{msg_send, sel, sel_impl};
 use objc::runtime::Object;
 
 use crate::utils::str_from;

appkit/src/view/class.rs

@@ -9,7 +9,7 @@
 
 use std::sync::Once;
 
-use cocoa::base::{id, nil, YES, NO};
+use cocoa::base::{id, nil, YES};
 
 use objc::declare::ClassDecl;
 use objc::runtime::{Class, Object, Sel, BOOL};

appkit/src/webview/action.rs

@@ -1,10 +1,10 @@
 //! Implements wrappers around `WKNavigationAction` and `WKNavigationActionPolicy`.
 
-use cocoa::base::{id, nil, YES, NO};
+use cocoa::base::{id, YES, NO};
 use cocoa::foundation::NSInteger;
 
-use objc::runtime::{Class, Object, Sel, BOOL};
-use objc::{class, msg_send, sel, sel_impl};
+use objc::runtime::BOOL;
+use objc::{msg_send, sel, sel_impl};
 
 use crate::networking::URLRequest;
 
@@ -73,8 +73,8 @@ impl NavigationResponse {
     pub fn new(response: id) -> Self {
         NavigationResponse {
             can_show_mime_type: unsafe {
-                let canShow: BOOL = msg_send![response, canShowMIMEType];
-                if canShow == YES { true } else { false }
+                let can_show: BOOL = msg_send![response, canShowMIMEType];
+                if can_show == YES { true } else { false }
             }
         }
     }

appkit/src/webview/config.rs

@@ -1,15 +1,13 @@
 //! A wrapper for `WKWebViewConfiguration`. It aims to (mostly) cover
 //! the important pieces of configuring and updating a WebView configuration.
 
-use cocoa::base::{id, nil, YES, NO};
+use cocoa::base::{id, nil, YES};
 use cocoa::foundation::NSString;
 
 use objc_id::Id;
 use objc::runtime::Object;
 use objc::{class, msg_send, sel, sel_impl};
 
-use crate::webview::process_pool::register_process_pool;
-
 /// Whether a script should be injected at the start or end of the document load.
 pub enum InjectAt {
     Start = 0,

appkit/src/webview/controller.rs

@@ -11,12 +11,12 @@ use cocoa::base::{id, nil, YES, NO};
 use cocoa::foundation::{NSRect, NSPoint, NSSize, NSString, NSArray, NSInteger};
 
 use objc::declare::ClassDecl;
-use objc::runtime::{Class, Object, Sel, BOOL};
+use objc::runtime::{Class, Object, Sel};
 use objc::{class, msg_send, sel, sel_impl};
 
 use crate::ViewController;
 use crate::view::class::register_view_class;
-use crate::webview::action::{NavigationAction, NavigationResponse, OpenPanelParameters};
+use crate::webview::action::{NavigationAction, NavigationResponse};
 use crate::webview::{WEBVIEW_VAR, WEBVIEW_CONFIG_VAR, WEBVIEW_CONTROLLER_PTR};
 use crate::webview::traits::WebViewController;
 use crate::utils::str_from;
@@ -136,12 +136,9 @@ extern fn decide_policy_for_response<T: WebViewController + 'static>(this: &Obje
     };
 
     let response = NavigationResponse::new(response);
-    webview.policy_for_navigation_response(response, |policy| {
-        // This is very sketch and should be heavily checked. :|
-        unsafe {
+    webview.policy_for_navigation_response(response, |policy| unsafe {
         let handler = handler as *const Block<(NSInteger,), c_void>;
         (*handler).call((policy.into(),));
-        }
     });
 }
 
@@ -153,9 +150,7 @@ extern fn run_open_panel<T: WebViewController + 'static>(this: &Object, _: Sel,
         &*webview
     };
 
-    webview.run_open_panel(params.into(), move |urls| {
-        // This is very sketch and should be heavily checked. :|
-        unsafe {
+    webview.run_open_panel(params.into(), move |urls| unsafe {
         let handler = handler as *const Block<(id,), c_void>;
 
         match urls {
@@ -171,10 +166,13 @@ extern fn run_open_panel<T: WebViewController + 'static>(this: &Object, _: Sel,
 
             None => { (*handler).call((nil,)); }
         }
-        }
     });
 }
 
+/// Called when a download has been initiated in the WebView, and when the navigation policy
+/// response is upgraded to BecomeDownload. Only called when explicitly linked since it's a private
+/// API.
+#[cfg(feature = "enable-webview-downloading")]
 extern fn handle_download<T: WebViewController + 'static>(this: &Object, _: Sel, download: id, suggested_filename: id, handler: usize) {
     let webview = unsafe {
         let ptr: usize = *this.get_ivar(WEBVIEW_CONTROLLER_PTR);
@@ -182,7 +180,7 @@ extern fn handle_download<T: WebViewController + 'static>(this: &Object, _: Sel,
         &*webview
     };
 
-    let handler = handler as *const Block<(BOOL, id), c_void>; 
+    let handler = handler as *const Block<(objc::runtime::BOOL, id), c_void>; 
     let filename = str_from(suggested_filename);
 
     webview.run_save_panel(filename, move |can_overwrite, path| unsafe {
@@ -190,7 +188,6 @@ extern fn handle_download<T: WebViewController + 'static>(this: &Object, _: Sel,
             let _: () = msg_send![download, cancel];
         }
 
-        println!("Saving to Path: {:?}", path);
         let path = NSString::alloc(nil).init_str(&path.unwrap());
         
         (*handler).call((match can_overwrite {
@@ -236,6 +233,7 @@ pub fn register_controller_class<
         // WKDownloadDelegate is a private class on macOS that handles downloading (saving) files.
         // It's absurd that this is still private in 2020. This probably couldn't get into the app
         // store, so... screw it, fine for now.
+        #[cfg(feature = "enable-webview-downloading")]
         decl.add_method(sel!(_download:decideDestinationWithSuggestedFilename:completionHandler:), handle_download::<T> as extern fn(&Object, _, id, id, usize));
 
         VIEW_CLASS = decl.register();

appkit/src/webview/mod.rs

@@ -7,7 +7,7 @@ pub(crate) static WEBVIEW_CONTROLLER_PTR: &str = "rstWebViewControllerPtr";
 pub mod action;
 
 pub(crate) mod controller;
-pub(crate) mod process_pool;
+//pub(crate) mod process_pool;
 
 pub mod traits;
 pub use traits::{WebViewController};

appkit/src/webview/traits.rs

@@ -1,22 +1,52 @@
+//! WebViewController is a combination of various delegates and helpers for an underlying
+//! `WKWebView`. It allows you to do things such as handle opening a file (for uploads or
+//! in-browser-processing), handling navigation actions or JS message callbacks, and so on.
 
-use crate::webview::action::{NavigationAction, NavigationPolicy, NavigationResponse, NavigationResponsePolicy, OpenPanelParameters};
+use crate::webview::action::{
+    NavigationAction, NavigationPolicy,
+    NavigationResponse, NavigationResponsePolicy,
+    OpenPanelParameters
+};
 
+/// You can implement this on structs to handle callbacks from the underlying `WKWebView`.
 pub trait WebViewController {
-    fn on_message(&self, name: &str, body: &str) {}
+    /// Called when a JS message is passed by the browser process. For instance, if you added
+    /// `notify` as a callback, and in the browser you called
+    /// `webkit.messageHandlers.notify.postMessage({...})` it would wind up here, with `name` being
+    /// `notify` and `body` being your arguments.
+    ///
+    /// Note that at the moment, you really should handle bridging JSON/stringification yourself.
+    fn on_message(&self, _name: &str, _body: &str) {}
 
-    fn policy_for_navigation_action<F: Fn(NavigationPolicy)>(&self, action: NavigationAction, handler: F) {
+    /// Given a callback handler, you can decide what policy should be taken for a given browser
+    /// action. By default, this is `NavigationPolicy::Allow`.
+    fn policy_for_navigation_action<F: Fn(NavigationPolicy)>(&self, _action: NavigationAction, handler: F) {
         handler(NavigationPolicy::Allow);
     }
 
-    fn policy_for_navigation_response<F: Fn(NavigationResponsePolicy)>(&self, response: NavigationResponse, handler: F) {
+    /// Given a callback handler, you can decide what policy should be taken for a given browser
+    /// response. By default, this is `NavigationResponsePolicy::Allow`.
+    fn policy_for_navigation_response<F: Fn(NavigationResponsePolicy)>(&self, _response: NavigationResponse, handler: F) {
         handler(NavigationResponsePolicy::Allow);
     }
 
-    fn run_open_panel<F: Fn(Option<Vec<String>>) + 'static>(&self, parameters: OpenPanelParameters, handler: F) {
+    /// Given a callback handler and some open panel parameters (e.g, if the user is clicking an
+    /// upload field that pre-specifies supported options), you should create a `FileSelectPanel`
+    /// and thread the callbacks accordingly.
+    fn run_open_panel<F: Fn(Option<Vec<String>>) + 'static>(&self, _parameters: OpenPanelParameters, handler: F) {
         handler(None);
     }
 
-    fn run_save_panel<F: Fn(bool, Option<String>) + 'static>(&self, suggested_filename: &str, handler: F) {
+    /// Given a callback handler and a suggested filename, you should create a `FileSavePanel`
+    /// and thread the callbacks accordingly.
+    ///
+    /// Note that this specific callback is only
+    /// automatically fired if you're linking in to the `enable_webview_downloads` feature, which
+    /// is not guaranteed to be App Store compatible. If you want a version that can go in the App
+    /// Store, you'll likely need to write some JS in the webview to handle triggering
+    /// downloading/saving. This is due to Apple not allowing the private methods on `WKWebView` to
+    /// be open, which... well, complain to them, not me. :)
+    fn run_save_panel<F: Fn(bool, Option<String>) + 'static>(&self, _suggested_filename: &str, handler: F) {
         handler(false, None);
     }
 }

appkit/src/webview/webview.rs

@@ -12,10 +12,10 @@ use std::rc::Rc;
 use std::cell::RefCell;
 
 use cocoa::base::{id, nil, YES, NO};
-use cocoa::foundation::{NSRect, NSPoint, NSSize, NSString};
+use cocoa::foundation::NSString;
 
 use objc_id::ShareId;
-use objc::runtime::{Class, Object, Sel, BOOL};
+use objc::runtime::Object;
 use objc::{class, msg_send, sel, sel_impl};
 
 use crate::ViewController;

appkit/src/window/controller.rs

@@ -3,11 +3,11 @@
 
 use std::sync::Once;
 
-use cocoa::base::{id, nil, YES, NO};
+use cocoa::base::id;
 
 use objc::declare::ClassDecl;
 use objc::runtime::{Class, Object, Sel};
-use objc::{class, msg_send, sel, sel_impl};
+use objc::{class, sel, sel_impl};
 
 use crate::window::WindowController;
 
@@ -30,7 +30,7 @@ pub(crate) fn register_window_controller_class<T: WindowController + 'static>()
     static INIT: Once = Once::new();
 
     INIT.call_once(|| unsafe {
-        let superclass = Class::get("NSWindowController").unwrap();
+        let superclass = class!(NSWindowController);
         let mut decl = ClassDecl::new("RSTWindowController", superclass).unwrap();
 
         decl.add_ivar::<usize>(WINDOW_CONTROLLER_PTR);

appkit/src/window/window.rs

@@ -26,9 +26,18 @@ pub struct WindowInner {
     pub toolbar: Option<Toolbar>
 }
 
-pub mod WindowTitleVisibility {
-    pub const Visible: usize = 0;
-    pub const Hidden: usize = 1;
+pub enum WindowTitleVisibility {
+    Visible,
+    Hidden
+}
+
+impl From<WindowTitleVisibility> for usize {
+    fn from(visibility: WindowTitleVisibility) -> usize {
+        match visibility {
+            WindowTitleVisibility::Visible => 0,
+            WindowTitleVisibility::Hidden => 1
+        }
+    }
 }
 
 impl WindowInner {