CB-14045 - reinit url after app freezes #363

svalaskevicius · 2018-04-23T09:55:47Z

Hi!

We've stumbled on a similar issue to this: https://issues.apache.org/jira/browse/CB-14045 although for us it takes a bit longer (10h or so of app being in background).

In our case the app resumes in a half working state, where the web view has been reinitialised pointing to about:blank with only default Js api available and no cordova js. Then, cordova tries to emit those resume and other events but all of them fail as there is no cordova js var available.

this approach does not fix app getting into that state as I am not fully aware how does it do that (12h debugging cycle is pretty hard).. but it gets it out of the broken state by pointing the webview back to the app's url.

what do you think?

Thanks!

codecov-io · 2018-04-23T10:03:02Z

Codecov Report

Merging #363 into 4.5.x will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##            4.5.x     #363   +/-   ##
=======================================
  Coverage   63.45%   63.45%           
=======================================
  Files          14       14           
  Lines        1691     1691           
  Branches      284      284           
=======================================
  Hits         1073     1073           
  Misses        618      618

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update acf9107...56f5dc6. Read the comment docs.

shazron · 2018-04-24T06:59:22Z

CordovaLib/Classes/Public/CDVViewController.m

@@ -720,12 +720,25 @@ - (void)onAppWillTerminate:(NSNotification*)notification
    }
 }

+- (void)checkAndReinitViewUrl
+{
+    if ([[[self.webViewEngine URL] absoluteString] isEqualToString:@"about:blank"]) {


In the unlikely scenario that appURL is actually about:blank, you should check for this condition.
RIght now there is no probability of an infinite loop, but you never know if there are side-effects with any calls further down the stack in checkAndReinitViewUrl

I would return a BOOL from this function (YES if it re-inited, NO if it did not), and add a unit test in https://github.com/apache/cordova-ios/blob/master/tests/CordovaLibTests/CDVViewControllerTest.m

thanks! I'll try to update these when I have a chance :) we've found another case when testing the loaded url in the webview is (null), so added those checks too

svalaskevicius · 2018-05-15T15:30:58Z

Hi @shazron , I've changed the function as requested, however I'm having problems with the test...

I've tried adding this test:

-(void)testIfItLoadsAppUrlIfCurrentViewIsBlank{
    CDVViewController* viewController = [self viewController];
    [viewController view];
    NSString* html = @"<html><body></body></html>";
    [viewController.webViewEngine loadHTMLString:html baseURL:@"about:blank"];
    XCTAssertTrue([viewController checkAndReinitViewUrl]);
    XCTAssertFalse([viewController checkAndReinitViewUrl]);
}

However, loadRequest does not load anything when running tests, and the url is always null..

Any chance you could take this PR from here and add the missing test? I suspect it should not be difficult, however might require some more background knowledge about cordova/ios..

I can also confirm that the approach to solve the initial problem (frozen apps for users) has been fixed by this PR (after also checking if it's not null and not "about:blank")

Thanks!

shazron · 2018-05-16T04:01:41Z

@svalaskevicius sure, will do. Can you make sure the checkbox "allow maintainers to make edits" is checked? Thanks: https://blog.github.com/2016-09-07-improving-collaboration-with-forks/

shazron · 2018-05-16T05:18:25Z

Never mind, I just did a push and it was successful. Looks like it was checked by default.

* test reinit url * refactor: split function + check if appUrl is not empty * Added tests.

This reverts commit 6b3f10f.

shazron · 2018-05-16T05:48:27Z

Made a mistake - didn't realize this was based off 4.5.x branch so it was pulled into that. Reverted the change to 4.5.x and applied to master instead.

svalaskevicius · 2018-05-16T09:34:21Z

thanks!

GulinSS · 2018-09-06T16:01:14Z

Did it been added for current 4.5.5 public release?

* test reinit url * refactor: split function + check if appUrl is not empty * Added tests.

brodybits · 2018-09-27T00:28:02Z

Did it been added for current 4.5.5 public release?

No it was removed from 4.5.5 since we intended to merge the fix on master as discussed in #363 (comment). I just raised PR #418 in case we do another cordova-ios@4.5.x patch release.

As a general rule I recommend asking this kind of question in a new issue or by email to avoid losing such comments in closed issues and merged PRs.

shazron requested changes Apr 24, 2018

View reviewed changes

test reinit url

33b678f

svalaskevicius force-pushed the test-reinit-url branch from 9776356 to 33b678f Compare April 26, 2018 14:28

refactor: split function + check if appUrl is not empty

14eea49

Added tests.

1b50935

Clean-up.

56f5dc6

shazron changed the title ~~reinit url after app freezes~~ CB-14045 - reinit url after app freezes May 16, 2018

shazron merged commit 6b3f10f into apache:4.5.x May 16, 2018

shazron pushed a commit that referenced this pull request May 16, 2018

CB-14045 - Reinit url after app freezes (#363)

6e790af

* test reinit url * refactor: split function + check if appUrl is not empty * Added tests.

shazron added a commit that referenced this pull request May 16, 2018

Revert "CB-14045 - Reinit url after app freezes (#363)"

00a4b27

This reverts commit 6b3f10f.

svalaskevicius deleted the test-reinit-url branch May 16, 2018 09:34

svalaskevicius restored the test-reinit-url branch May 16, 2018 09:34

brodybits pushed a commit to brodybits/cordova-ios that referenced this pull request Sep 27, 2018

CB-14045 - Reinit url after app freezes (apache#363)

b5f6f8d

* test reinit url * refactor: split function + check if appUrl is not empty * Added tests.

brodybits pushed a commit to brodybits/cordova-ios that referenced this pull request Sep 27, 2018

CB-14045 - Reinit url after app freezes (apache#363)

bab1654

* test reinit url * refactor: split function + check if appUrl is not empty * Added tests.

brodybits mentioned this pull request Sep 27, 2018

CB-14045 - reinit url after app freezes on 4.5.x #418

Merged

alexhisen mentioned this pull request May 14, 2019

Reload fails after Webkit is terminated microsoft/cordova-plugin-code-push#529

Closed

cclobo mentioned this pull request Oct 28, 2019

Fonts and images linked in css files not loading on app start #701

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CB-14045 - reinit url after app freezes #363

CB-14045 - reinit url after app freezes #363

svalaskevicius commented Apr 23, 2018

codecov-io commented Apr 23, 2018 •

edited

Loading

shazron Apr 24, 2018 •

edited

Loading

svalaskevicius Apr 26, 2018 •

edited

Loading

svalaskevicius commented May 15, 2018

shazron commented May 16, 2018

shazron commented May 16, 2018

shazron commented May 16, 2018

svalaskevicius commented May 16, 2018

GulinSS commented Sep 6, 2018 •

edited

Loading

brodybits commented Sep 27, 2018

CB-14045 - reinit url after app freezes #363

CB-14045 - reinit url after app freezes #363

Conversation

svalaskevicius commented Apr 23, 2018

codecov-io commented Apr 23, 2018 • edited Loading

Codecov Report

shazron Apr 24, 2018 • edited Loading

Choose a reason for hiding this comment

svalaskevicius Apr 26, 2018 • edited Loading

Choose a reason for hiding this comment

svalaskevicius commented May 15, 2018

shazron commented May 16, 2018

shazron commented May 16, 2018

shazron commented May 16, 2018

svalaskevicius commented May 16, 2018

GulinSS commented Sep 6, 2018 • edited Loading

brodybits commented Sep 27, 2018

codecov-io commented Apr 23, 2018 •

edited

Loading

shazron Apr 24, 2018 •

edited

Loading

svalaskevicius Apr 26, 2018 •

edited

Loading

GulinSS commented Sep 6, 2018 •

edited

Loading