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

feat: Notify JS-layer when webview crashed #2379

Open
sandstrom opened this issue Jan 28, 2020 · 18 comments
Open

feat: Notify JS-layer when webview crashed #2379

sandstrom opened this issue Jan 28, 2020 · 18 comments
Labels

Comments

@sandstrom
Copy link
Contributor

sandstrom commented Jan 28, 2020

Feature Request

It would be great if the JS-layer (Capacitor app) could know that the underlying web view crashed and subsequently restarted. Right now there is nothing to discern these from a regular app startup and users won't be aware that data was lost.

More concretely, an issue that occur frequently is that we take a photograph via an <input capture> element and do on-the-fly resize using JS and canvas, and the web view crashes, causing the page to reload. But for the user this isn't obvious, it's just a short flash when the web view reloads, and they don't know that the photograph failed and they've just lost data.

If we could listen to these events we could show a warning to the user.

This is the code on iOS today:

public func webViewWebContentProcessDidTerminate(_ webView: WKWebView) {
webView.reload()
}

Platform Support Requested

  • Android
  • iOS

Describe Preferred Solution

There are multiple solutions, but one could be a listener similar to appRestoredResult , that would let us know that the app restored after a crash.

We'd then have the ability to show a warning to the user.

Additional Context

This related issue about the web view crashing when taking photos: #2265

@jcesarmobile
Copy link
Member

Since appRestoredResult is Android only, we could just try to implement it for iOS too, but that won't work for your user case as it passes info from the last plugin call and you are not using a plugin.

@sandstrom
Copy link
Contributor Author

@jcesarmobile Yeah, I looked at that event too, but seems like it has slightly different semantics/meaning. It's more of a switching thing.

Maybe a separate event for crashes would make more sense, e.g. restartedAfterCrash or similar.

@diachedelic
Copy link
Contributor

Would it be so crazy to crash the app if webViewWebContentProcessDidTerminate was called? I think I would prefer the app to quit immediately and obviously, rather than data disappearing silently. Perhaps it could be a configuration option.

@jcesarmobile
Copy link
Member

Yeah, it’s crazy to crash an iOS app, apple will reject apps if they crash

@diachedelic
Copy link
Contributor

In that case I support @sandstrom 's idea of something like a webViewWebContentProcessDidTerminate event. In my case the webview was crashing intermittently due to a memory leak, which was fairly trivial to fix, but painful to diagnose due to the absence of any diagnostic.

@diachedelic
Copy link
Contributor

Just another though – silently restarting the webview can leave plugins in an inconsistent state. For example, if you have called Geolocation.watchPosition, and the webview crashes and reloads, there is no way to halt location updates other than manually killing and restarting the app.

I have been running Capacitor in development with the below modification for months now, and I would love to see this behaviour behind a configuration option.

diff --git a/CAPBridgeViewController.swift b/CAPBridgeViewController.swift
index 431c46ae..fb1c8bdd 100644
--- a/CAPBridgeViewController.swift
+++ b/CAPBridgeViewController.swift
@@ -344,7 +344,7 @@ public class CAPBridgeViewController: UIViewController, CAPBridgeDelegate, WKScr
   }
 
   public func webViewWebContentProcessDidTerminate(_ webView: WKWebView) {
-    webView.reload()
+    fatalError("webViewWebContentProcessDidTerminate")
   }

@dodomui
Copy link

dodomui commented Jan 20, 2022

Does this work?

public func webViewWebContentProcessDidTerminate(_ webView: WKWebView) { 
   webView.reload() 
 } 

@diachedelic
Copy link
Contributor

I believe that is the default behaviour...

@dodomui
Copy link

dodomui commented Jan 21, 2022

@diachedelic cannot find this function at Capacitor 3.3.4 @capacitor/ios/Capacitor/Capacitor/CapacitorBridge.swift

@diachedelic
Copy link
Contributor

Oh wow. So what happens now when the webview crashes on iOS?

@KevinKelchen
Copy link

Not sure what happens now, but it appears the reload was just added back in #5391

I'm glad it was added back as we've had issues where the OS kills the WebView process and we need to recover (JetSam or what-have-you).

We plan to very soon add logging to Sentry when it occurs so we can learn more about it happening in production and make better informed decisions on our technical approaches. That's why I'm interested in this issue. Not sure exactly what we'll do at the moment to capture the automatic reload event but I have some not-amazing ideas to try out. 🙂

@diachedelic
Copy link
Contributor

diachedelic commented Jan 28, 2022

We have found the iOS WebView to be very sensitive to memory usage. It crashes quite often on startup, possibly due to this bug: https://bugs.webkit.org/show_bug.cgi?id=212790. If you have any memory leaks in the WebView, it will eventually be terminated. WebKit unfortunately has some nasty memory leaks associated with the <video> and <canvas> elements.

In general, an automatic reload is necessary in production. But I would prefer a crash in development. And ideally, we should be able to override the default behaviour, so we can maintain the integrity of the app.

@KevinKelchen
Copy link

Yep, it can be rough.

Using more resource/memory-intensive technologies like WebGL (used in modern map control JS libraries) can exacerbate it as well. We'd like to embrace that technology but are held back.

@briandpeterson
Copy link

Yes! We need a hook in as well to know when this happens. Maybe having a event that I can hook into so I can "embrace the crash" and re-hydrate my application to where they left off? iOS is not getting any better with new devices. It seems the govenator is set with 5 year old specs in mind :(

@KevinKelchen
Copy link

I posted a Q&A Discussion item related to detecting the involuntary reload for anyone who's curious.

It also summarizes our approach to informing the JS layer about the reload (albeit not with an event).

@rossholdway
Copy link

We're also running into the webview crash when switching back to our backgrounded app after using an intensive app (such as a game). We suspect it's due to low RAM.

We've found that for our Angular applicationwebView.reload() causes it to run in an inconsistent state. A normal reload is fine, but after a webView.reload() there are a number of strange JavaScript related issues including issues with zone.js, routing and Angular lifecycle events not firing.

Interestingly, webView.load(currentUrl) seems to return the application in a more stable state, but we're still left with abandoned listeners etc as mentioned by #2379 (comment)

@Eric-Savvi
Copy link

@rossholdway what you are describing in the last comment here is exactly what we are currently experiencing. We have built an Ionic app that crashes when coming back from the background mode after a heavy load in a separate app like the camera or a game. So far we are not able to correctly restart our application to be in a working state again.

Therefore it would be very interesting to know, if you have been able to solve this issue?

@peitschie
Copy link

Just as a point of interest, I have a patch up for Android at least that works to expose this crash event through the plugin interfaces. Hypothetically, it should be possible to build a plugin that catches and restarts the app when this case is detected: #6416

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants