DialogInterface in DialogFragment.onDismiss() might be null but is marked as NonNull [141974033]

Fixed

Bug

Status Update

No update yet.

Description

[Deleted User]

created issue #1

Oct 2, 2019 12:35PM

Component used: DialogFragment
Version used: 1.0.2
Devices/Android versions reproduced on: I did not reproduce it myself but I do have stack traces from Samsun/Huawei/Xiaomi Android 8 and 9

DialogInterface param of DialogFragment.onDismiss() function is marked as @NonNull
```

@Override
public void onDismiss(@NonNull DialogInterface dialog) {
if (!mViewDestroyed) {
// Note: we need to use allowStateLoss, because the dialog
// dispatches this asynchronously so we can receive the call
// after the activity is paused. Worst case, when the user comes
// back to the activity they see the dialog again.
dismissInternal(true);
}
}
```

...but I've seen crashes in my app with stack traces showing that this param was null

```
Fatal Exception: java.lang.IllegalArgumentException: Parameter specified as non-null is null: method l.w.d.j.b, parameter dialog
at com.my.app.ui.dialog.MyCustomDialog.onDismiss + 88(MyCustomDialog.java:88)
at android.app.Dialog$ListenersHandler.handleMessage + 1503(Dialog.java:1503)
at android.os.Handler.dispatchMessage + 105(Handler.java:105)
at android.os.Looper.loop + 164(Looper.java:164)
at android.app.ActivityThread.main + 6944(ActivityThread.java:6944)
at java.lang.reflect.Method.invoke(Method.java)
at com.android.internal.os.Zygote$MethodAndArgsCaller.run + 327(Zygote.java:327)
at com.android.internal.os.ZygoteInit.main + 1374(ZygoteInit.java:1374)
```

Dialog source code shows that the DialogInterface can be indeed null

https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/app/Dialog.java#1407

This is kind of problematic because the app written in Kotlin crashes and you can't really fix this without using Java code.
Kotlin does not allow us to mark DialogInterface as a nullable value.

Comments

il...@google.com <il...@google.com> #2Oct 5, 2019 12:08AM

Assigned to jb...@google.com.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

ap...@google.com <ap...@google.com> #3Oct 16, 2019 02:26AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit ad4b89664227dc5eaf1265371892afc9b693fcd9
Author: Jeremy Woods <jbwoods@google.com>
Date: Tue Oct 15 13:10:59 2019

Drop DialogFragment callbacks if Dialog is gone

Cancel requests are posted and as a result a Dialog can get a onCancel()
callback even after the dialog has already been destroyed.

This change uses explicit listeners to make the indirect call to the
DialogFragment onDismiss() and onCancel() callback methods.

Test: Added tests
BUG: 140149502
BUG: 141974033
Change-Id: I21e767ee558a72cddec0564faf01cce2389f14eb

M fragment/fragment/src/androidTest/java/androidx/fragment/app/DialogFragmentDismissTest.kt
M fragment/fragment/src/androidTest/java/androidx/fragment/app/DialogFragmentTest.kt
M fragment/fragment/src/main/java/androidx/fragment/app/DialogFragment.java

https://android-review.googlesource.com/1143185

https://goto.google.com/android-sha1/ad4b89664227dc5eaf1265371892afc9b693fcd9

jb...@google.com <jb...@google.com> #4Oct 16, 2019 02:31AM

Marked as fixed.

This has been fixed internally and will be available in the Fragment 1.2.0-rc01 release.

an...@google.com <an...@google.com> #5Oct 16, 2019 06:08PM

The following changes were cherrypicked through Coastguard (

https://android-build.googleplex.com/coastguard/)
Release Track: (

https://android-build.googleplex.com/coastguard/dashboard/6552735953321984/#/cherrypickHistory/all)
Created: 2019-10-16 18:06:55.006249+00:00
Changes: aosp/1143185

Issue 141974033

Description

Issue summary

Comments

il...@google.com <il...@google.com> #2Oct 5, 2019 12:08AM

ap...@google.com <ap...@google.com> #3Oct 16, 2019 02:26AM

jb...@google.com <jb...@google.com> #4Oct 16, 2019 02:31AM

an...@google.com <an...@google.com> #5Oct 16, 2019 06:08PM

Add comment

Issue metadata