Added fail-safe check to catch MissingWebViewPackage Exception.

[KunalFarmah98]

Summary

The check implemented in PR #29089 is flawed as the exception class name and message depends on the OS version as well as the OEM. In OxygenOS running android 11, it comes out as RuntimeException and the check fails and hence the crash occurs again. But on observing closely, its clear that the exception message is consistent across OEMs with similar strings appearing in them that have been included in the if check.
Hence there is a simple fix to this issue, by checking the message instead of the exception class.

Here is the snippet:

private @Nullable CookieManager getCookieManager() {
    if (mCookieManager == null) {
      possiblyWorkaroundSyncManager(mContext);
      try {
        mCookieManager = CookieManager.getInstance();
      } catch (IllegalArgumentException ex) {
        // https://bugs.chromium.org/p/chromium/issues/detail?id=559720
        return null;
      } catch (Exception exception) {
        String message = exception.getMessage();
        // We cannot catch MissingWebViewPackageException as it is in a private / system API
        // class. This validates the exception's message to ensure we are only handling this
        // specific exception.
        // The exception class doesn't always contain the correct name as it depends on the OEM
        // and OS version. It is better to check the message for clues regarding the exception
        // as that is somewhat consistent across OEMs.
        // For instance, the Exception thrown on OxygenOS 11 is a RuntimeException but the message contains the required strings.
        // https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/webkit/WebViewFactory.java#348
       if (exception.getClass().getCanonicalName().contains("MissingWebViewPackageException") || 
            (message!=null && (message.contains("WebView provider") || 
               message.contains("No WebView installed")))){
            return null;
        } else {
          throw exception;
        }
      }
    }

    return mCookieManager;
  }

Changelog

[General] [Added] - A fail proof check to catch any crash involving webview:
if (exception.getClass().getCanonicalName().contains("MissingWebViewPackageException") || (message!=null && (message.contains("WebView provider") || message.contains("No WebView installed"))))
[General] [Removed] - Flawed check to catch WebViewProvider crash:
if (message != null && exception.getClass().getCanonicalName().contains("MissingWebViewPackageException"))

Test Plan

This code has been tested rigorously on OnePlus Nord CE 5G (Oxygen OS 11.0) and Realme X (Realme UI 2.0) both running on Android 11 and reproducing the crash on a hybrid (Native android + ReactNative) app that showed this crash in production being dependent on WebViews. I have implemented an entire patched fork of 0.67.2 to fix this issue in our app.

How to test:

Launch any app that has a webview.
Go to settings->apps->Android System Webview -> disable.
Resume/Restart the app.

In this fix:
Open a webview and notice that the app will not crash.

In current version:
App crashes on startup as the exception escapes the catch block.
Even if it survives startup (did on Realme X), it will crash once you try to open a webview.

[analysis-bot]

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

Base commit: 7cece34
Branch: main

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,188,990	-118
android	hermes	armeabi-v7a	7,789,417	-122
android	hermes	x86	8,559,080	-124
android	hermes	x86_64	8,511,464	-124
android	jsc	arm64-v8a	9,857,690	+85
android	jsc	armeabi-v7a	8,843,004	+93
android	jsc	x86	9,824,381	+89
android	jsc	x86_64	10,420,850	+86

Base commit: 7cece34
Branch: main

[robhogan]

Thanks for looking into this. It looks like that check (contains any of WebView provider, No WebView installed, WebView) has some redundancy and could be rewritten as message.contains("WebView").

If #29089 (comment) is correct, that's not guaranteed to catch all of these cases, where the name might contain WebView but the message doesn't. I'm not an android dev, but it also seems quite wide to me and open to false positives, though maybe that's not so much an issue in this situation.

Combining the old check and yours would certainly be wide enough, but maybe too wide, ie.

exception.getClass().getCanonicalName().contains("MissingWebViewPackageException") || 
(message != null && message.contains("WebView")))

Looks like @sota000 dealt with this previously and might have some thoughts here?

[KunalFarmah98]

Thanks for looking into this. It looks like that check (contains any of WebView provider, No WebView installed, WebView) has some redundancy and could be rewritten as message.contains("WebView").

If #29089 (comment) is correct, that's not guaranteed to catch all of these cases, where the name might contain WebView but the message doesn't. I'm not an android dev, but it also seems quite wide to me and open to false positives, though maybe that's not so much an issue in this situation.

Combining the old check and yours would certainly be wide enough, but maybe too wide, ie.

exception.getClass().getCanonicalName().contains("MissingWebViewPackageException") || 
(message != null && message.contains("WebView")))

Looks like @sota000 dealt with this previously and might have some thoughts here?

@rh389 I added the first 2 checks only to prevent false positives and added a fail-save in case exception does not contain a clear message. We are only using the message.contains("Webview") in the patch used in our app because that is the only use case for our app's features and we have seen 2 different types of messages in our crash logs.

I only tried to make the check slightly less generic.

[robhogan]

I only tried to make the check slightly less generic.

Appreciate that was that was the intention, but the end result is that the error will be suppressed if and only if the message contains WebView. Because the other two strings themselves contain WebView, they're strictly stronger conditions and serve no purpose when you || them together with the weak check.

[KunalFarmah98]

I only tried to make the check slightly less generic.

Appreciate that was that was the intention, but the end result is that the error will be suppressed if and only if the message contains WebView. Because the other two strings themselves contain WebView, they're strictly stronger conditions and serve no purpose when you || them together with the weak check.

Hey @rh389, I have updated the PR with the new check

        if (exception.getClass().getCanonicalName().contains("MissingWebViewPackageException") || 
            (message!=null && (message.contains("WebView provider") ||
              message.contains("No WebView installed")))) {
            return null;

[cortinico]

Appreciate that was that was the intention, but the end result is that the error will be suppressed if and only if the message contains WebView. Because the other two strings themselves contain WebView, they're strictly stronger conditions and serve no purpose when you || them together with the weak check.

That's a good point @rh389 👍

I believe we can merge this. I believe there might be some linting issue to address as the formatting looks a bit odd. But from the content point of view we're good to go 👍

[facebook-github-bot]

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

[react-native-bot]

This pull request was successfully merged by Kunal Farmah in 8c573d9.

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