react-native icon indicating copy to clipboard operation
react-native copied to clipboard

fix: Crash if WebView is disabled

Open Pajn opened this issue 1 year ago • 8 comments

Summary

When the webview is updated by play store, all apps that has loaded webview gets killed. For background see https://issuetracker.google.com/issues/228611949?pli=1

We have a long-running app and don't want this to happen and while we are not using webview anywhere, we are using websockets that loads it for reading cookies, however we are not using cookies to authenticate the websocket and thus want to disable the webview to avoid loading it unnecessarily and then getting killed unnecessarily.

The webview has support for beeing disabled https://developer.android.com/reference/android/webkit/WebView#disableWebView() however as-is this crashes React Native. It seems to me like this case is very similar to not having web view installed and should be handled in the same way.

Changelog

[Android] [Fixed] - Avoid crash in ForwardingCookieHandler if webview is disabled

Test Plan

Add WebView.disableWebView(); as the first line in MainActivity.onCreate.

Pajn avatar Aug 23 '22 17:08 Pajn

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,617,924 +12
android hermes armeabi-v7a 7,032,157 -6
android hermes x86 7,917,986 +13
android hermes x86_64 7,891,625 +35
android jsc arm64-v8a 9,495,632 -62
android jsc armeabi-v7a 8,273,240 -77
android jsc x86 9,433,426 -53
android jsc x86_64 10,026,421 -43

Base commit: 2452c5f16e8bbbf6f17d09e77d3b8f073837bf18 Branch: main

analysis-bot avatar Aug 23 '22 17:08 analysis-bot

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Aug 24 '22 16:08 facebook-github-bot

Hi @Pajn, thanks for the PR. Could you please rebase on main? We should have fixed the CI issues.

cipolleschi avatar Aug 24 '22 16:08 cipolleschi

Sorry to ask that, but could you rebase again? The CI is still red but for some other commits that landed in between.. :(

cipolleschi avatar Aug 26 '22 13:08 cipolleschi

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 2452c5f16e8bbbf6f17d09e77d3b8f073837bf18 Branch: main

analysis-bot avatar Aug 26 '22 14:08 analysis-bot

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Aug 26 '22 15:08 facebook-github-bot

Sorry, my additions skipped the earlier nullcheck. It should be correct now.

Pajn avatar Aug 26 '22 16:08 Pajn

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Aug 26 '22 16:08 facebook-github-bot

This pull request was successfully merged by @Pajn in 5451cd48bd0166ba70d516e3a11c6786bc22171a.

When will my fix make it into a release? | Upcoming Releases

react-native-bot avatar Aug 30 '22 14:08 react-native-bot