You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by cjpearson <gi...@git.apache.org> on 2016/10/11 18:09:01 UTC

[GitHub] cordova-plugin-wkwebview-engine pull request #23: CB-11997: Add crash recove...

GitHub user cjpearson opened a pull request:

    https://github.com/apache/cordova-plugin-wkwebview-engine/pull/23

    CB-11997: Add crash recovery for iOS 8

    <!--
    Please make sure the checklist boxes are all checked before submitting the PR. The checklist
    is intended as a quick reference, for complete details please see our Contributor Guidelines:
    
    http://cordova.apache.org/contribute/contribute_guidelines.html
    
    Thanks!
    -->
    
    ### Platforms affected
    iOS
    
    ### What does this PR do?
    Reloads the WebView when it crashes. This behavior already exists on iOS >= 9, but has not been added for iOS 8.
    
    ### What testing has been done on this change?
    I've tried consuming a bunch of memory in javascript to cause a crash. The app reloads successfully.
    
    ### Checklist
    - [ ] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
    - [ ] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
    - [ ] Added automated test coverage as appropriate for this change.
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/cjpearson/cordova-plugin-wkwebview-engine master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cordova-plugin-wkwebview-engine/pull/23.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #23
    
----
commit d9d30f2cf91f307b44ecc5efe1763ae7d0b3bdb3
Author: Connor Pearson <cj...@gmail.com>
Date:   2016-10-11T18:04:37Z

    CB-11997: Add crash recovery for iOS 8

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-wkwebview-engine issue #23: CB-11997: Add crash recovery for ...

Posted by shazron <gi...@git.apache.org>.
Github user shazron commented on the issue:

    https://github.com/apache/cordova-plugin-wkwebview-engine/pull/23
  
    Note: this passes tests when run locally (the CI has a known issue that it fails). Pulling this in now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-wkwebview-engine pull request #23: CB-11997: Add crash recove...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cordova-plugin-wkwebview-engine/pull/23


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-wkwebview-engine pull request #23: CB-11997: Add crash recove...

Posted by shazron <gi...@git.apache.org>.
Github user shazron commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-wkwebview-engine/pull/23#discussion_r90091089
  
    --- Diff: src/ios/CDVWKWebViewEngine.m ---
    @@ -123,6 +123,32 @@ - (void)pluginInitialize
                    name:UIApplicationWillEnterForegroundNotification object:nil];
     
         NSLog(@"Using WKWebView");
    +
    +    [self addURLObserver];
    +}
    +
    +- (void)onReset {
    +    [self addURLObserver];
    +}
    +
    +static void * KVOContext = &KVOContext;
    +
    +- (void)addURLObserver {
    +    if(![[NSProcessInfo processInfo] isOperatingSystemAtLeastVersion:(NSOperatingSystemVersion){.majorVersion = 9, .minorVersion = 0, .patchVersion = 0 }]){
    --- End diff --
    
    To make it simpler, you can use `IsAtLeastiOSVersion(@"9.0")`  (from CDVAvailability.h)
    I'm not sure how this whole patch would work for iOS 8, because of this note: https://github.com/apache/cordova-plugin-wkwebview-engine#notes
    
    Is this for a "using a local webserver" context?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-wkwebview-engine pull request #23: CB-11997: Add crash recove...

Posted by cjpearson <gi...@git.apache.org>.
Github user cjpearson commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-wkwebview-engine/pull/23#discussion_r90223157
  
    --- Diff: src/ios/CDVWKWebViewEngine.m ---
    @@ -123,6 +123,32 @@ - (void)pluginInitialize
                    name:UIApplicationWillEnterForegroundNotification object:nil];
     
         NSLog(@"Using WKWebView");
    +
    +    [self addURLObserver];
    +}
    +
    +- (void)onReset {
    +    [self addURLObserver];
    +}
    +
    +static void * KVOContext = &KVOContext;
    +
    +- (void)addURLObserver {
    +    if(![[NSProcessInfo processInfo] isOperatingSystemAtLeastVersion:(NSOperatingSystemVersion){.majorVersion = 9, .minorVersion = 0, .patchVersion = 0 }]){
    --- End diff --
    
    Yeah I was using the local webserver plugin to support iOS 8.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-wkwebview-engine pull request #23: CB-11997: Add crash recove...

Posted by shazron <gi...@git.apache.org>.
Github user shazron commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-wkwebview-engine/pull/23#discussion_r90092299
  
    --- Diff: src/ios/CDVWKWebViewEngine.m ---
    @@ -123,6 +123,32 @@ - (void)pluginInitialize
                    name:UIApplicationWillEnterForegroundNotification object:nil];
     
         NSLog(@"Using WKWebView");
    +
    +    [self addURLObserver];
    +}
    +
    +- (void)onReset {
    +    [self addURLObserver];
    +}
    +
    +static void * KVOContext = &KVOContext;
    +
    +- (void)addURLObserver {
    +    if(![[NSProcessInfo processInfo] isOperatingSystemAtLeastVersion:(NSOperatingSystemVersion){.majorVersion = 9, .minorVersion = 0, .patchVersion = 0 }]){
    +        [self.webView addObserver:self forKeyPath:@"URL" options:0 context:KVOContext];
    +    }
    +}
    +
    +- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSString *,id> *)change context:(void *)context
    +{
    +    if (context == KVOContext) {
    +        if (object == [self webView] && [keyPath isEqualToString: @"URL"] && [object valueForKeyPath:keyPath] == nil){
    +            NSLog(@"URL is nil. Reloading WebView");
    --- End diff --
    
    Change WebView to WKWebView to give more clarity in the log here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org