Add Lint check for enforcing the use of viewLifecycleOwner when observing a LiveData in Fragment [137122478]

Fixed

Bug

Status Update

No update yet.

Description

pi...@gmail.com

created issue #1

Jul 9, 2019 10:18PM

Component used: Fragment
Version used: 1.2.0-alpha01
Devices/Android versions reproduced on:

Observing a LiveData in OnViewCreated using this as the LifecycleOwner can lead to unexpected behaviors on fragment re-attachment.
Instead, getViewLifecycleOwner() should be used as the LifecyclerOwner.

Having a Lint check that warns when a LiveData is observed with this inside a Fragment could help the new users of Fragment and LiveData.

Comments

il...@google.com <il...@google.com> Jul 31, 2019 11:02PM

Assigned to jb...@google.com.

il...@google.com <il...@google.com> Oct 2, 2019 08:06PM

Reassigned to fr...@google.com.

il...@google.com <il...@google.com> #2Oct 4, 2019 05:45PM

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 14, 2019 10:42PM

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

commit d26126d23175fc55c77788b95989f1ecc873eb9c
Author: Matthew Fraschilla <fraschilla@google.com>
Date: Mon Oct 07 13:05:15 2019

Add lint check for unsafe livedata observe in Fragments

A Fragment's lifecycle can last longer than its view resulting in
LiveData objects being observed longer than intended. This change
introduces a lint check to warn developers that they should use the
getViewLifecyclOwner() method as the LifecycleOwner when observing
LiveData objects.

Test: new FragmentLiveDataObserveDetectorTest
Bug: 137122478
Change-Id: If002f6be74d9b943f1b43cf2515b20337489b704

A fragment/fragment-lint/src/main/java/androidx/fragment/lint/FragmentLiveDataObserverDetector.kt
A fragment/fragment-lint/src/main/java/androidx/fragment/lint/LintUtils.kt
A fragment/fragment-lint/src/test/java/androidx/fragment/lint/FragmentLiveDataObserveDetectorTest.kt
A fragment/fragment-lint/src/test/java/androidx/fragment/lint/stubs/Stubs.kt

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

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

il...@google.com <il...@google.com> #4Oct 14, 2019 11:07PM

Marked as fixed.

We've added this Lint check internally and it will be available in Fragment 1.2.0-rc01

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/1137099

pa...@gmail.com <pa...@gmail.com> #6Nov 11, 2019 10:34PM

It seems the FragmentLiveDataObserve lint check is missing cases for, when one uses the observe extension function and not the observe function found on the LiveData class.
I created a repo

https://github.com/patrickusiewicz/missing-lint-check-for-observe to show the issue.

il...@google.com <il...@google.com> #7Nov 11, 2019 10:40PM

Re #6 - please file a separate issue.

l....@gmail.com <l....@gmail.com> #8Dec 6, 2022 11:34AM

Comment has been deleted.

Message last modified on Dec 6, 2022 11:34AM