apache / cordova-plugin-inappbrowser

Apache Cordova InAppBrowser Plugin
https://cordova.apache.org/
Apache License 2.0
1.11k stars 2.14k forks source link

IllegalArgumentException when closing InAppBrowser on android #1007

Open predatorx7 opened 11 months ago

predatorx7 commented 11 months ago

Bug Report

Problem

We are facing a crash (reported from Google Play Console), that is happening on some phones, mentioned InAppBrowser in the stack trace.

The app seems to be crashing when closing the in-app browser. I see that a very similar issue was closed in https://github.com/apache/cordova-plugin-inappbrowser/pull/616 but we're using plugin version 5 and the issue was still experienced by a user last week.

Information

We could not reproduce the issue on our devices, I'm sharing screenshots from the console.

image image

Logs

Exception java.lang.IllegalArgumentException:
  at android.view.WindowManagerGlobal.findViewLocked (WindowManagerGlobal.java:529)
  at android.view.WindowManagerGlobal.removeView (WindowManagerGlobal.java:438)
  at android.view.WindowManagerImpl.removeViewImmediate (WindowManagerImpl.java:126)
  at android.app.Dialog.dismissDialog (Dialog.java:375)
  at android.app.Dialog.dismiss (Dialog.java:358)
  at org.apache.cordova.inappbrowser.InAppBrowser$6$1.onPageFinished (InAppBrowser.java:534)
  at YM4.a (chromium-Monochrome.aab-stable-<US_SOCIAL_SECURITY_NUMBER>:425)
  at CA.handleMessage (chromium-Monochrome.aab-stable-<US_SOCIAL_SECURITY_NUMBER>:220)
  at android.os.Handler.dispatchMessage (Handler.java:106)
  at android.os.Looper.loop (Looper.java:201)
  at android.app.ActivityThread.main (ActivityThread.java:6810)
  at java.lang.reflect.Method.invoke
  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:547)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:873)

Environment, Platform, Device

Android 9 (SDK 28), Xiaomi Redmi Note 5 Pro.

Version information

cordova-plugin-inappbrowser: v5.0.0 cordova android version: ^10,0,0

Ionic:

Ionic CLI : 5.4.16 Ionic Framework : @ionic/angular 6.5.1 @angular-devkit/build-angular : 14.2.10 @angular-devkit/schematics : 14.2.10 @angular/cli : 14.2.10 @ionic/angular-toolkit : 6.1.0

Cordova:

Cordova CLI : 11.0.0 Cordova Platforms : android 10.1.2 Cordova Plugins : cordova-plugin-ionic-keyboard 2.2.0, cordova-plugin-ionic-webview 5.0.0, (and 27 other plugins)

Utility:

cordova-res : 0.15.4 native-run : 1.7.2

System:

Android SDK Tools : 26.1.1 (C:\Users\pcadm\AppData\Local\Android\Sdk) NodeJS : v16.10.0 (C:\Program Files\nodejs\node.exe) npm : 7.24.0 OS : Windows 10

Checklist

breautek commented 10 months ago

I'm not that familiar with this plugin's codebase but did some exploratory digging and

at org.apache.cordova.inappbrowser.InAppBrowser$6$1.onPageFinished (InAppBrowser.java:534)

Is when it reaches Cordova code, which is a part that calls on the Android SDK.

https://github.com/apache/cordova-plugin-inappbrowser/blob/8ee1d3882163ee1dedb86fb3d0d1f8b7e16c3e42/src/android/InAppBrowser.java#L532-L540

Line 534 is the code entering the onPageFinished callback, in which Cordova tests for an existence of and attempts to dismiss() an [android.app.Dialog](https://developer.android.com/reference/android/app/Dialog#dismiss()).

The Android SDK claims that:

This method can be invoked safely from any thread.

and doesn't document any other causes for an IllegalArgumentException.

I however did find hints at SO that suggest that this exception will raise if attempting to dismiss a dialog when the Dialog is already destroyed. I think we protect ourselves by explicitly setting the dialog to null after we dismiss it, but this might not be sufficient as there are likely other user actions that can happen that can cause a dialog to be dismissed.

I think test code needs to be added to confirm that double-dismissing a dialog (or at least attempting to dismiss it while isShowing() is false) will trigger this kind of error. If it does, then adding a isShowing() condition check before dismissing might be a solution.

And if Cordova needs to clean up anything(like setting the dialog to null), I think the onStop hook should be used.