Skip to content
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

(ios) Adds a routine to flush WkWebView diskcache when the bundle ver… #1451

Closed

Conversation

BBlashko
Copy link

@BBlashko BBlashko commented Aug 1, 2024

…ion changes

Platforms affected

  • ios

Motivation and Context

This Pull Request adds a routine to the Boot of cordova-ios so that we can flush diskcache in the WkWebView. There is a problem with iOS where the diskcache aka .js, html, and css etc files are cached between .ipa installs.

There is no issue for this at this time, I was discussing it with Norman Breau in the Slack Space

Description

In the AppDelegate.m file I have added a routine to clear the WKWebsiteDataTypeDiskCache for the domain from the Bundle ID, and localhost. This cache needs to be flushed because it latches on to previous js, html, css, etc files from a previous .ipa installation. These cached files include the cordova.js and cordova_plugins.js files. I know this because I added hacks into these files, but they were not registered on the target production application.

The main problem this addresses is removals of Cordova Plugins. Where the cached cordova_plugins.js thinks that it needs to load a removed plugin, but since the plugins www directory was removed it doesn't exist.

An example error message is as follows:

Module branch-cordova-sdk.Branch does not exist.

Now I cannot confirm at this time, but it does indicate that with this cached files, that any additions to cordova_plugins.js may not be loaded at runtime.

I want to mention that this problem is extremely hard to reproduce in local environments. It needs to be on device, and it needs to be an update to an already installed application in production. Eg. I have 3.72.0 on my device, a new 3.73.0 version is released with removed/updated plugins, then this case can happen it is not guaranteed to happen for all users.

Testing

Our QA engineer had an instance of the application stuck in this state. The Error:

Module branch-cordova-sdk.Branch does not exist.

This error was thrown everytime they booted the application, where the UI was stuck on the Splashscreen.

Updating to the latest .ipa (with these changes in the PR) and booting, the user was able to get "unstuck". However, it's not a perfect solution because while the cache did get flushed, it didn't immediately repopulate it, and the user had to reboot the app. On second boot after this fix was introduced the user was able to boot the application successfully.

Checklist

  • [*] I've run the tests to see all new and existing tests pass
  • [*] I added automated test coverage as appropriate for this change
  • [*] Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • [*] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • [*] I've updated the documentation if necessary

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.35%. Comparing base (d8ebfb3) to head (99a44db).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1451   +/-   ##
=======================================
  Coverage   78.35%   78.35%           
=======================================
  Files          16       16           
  Lines        1825     1825           
=======================================
  Hits         1430     1430           
  Misses        395      395           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dpogue
Copy link
Member

dpogue commented Aug 14, 2024

I wonder if an easier/better fix would be to prevent the local web content from getting cached at all (since it's already getting loaded from the device itself)?

@dpogue dpogue added this to the 8.0.0 milestone Aug 14, 2024
@BBlashko
Copy link
Author

I wonder if an easier/better fix would be to prevent the local web content from getting cached at all (since it's already getting loaded from the device itself)?

Ya that would make sense to me 🤔

Could you point me in a bit of a direction as to where cordova-ios initializing the WKWebView? so i can see if I can put some no cache configuration on it? I'm a bit inexperienced with Native iOS. I understand the AppDelegate, and that initializes the MainViewController, but where does the framework initialize the WebView?

@dpogue
Copy link
Member

dpogue commented Aug 20, 2024

I think what we'd need to do is explicitly set a cache policy on the NSURLRequest that is created when we load a URL into the WKWebView:

request = [NSURLRequest requestWithURL:url];

By default, that's going to use "default protocol cache behaviour", which is documented here: https://developer.apple.com/documentation/foundation/nsurlrequestcachepolicy/nsurlrequestuseprotocolcachepolicy?language=objc

We might want to set the policy to NSURLRequestReloadIgnoringLocalCacheData and test if that solves the problem.

To set the cache policy when constructing the NSURLRequest you can use the requestWithURL:cachePolicy:timeoutInterval: method: https://developer.apple.com/documentation/foundation/nsurlrequest/1528579-requestwithurl?language=objc

@BBlashko
Copy link
Author

BBlashko commented Aug 20, 2024

I created a new pull request with the update. Since it was a completely different approach I figured a new branch and new pull request was the best path.

#1471

@dpogue dpogue removed this from the 8.0.0 milestone Aug 24, 2024
@dpogue
Copy link
Member

dpogue commented Aug 29, 2024

Closing to close this PR in favour of getting #1471 merged in.

@dpogue dpogue closed this Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants