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

bug + fix: Push Notification handlers not called after webViewWebContentProcessDidTerminate #6676

Open
hermitdemschoenenleben opened this issue Jun 26, 2023 · 8 comments

Comments

@hermitdemschoenenleben
Copy link
Contributor

hermitdemschoenenleben commented Jun 26, 2023

Bug Report

In our ios app we found that a click on a push message sometimes doesn't execute the javascript push message handlers. We found out that this happens after the webview process has been shut down in background (see also #5488 and #2379) which may happen due to memory pressure. This shutdown calls webViewWebContentProcessDidTerminate which reloads the app. After this reload, push notification handlers aren't executed.
This issue can be reproduced reliably (see code reproduction below).

Update: I now think this problem isn't specific to the push notifications plugin. I think it's a general problem caused by not clearing plugins' event listeners after reloading the webview. See my comment for a suggested fix.

Capacitor Version

💊   Capacitor Doctor  💊

Latest Dependencies:

  @capacitor/cli: 5.0.5
  @capacitor/core: 5.0.5
  @capacitor/android: 5.0.5
  @capacitor/ios: 5.0.5

Installed Dependencies:

  @capacitor/android: not installed
  @capacitor/cli: 5.0.5
  @capacitor/core: 5.0.5
  @capacitor/ios: 5.0.5

[success] iOS looking great! 👌

Platform(s)

ios

Current Behavior

After webViewWebContentProcessDidTerminate, a tap on a push message just opens the app but doesn't call pushNotificationActionPerformed.

Expected Behavior

After app-reload following webViewWebContentProcessDidTerminate, push messages that were delivered to the native layer in the meantime should be replayed such that the web app may handle them.

Code Reproduction

  • Clone https://github.com/hermitdemschoenenleben/pushmsgtester
  • Run it in iPhone simulator
  • Go the ios homescreen in order to view incoming push messages
  • Drag and drop the file test-push-message.apns to the simulator to simulate a push message
  • Click on the notification -> the app opens and an alert shows: push message click was recognized by the app
  • Now, go to the ios homescreen again
  • On your mac, execute kill $(pgrep -P $(pgrep launchd_sim) 'com.apple.WebKit.WebContent') to kill the webview process (taken from https://stackoverflow.com/a/73609765/5895643 ). This simulates what happens under memory pressure.
  • Drag and drop test-push-message.apns to the simulator again
  • Click on the notification -> the app opens. This time, no alert is shown, though, as the push message handler is not called after a webview crash

Additional Context

@hermitdemschoenenleben
Copy link
Contributor Author

I investigated the bug a little further and I think the fix is quite easy: adding this code in webViewWebContentProcessDidTerminate seems to work:

for pluginObject in bridge!.plugins {
  let plugin = pluginObject.value
  plugin.eventListeners!.removeAllObjects()
}

Note: as this problem may occur with every plugin, I think it makes sense to not just tackle it with the push notifications plugin. As all event listeners are dead anyway after reloading the webview, I think it makes sense to clear them completely on the native side as well.

@hermitdemschoenenleben hermitdemschoenenleben changed the title bug: Push Notification handlers not called after webViewWebContentProcessDidTerminate bug + fix: Push Notification handlers not called after webViewWebContentProcessDidTerminate Aug 10, 2023
@vilicvane
Copy link

I was experiencing this problem for a long time. It is not until today I realized it's caused by webview crash. And the fix looks really simple and promising! Testing it now and hope it works.

@vilicvane
Copy link

vilicvane commented Sep 2, 2023

I investigated the bug a little further and I think the fix is quite easy: adding this code in webViewWebContentProcessDidTerminate seems to work:

for pluginObject in bridge!.plugins {
  let plugin = pluginObject.value
  plugin.eventListeners!.removeAllObjects()
}

Note: as this problem may occur with every plugin, I think it makes sense to not just tackle it with the push notifications plugin. As all event listeners are dead anyway after reloading the webview, I think it makes sense to clear them completely on the native side as well.

I tried this but it does not work reliably. I think the problem might be webViewWebContentProcessDidTerminate invoking just after app resuming after a long time of hibernation, and pushNotificationActionPerformed actually triggered earlier.

So I added the some code to test and retry webView JavaScript evaluation before pushNotificationActionPerformed in PushNotificationHandler.swift (didReceive()):

self.plugin?.bridge?.webView?.evaluateJavaScript("void window.Capacitor.fromNative", completionHandler: {(_, error) in
    if let error = error {
        CAPLog.print("⚡️  Failed to test webview (crashed?)")
        CAPLog.print("⚡️  Error: " + error.localizedDescription)

        DispatchQueue.main.asyncAfter(deadline: .now() + 1) {
            self.didReceive(response: response)
        }
    } else {
        self.plugin?.notifyListeners("pushNotificationActionPerformed", data: data, retainUntilConsumed: true)
    }
})

Ideally this should be added to notifyListeners but it's good enough for me now.


Update: it seems that this approach is neither stable, still needs some improvement.

@hermitdemschoenenleben
Copy link
Contributor Author

hermitdemschoenenleben commented Sep 4, 2023

@vilic interesting idea! Do you use it in combination with my fix or just yours?

I tried this but it does not work reliably

when I tested it as described in my initial post, it worked reliably for me. How did you test it? On a simulator or real device? And which ios / iphone version did you use?

@vilicvane
Copy link

vilicvane commented Sep 9, 2023

@hermitdemschoenenleben Just mine. I test it by waiting till the second day and click the morning notification. 😂

I tried to simulate the situation by deliberately creating out of memory situation to crash Safari, but as the app is still active, the behavior seems different. As I don't know how long is long enough, I just wait for a whole night...

And I tested on a real device with latest stable iOS 16.

@ZoneMR
Copy link

ZoneMR commented Oct 15, 2023

I can confirm that although progress on this issue appears to have reached a plateau, it's still an important issue.

As it stands, push notification handlers are intermittently broken, with actions not notified to the app if it's not in a "fresh enough" state in the background.

@graemeenglish
Copy link

Seeing this also.

In our case a reliable reproduction (without any artificial process killing) is to leave the app overnight. In the morning, the first notification fails to invoke the pushNotificationActionPerformed listener.

@ZoneMR
Copy link

ZoneMR commented Mar 20, 2024

This is a fundamental and frequent use case - and it's been broken for almost a year. I would urge it's given some attention, as I'm at a loss for how the issue can be worked around.

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

No branches or pull requests

5 participants