-
Notifications
You must be signed in to change notification settings - Fork 984
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UIWebView call loadRequest multiple times lead to deadlock in iOS12 (fixes #447) #450
Conversation
Codecov Report
@@ Coverage Diff @@
## master #450 +/- ##
=======================================
Coverage 74.29% 74.29%
=======================================
Files 12 12
Lines 1564 1564
=======================================
Hits 1162 1162
Misses 402 402 Continue to review full report at Codecov.
|
@@ -329,9 +329,14 @@ - (void)webViewDidFinishLoad:(UIWebView*)webView | |||
break; | |||
|
|||
case STATE_WAITING_FOR_LOAD_FINISH: | |||
if (_loadCount == 1) { | |||
// fix call loadRequest multiple times just callback webViewDidFinishLoad once time in iOS 12 | |||
if (@available(iOS 12.0, *)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly does this line do?
Does it check for "iOS 12.0"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes,check if the system version is "12.0"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't that mean that is already broken again in 12.1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like @janpio said, you need to test for iOS 12 and greater. Use IsAtLeastiOSVersion
#define IsAtLeastiOSVersion(X) ([[[UIDevice currentDevice] systemVersion] compare:X options:NSNumericSearch] != NSOrderedAscending) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I test for iOS 12.1 (Xcode Version 10.1 (10B61)),this problem has not been reproduced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@crmo So this fix is just for 12.0 specifically, you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @available
check includes newer versions than the one specified.
I have the same problem in iOS12.0 .can these codes be merged? |
Detail Explanation * Forces iOS 12 to fire callback in webViewDidFinish to disable the splash screen and load the webview for the app. * Fix ported from: apache#450
We were running into the same issue in one of our apps (symptom being that the splashscreen stayed up forever because the UIWebView never fired its loaded callback). @jeff2013 did some investigating and determined that the changes in this PR solve the issue. Worth noting, we were seeing this issue on iOS 12.1.2 and iOS 12.2 beta, so it seems to affect more than just iOS 12.0. So I'm going to approve this for merge and hopefully get it into cordova-ios@5.0.1. |
Platforms affected
iOS
What does this PR do?
Fix issue 447 UIWebView call loadRequest multiple times lead to deadlock in iOS12.
The number of callback of
webViewDidStartLoad
andwebViewDidFinishLoad
is not equal._loadCount
will never equals 1, delegate'swebViewDidFinishLoad
and[CDVUserAgentUtil releaseLock:vc.userAgentLockToken];
will never call.Then new CDVViewcontroller won't load page.What testing has been done on this change?
Checklist