Add workaround for Robolectric not posting requestApplyInsets() [137201343]

Fixed

Bug

Status Update

No update yet.

Description

il...@google.com

created issue #1

Jul 10, 2019 05:44PM

Component used: Fragment
Version used: 1.2.0-alpha01

When running a test in Robolectric, the new behavior introduced in

https://issuetracker.google.com/issues/135945162 causes issues when you have a ViewPager since Robolectric, instead of properly posting the requestApplyInsets (and the subsequent measure/layout) runs it inline, causing an IllegalStateException when FragmentPagerAdapter / FragmentStatePagerAdapter use commitNowAllowingStateLoss() when the FragmentManager is in the middle of processing transactions.

We should continue to use commitNowAllowingStateLoss(), but catch the IllegalStateException and fall back to commitAllowingStateLoss() specifically in the case where we're already performing transactions.

Comments

il...@google.com <il...@google.com> Jul 10, 2019 06:13PM

Accepted by il...@google.com.

ap...@google.com <ap...@google.com> #2Jul 10, 2019 07:59PM

Marked as fixed.

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

om...@gmail.com <om...@gmail.com> #3Jan 6, 2020 09:24PM

Using version: fragmentx: 1.2.0-rc4

If multiple finishUpdate() calls are made, it is possible that commitNowAllowingStateLoss() can throw multiple IllegalStateExceptions, which will cause an NPE since mCurTransaction is set to null after the try/catch method. See the stacktrace below:

java.lang.NullPointerException
at androidx.fragment.app.FragmentStatePagerAdapter.finishUpdate(FragmentStatePagerAdapter.java:267)
at androidx.viewpager.widget.ViewPager.populate(ViewPager.java:1244)
at androidx.viewpager.widget.ViewPager.populate(ViewPager.java:1092)
at androidx.viewpager.widget.ViewPager.onMeasure(ViewPager.java:1622)
at android.view.View.measure(View.java:22071)
at androidx.constraintlayout.widget.ConstraintLayout.internalMeasureChildren(ConstraintLayout.java:1227)
at androidx.constraintlayout.widget.ConstraintLayout.onMeasure(ConstraintLayout.java:1572)
at android.view.View.measure(View.java:22071)
at android.view.ViewGroup.measureChildWithMargins(ViewGroup.java:6602)
at android.widget.FrameLayout.onMeasure(FrameLayout.java:185)
at android.view.View.measure(View.java:22071)
at android.view.ViewGroup.measureChildWithMargins(ViewGroup.java:6602)
at android.widget.FrameLayout.onMeasure(FrameLayout.java:185)
at androidx.appcompat.widget.ContentFrameLayout.onMeasure(ContentFrameLayout.java:143)
at android.view.View.measure(View.java:22071)
at android.view.ViewGroup.measureChildWithMargins(ViewGroup.java:6602)
at android.widget.LinearLayout.measureChildBeforeLayout(LinearLayout.java:1514)
at android.widget.LinearLayout.measureVertical(LinearLayout.java:806)
at android.widget.LinearLayout.onMeasure(LinearLayout.java:685)
at android.view.View.measure(View.java:22071)
at android.view.ViewGroup.measureChildWithMargins(ViewGroup.java:6602)
at android.widget.FrameLayout.onMeasure(FrameLayout.java:185)
at android.view.View.measure(View.java:22071)
at android.view.ViewGroup.measureChildWithMargins(ViewGroup.java:6602)
at android.widget.LinearLayout.measureChildBeforeLayout(LinearLayout.java:1514)
at android.widget.LinearLayout.measureVertical(LinearLayout.java:806)
at android.widget.LinearLayout.onMeasure(LinearLayout.java:685)
at android.view.View.measure(View.java:22071)
at android.view.ViewGroup.measureChildWithMargins(ViewGroup.java:6602)
at android.widget.FrameLayout.onMeasure(FrameLayout.java:185)
at com.android.internal.policy.DecorView.onMeasure(DecorView.java:724)
at android.view.View.measure(View.java:22071)
at android.view.ViewRootImpl.performMeasure(ViewRootImpl.java:2422)
at android.view.ViewRootImpl.measureHierarchy(ViewRootImpl.java:1504)
at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1761)
at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1392)
at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:6752)
at android.os.Handler.handleCallback(Handler.java:790)
at android.os.Handler.dispatchMessage(Handler.java:99)
at org.robolectric.shadows.ShadowMessageQueue.dispatchMessage(ShadowMessageQueue.java:148)
at org.robolectric.shadows.ShadowMessageQueue.access$200(ShadowMessageQueue.java:38)
at org.robolectric.shadows.ShadowMessageQueue$1.run(ShadowMessageQueue.java:126)
at org.robolectric.util.Scheduler$ScheduledRunnable.run(Scheduler.java:386)
at org.robolectric.util.Scheduler.runOneTask(Scheduler.java:278)
at org.robolectric.util.Scheduler.advanceTo(Scheduler.java:260)
at org.robolectric.util.Scheduler.advanceBy(Scheduler.java:243)
at org.robolectric.util.Scheduler.advanceBy(Scheduler.java:233)
at org.robolectric.util.Scheduler.setIdleState(Scheduler.java:88)
at org.robolectric.util.Scheduler.unPause(Scheduler.java:123)
at org.robolectric.shadows.ShadowLooper.unPause(ShadowLooper.java:347)
at org.robolectric.shadows.ShadowLooper.runPaused(ShadowLooper.java:398)
at org.robolectric.android.controller.ActivityController.visible(ActivityController.java:161)

ap...@google.com <ap...@google.com> #4Jan 21, 2020 09:34PM

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

commit eb154034645a875304797a38995baac5ddd3346d
Author: Jeremy Woods <jbwoods@google.com>
Date: Tue Jan 14 12:40:54 2020

Avoid try/catching exceptions in FragmentPagerAdapters

When FragmentPagerAdapter or FragmentStatePagerAdapter
runs finishUpdate() in response to a measure/layout
pass, it runs commitNowAllowingStateLoss() as all
normal devices don't do measure/layout while processing
pending FragmentTransactions, but Robolectric will. In
order to ensure that Robolectric continued to work,

b/137201343

provided behavior to fallback to
commitAllowingStateLoss() when processing pending
FragmentTransactions. It used a try/catch for
IllegalStateExceptions, which means valid exceptions
could be ignored.

This change drops any transactions that attempt to be
committed while the FragmentManager is already executing
transactions.

Test: FragmentPagerAdapterRoboTest
Fixes: 147359544

Change-Id: I21ebbfe67d349980003b6df77704f005e693e664

M fragment/fragment/src/main/java/androidx/fragment/app/FragmentPagerAdapter.java
M fragment/fragment/src/main/java/androidx/fragment/app/FragmentStatePagerAdapter.java
A fragment/fragment/src/test/java/androidx/fragment/app/FragmentPagerActivity.kt
A fragment/fragment/src/test/java/androidx/fragment/app/FragmentPagerAdapterRoboTest.kt

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

an...@google.com <an...@google.com> #5Jan 22, 2020 12:10AM

The following changes were cherrypicked through Coastguard (

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

https://android-build.googleplex.com/coastguard/dashboard/6552735953321984/#/history/all)
Created: 2020-01-22 00:07:09.044119+00:00
Changes: aosp/1208711

Issue 137201343