msmtamburro commented on a change in pull request #1030:
URL: https://github.com/apache/cordova-ios/pull/1030#discussion_r532844999
##########
File path: CordovaLib/Classes/Public/CDVURLSchemeHandler.m
##########
@@ -39,45 +41,70 @@ - (void)webView:(WKWebView *)webView startURLSchemeTask:(id
<WKURLSchemeTask>)ur
NSURL * url = urlSchemeTask.request.URL;
NSString * stringToLoad = url.path;
NSString * scheme = url.scheme;
-
- if ([scheme isEqualToString:self.viewController.appScheme]) {
- if ([stringToLoad hasPrefix:@"/_app_file_"]) {
- startPath = [stringToLoad
stringByReplacingOccurrencesOfString:@"/_app_file_" withString:@""];
- } else {
- if ([stringToLoad isEqualToString:@""] || [url.pathExtension
isEqualToString:@""]) {
- startPath = [startPath
stringByAppendingPathComponent:self.viewController.startPage];
- } else {
- startPath = [startPath
stringByAppendingPathComponent:stringToLoad];
+
+ CDVViewController* vc = (CDVViewController*)self.viewController;
+
+ /*
+ * Give plugins the chance to handle the url
+ */
+ BOOL anyPluginsResponded = NO;
+ BOOL handledRequest = NO;
+
+ for (NSString* pluginName in vc.pluginObjects) {
+ CDVPlugin* plugin = [vc.pluginObjects objectForKey:pluginName];
+ SEL selector = NSSelectorFromString(@"handleSchemeURL:");
+ if ([plugin respondsToSelector:selector]) {
+ handledRequest = (((BOOL (*)(id, SEL, id
<WKURLSchemeTask>))objc_msgSend)(plugin, selector, urlSchemeTask));
+ NSLog(@"Handled: %d", handledRequest);
Review comment:
Hi! Mind not adding NSLogs to code that can run in production
environments?
##########
File path: CordovaLib/Classes/Public/CDVURLSchemeHandler.m
##########
@@ -39,45 +41,70 @@ - (void)webView:(WKWebView *)webView startURLSchemeTask:(id
<WKURLSchemeTask>)ur
NSURL * url = urlSchemeTask.request.URL;
NSString * stringToLoad = url.path;
NSString * scheme = url.scheme;
-
- if ([scheme isEqualToString:self.viewController.appScheme]) {
- if ([stringToLoad hasPrefix:@"/_app_file_"]) {
- startPath = [stringToLoad
stringByReplacingOccurrencesOfString:@"/_app_file_" withString:@""];
- } else {
- if ([stringToLoad isEqualToString:@""] || [url.pathExtension
isEqualToString:@""]) {
- startPath = [startPath
stringByAppendingPathComponent:self.viewController.startPage];
- } else {
- startPath = [startPath
stringByAppendingPathComponent:stringToLoad];
+
+ CDVViewController* vc = (CDVViewController*)self.viewController;
+
+ /*
+ * Give plugins the chance to handle the url
+ */
+ BOOL anyPluginsResponded = NO;
+ BOOL handledRequest = NO;
+
+ for (NSString* pluginName in vc.pluginObjects) {
Review comment:
This for loop pattern, which we also see in CDVWebViewEngine.m,
occasionally causes crashes in our production apps because `vc.pluginObjects`
can be mutated on a different thread while you're looping. Here's an example:
```
0 CoreFoundation 0x1ac96a5ac __exceptionPreprocess + 220
(NSException.m:199)
1 libobjc.A.dylib 0x1c0a5842c objc_exception_throw + 60
(objc-exception.mm:565)
2 CoreFoundation 0x1ac969f04
__NSFastEnumerationMutationHandler + 124 (NSEnumerator.m:129)
3 Our App 0x1022e13ec -[CDVWebViewEngine
webView:decidePolicyForNavigationAction:decisionHandler:] + 336
(CDVWebViewEngine.m:543)
```
There are several right ways to deal with this:
1. The best would be to use a protocol, like my similar request here:
https://github.com/apache/cordova-ios/issues/900
2. You could alternatively use DispatchQueue to safely read and write
pluginObjects on separate threads
3. You could simply make a copy of pluginObjects and enumerate the copy
instead
Since #3 is the easiest to write in a PR comment, that would look like the
below, but I'd be happy to help you do #1, which would be a lot better.
```
/*
* Give plugins the chance to handle the url
*/
__block BOOL anyPluginsResponded = NO;
__block BOOL shouldAllowRequest = NO;
NSDictionary *pluginObjects = [[vc pluginObjects] copy];
SEL selector =
NSSelectorFromString(@"shouldOverrideLoadWithRequest:navigationType:");
[pluginObjects enumerateKeysAndObjectsUsingBlock:^(id _Nonnull key, id
_Nonnull plugin, BOOL * _Nonnull stop) {
if ([plugin respondsToSelector:selector]) {
anyPluginsResponded = YES;
// https://issues.apache.org/jira/browse/CB-12497
int navType = (int)navigationAction.navigationType;
shouldAllowRequest = (((BOOL (*)(id, SEL, id,
int))objc_msgSend)(plugin, selector, navigationAction.request, navType));
if (!shouldAllowRequest) {
*stop = YES;
}
}
}];
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
