refactor: Clean up properties/ivars in CDVViewController (#1474)

Rename wwwFolderName to webContentFolderName so that it gets a useful label in Xcode's Interface Builder for people embedding CDVViewController inside their own apps.
apache · Aug 24, 2024 · 27a6068 · 27a6068
1 parent ab78439
commit 27a6068
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 47 deletions.
diff --git a/CordovaLib/Classes/Private/CDVCommandDelegateImpl.m b/CordovaLib/Classes/Private/CDVCommandDelegateImpl.m
@@ -52,10 +52,10 @@ - (NSString*)pathForResource:(NSString*)resourcepath
     [directoryParts removeLastObject];
 
     NSString* directoryPartsJoined = [directoryParts componentsJoinedByString:@"/"];
-    NSString* directoryStr = _viewController.wwwFolderName;
+    NSString* directoryStr = _viewController.webContentFolderName;
 
     if ([directoryPartsJoined length] > 0) {
-        directoryStr = [NSString stringWithFormat:@"%@/%@", _viewController.wwwFolderName, [directoryParts componentsJoinedByString:@"/"]];
+        directoryStr = [NSString stringWithFormat:@"%@/%@", _viewController.webContentFolderName, [directoryParts componentsJoinedByString:@"/"]];
     }
 
     return [mainBundle pathForResource:filename ofType:@"" inDirectory:directoryStr];

diff --git a/CordovaLib/Classes/Public/CDVURLSchemeHandler.m b/CordovaLib/Classes/Public/CDVURLSchemeHandler.m
@@ -37,7 +37,7 @@ - (instancetype)initWithVC:(CDVViewController *)controller
 
 - (void)webView:(WKWebView *)webView startURLSchemeTask:(id <WKURLSchemeTask>)urlSchemeTask
 {
-    NSString * startPath = [[NSBundle mainBundle] pathForResource:self.viewController.wwwFolderName ofType: nil];
+    NSString * startPath = [[NSBundle mainBundle] pathForResource:self.viewController.webContentFolderName ofType: nil];
     NSURL * url = urlSchemeTask.request.URL;
     NSString * stringToLoad = url.path;
     NSString * scheme = url.scheme;

diff --git a/CordovaLib/Classes/Public/CDVViewController.m b/CordovaLib/Classes/Public/CDVViewController.m
@@ -42,34 +42,30 @@ Licensed to the Apache Software Foundation (ASF) under one
 @interface CDVViewController () <CDVWebViewEngineConfigurationDelegate> {
     id <CDVWebViewEngineProtocol> _webViewEngine;
     id <CDVCommandDelegate> _commandDelegate;
+    NSMutableDictionary<NSString *, CDVPlugin *> *_pluginObjects;
+    NSMutableDictionary<NSString *, NSString *> *_pluginsMap;
     CDVCommandQueue* _commandQueue;
     UIColor* _backgroundColor;
     UIColor* _splashBackgroundColor;
+    CDVSettingsDictionary* _settings;
 }
 
-@property (nonatomic, readwrite, strong) NSXMLParser* configParser;
-@property (nonatomic, readwrite, strong) CDVSettingsDictionary* settings;
-@property (nonatomic, readwrite, strong) NSMutableDictionary* pluginObjects;
 @property (nonatomic, readwrite, strong) NSMutableArray* startupPluginNames;
-@property (nonatomic, readwrite, strong) NSDictionary* pluginsMap;
 @property (nonatomic, readwrite, strong) UIView* launchView;
-
 @property (readwrite, assign) BOOL initialized;
 
-@property (atomic, strong) NSURL* openURL;
-
 @end
 
 @implementation CDVViewController
 
-@synthesize pluginObjects, pluginsMap, startupPluginNames;
-@synthesize configParser, settings;
-@synthesize wwwFolderName, startPage, initialized, openURL;
+@synthesize pluginObjects = _pluginObjects;
+@synthesize pluginsMap = _pluginsMap;
 @synthesize commandDelegate = _commandDelegate;
 @synthesize commandQueue = _commandQueue;
 @synthesize webViewEngine = _webViewEngine;
 @synthesize backgroundColor = _backgroundColor;
 @synthesize splashBackgroundColor = _splashBackgroundColor;
+@synthesize settings = _settings;
 @dynamic webView;
 
 #pragma mark - Initializers
@@ -123,8 +119,12 @@ - (void)_cdv_init
 
         // Default property values
         self.configFile = @"config.xml";
+        self.webContentFolderName = @"www";
         self.showInitialSplashScreen = YES;
 
+        // Initialize the plugin objects dict.
+        _pluginObjects = [[NSMutableDictionary alloc] initWithCapacity:20];
+
         // Prevent reinitializing
         self.initialized = YES;
     }
@@ -135,12 +135,14 @@ - (void)_cdv_init
 - (void)dealloc
 {
     [[NSNotificationCenter defaultCenter] removeObserver:self];
-
     [_commandQueue dispose];
-    [[self.pluginObjects allValues] makeObjectsPerformSelector:@selector(dispose)];
 
     [self.webViewEngine loadHTMLString:@"about:blank" baseURL:nil];
-    [self.pluginObjects removeAllObjects];
+
+    @synchronized(_pluginObjects) {
+        [[_pluginObjects allValues] makeObjectsPerformSelector:@selector(dispose)];
+        [_pluginObjects removeAllObjects];
+    }
 
     [self.webView removeFromSuperview];
     [self.launchView removeFromSuperview];
@@ -150,6 +152,16 @@ - (void)dealloc
 
 #pragma mark - Getters & Setters
 
+- (NSString *)wwwFolderName
+{
+    return self.webContentFolderName;
+}
+
+- (void)setWwwFolderName:(NSString *)name
+{
+    self.webContentFolderName = name;
+}
+
 - (void)setBackgroundColor:(UIColor *)color
 {
     _backgroundColor = color ?: defaultBackgroundColor();
@@ -160,6 +172,12 @@ - (void)setSplashBackgroundColor:(UIColor *)color
     _splashBackgroundColor = color ?: self.backgroundColor;
 }
 
+// Only for testing
+- (void)setSettings:(CDVSettingsDictionary *)settings
+{
+    _settings = settings;
+}
+
 - (nullable NSURL *)configFilePath
 {
     NSString* path = self.configFile;
@@ -187,23 +205,17 @@ - (void)loadSettings
     CDVConfigParser *parser = [CDVConfigParser parseConfigFile:self.configFilePath];
 
     // Get the plugin dictionary, allowList and settings from the delegate.
-    self.pluginsMap = parser.pluginsDict;
+    _pluginsMap = parser.pluginsDict;
     self.startupPluginNames = parser.startupPluginNames;
     self.settings = [[CDVSettingsDictionary alloc] initWithDictionary:parser.settings];
 
-    // And the start folder/page.
-    if(self.wwwFolderName == nil){
-        self.wwwFolderName = @"www";
-    }
+    // And the start page
     if(parser.startPage && self.startPage == nil){
         self.startPage = parser.startPage;
     }
     if (self.startPage == nil) {
         self.startPage = @"index.html";
     }
-
-    // Initialize the plugin objects dict.
-    self.pluginObjects = [[NSMutableDictionary alloc] initWithCapacity:20];
 }
 
 - (NSURL*)appUrl
@@ -212,15 +224,15 @@ - (NSURL*)appUrl
 
     if ([self.startPage rangeOfString:@"://"].location != NSNotFound) {
         appURL = [NSURL URLWithString:self.startPage];
-    } else if ([self.wwwFolderName rangeOfString:@"://"].location != NSNotFound) {
-        appURL = [NSURL URLWithString:[NSString stringWithFormat:@"%@/%@", self.wwwFolderName, self.startPage]];
-    } else if([self.wwwFolderName rangeOfString:@".bundle"].location != NSNotFound){
+    } else if ([self.webContentFolderName rangeOfString:@"://"].location != NSNotFound) {
+        appURL = [NSURL URLWithString:[NSString stringWithFormat:@"%@/%@", self.webContentFolderName, self.startPage]];
+    } else if([self.webContentFolderName rangeOfString:@".bundle"].location != NSNotFound){
         // www folder is actually a bundle
-        NSBundle* bundle = [NSBundle bundleWithPath:self.wwwFolderName];
+        NSBundle* bundle = [NSBundle bundleWithPath:self.webContentFolderName];
         appURL = [bundle URLForResource:self.startPage withExtension:nil];
-    } else if([self.wwwFolderName rangeOfString:@".framework"].location != NSNotFound){
+    } else if([self.webContentFolderName rangeOfString:@".framework"].location != NSNotFound){
         // www folder is actually a framework
-        NSBundle* bundle = [NSBundle bundleWithPath:self.wwwFolderName];
+        NSBundle* bundle = [NSBundle bundleWithPath:self.webContentFolderName];
         appURL = [bundle URLForResource:self.startPage withExtension:nil];
     } else {
         // CB-3005 strip parameters from start page to check if page exists in resources
@@ -316,7 +328,7 @@ - (void)viewDidLoad
         NSURLRequest* appReq = [NSURLRequest requestWithURL:appURL cachePolicy:NSURLRequestUseProtocolCachePolicy timeoutInterval:20.0];
         [_webViewEngine loadRequest:appReq];
     } else {
-        NSString* loadErr = [NSString stringWithFormat:@"ERROR: Start Page at '%@/%@' was not found.", self.wwwFolderName, self.startPage];
+        NSString* loadErr = [NSString stringWithFormat:@"ERROR: Start Page at '%@/%@' was not found.", self.webContentFolderName, self.startPage];
         NSLog(@"%@", loadErr);
 
         NSURL* errorUrl = [self errorURL];
@@ -521,7 +533,9 @@ - (void)registerPlugin:(CDVPlugin*)plugin withClassName:(NSString*)className
     plugin.viewController = self;
     plugin.commandDelegate = _commandDelegate;
 
-    [self.pluginObjects setObject:plugin forKey:className];
+    @synchronized(_pluginObjects) {
+        [_pluginObjects setObject:plugin forKey:className];
+    }
     [plugin pluginInitialize];
 }
 
@@ -531,8 +545,11 @@ - (void)registerPlugin:(CDVPlugin*)plugin withPluginName:(NSString*)pluginName
     plugin.commandDelegate = _commandDelegate;
 
     NSString* className = NSStringFromClass([plugin class]);
-    [self.pluginObjects setObject:plugin forKey:className];
-    [self.pluginsMap setValue:className forKey:[pluginName lowercaseString]];
+
+    @synchronized(_pluginObjects) {
+        [_pluginObjects setObject:plugin forKey:className];
+    }
+    [_pluginsMap setValue:className forKey:[pluginName lowercaseString]];
     [plugin pluginInitialize];
 }
 
@@ -546,7 +563,7 @@ - (nullable CDVPlugin *)getCommandInstance:(NSString *)pluginName
     // NOTE: plugin names are matched as lowercase to avoid problems - however, a
     // possible issue is there can be duplicates possible if you had:
     // "org.apache.cordova.Foo" and "org.apache.cordova.foo" - only the lower-cased entry will match
-    NSString* className = [self.pluginsMap objectForKey:[pluginName lowercaseString]];
+    NSString* className = [_pluginsMap objectForKey:[pluginName lowercaseString]];
 
     if (className == nil) {
         return nil;

diff --git a/CordovaLib/include/Cordova/CDVViewController.h b/CordovaLib/include/Cordova/CDVViewController.h
@@ -36,24 +36,24 @@ NS_ASSUME_NONNULL_BEGIN
 
 @property (nonatomic, nullable, readonly, strong) NSXMLParser *configParser CDV_DEPRECATED(8, "Unused");
 
-@property (nonatomic, readonly, nullable, weak) IBOutlet UIView* webView;
+@property (nonatomic, readonly, nullable, weak) IBOutlet UIView *webView;
 
-@property (nullable, nonatomic, readonly, strong) NSMutableDictionary* pluginObjects;
-@property (nonatomic, readonly, strong) NSDictionary* pluginsMap;
+@property (nonatomic, readonly, strong) NSDictionary<NSString *, CDVPlugin *> *pluginObjects;
+@property (nullable, nonatomic, readonly, strong) NSDictionary<NSString *, NSString *> *pluginsMap CDV_DEPRECATED(8, "Internal implementation detail, should not be used");
 
 /**
  The Cordova preferences for this view.
 
  This is a dictionary populated from the preference key/value pairs in the
  Cordova XML configuration file.
  */
-@property (nonatomic, readonly, strong) CDVSettingsDictionary* settings;
+@property (nonatomic, readonly, strong) CDVSettingsDictionary *settings;
 
-@property (nonatomic, readwrite, copy) NSString* appScheme;
-@property (nonatomic, readwrite, copy) NSString* configFile;
-@property (nonatomic, readwrite, copy) NSString* wwwFolderName;
-@property (nonatomic, readwrite, copy) NSString* startPage;
-@property (nonatomic, readonly, strong) CDVCommandQueue* commandQueue;
+@property (nonatomic, readwrite, copy) NSString *appScheme;
+@property (nonatomic, readwrite, copy) IBInspectable NSString *configFile;
+@property (nonatomic, readwrite, copy) NSString *wwwFolderName CDV_DEPRECATED_WITH_REPLACEMENT(8, "Use webContentFolderName instead", "webContentFolderName");
+@property (nonatomic, nullable, readwrite, copy) IBInspectable NSString *startPage;
+@property (nonatomic, readonly, strong) CDVCommandQueue *commandQueue;
 @property (nonatomic, readonly, strong) id <CDVWebViewEngineProtocol> webViewEngine;
 @property (nonatomic, readonly, strong) id <CDVCommandDelegate> commandDelegate;
 
@@ -62,6 +62,18 @@ NS_ASSUME_NONNULL_BEGIN
  */
 @property (nonatomic, nullable, readonly, copy) NSURL *configFilePath;
 
+/**
+ The filepath to the HTML error fallback page, if one has been provided.
+ */
+@property (nonatomic, nullable, readonly, copy) NSURL *errorURL;
+
+/**
+ The folder path containing the web content to be displayed.
+ The default value is `"www"`.
+ This can be set in the storyboard file as a view controller attribute.
+  */
+@property (nonatomic, readwrite, copy) IBInspectable NSString *webContentFolderName;
+
 /**
  A boolean value indicating whether to show the splash screen while the webview
  is initially loading.
@@ -96,8 +108,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (UIView*)newCordovaViewWithFrame:(CGRect)bounds;
 
-- (nullable NSString*)appURLScheme;
-- (nullable NSURL*)errorURL;
+- (nullable NSString*)appURLScheme CDV_DEPRECATED(8, "Unused");
 
 - (nullable CDVPlugin *)getCommandInstance:(NSString *)pluginName;
 - (void)registerPlugin:(CDVPlugin*)plugin withClassName:(NSString*)className;

diff --git a/tests/CordovaLibTests/CordovaLibApp/AppDelegate.m b/tests/CordovaLibTests/CordovaLibApp/AppDelegate.m
@@ -34,7 +34,7 @@ @implementation AppDelegate
 - (void)createViewController
 {
     _viewController = [[ViewController alloc] init];
-    _viewController.wwwFolderName = @"www";
+    _viewController.webContentFolderName = @"www";
     _viewController.startPage = @"index.html";
 
     // NOTE: To customize the view's frame size (which defaults to full screen), override