setDefaultNightMode does not update resources nor trigger activity onNightModeChanged in some instances [188681415]

Fixed

Bug

adexe s nau

Status Update

No update yet.

Description

cf...@coyote-group.com

created issue #1

May 20, 2021 12:34PM

Version used: 1.4.0-alpha01
Devices/Android versions reproduced on: pretty much any device and api (seen on emulators/Huawei/Samsungs, api 27/29/30)

If you have a stack of activities with android:configChanges="uiMode" and you call AppCompatDelegate.setDefaultNightMode it can cause other activities resources (1 or more) not to be updated (and onNightModeChanged not triggered).
It happens when the setDefaultNightMode is called from another thread that the top activity (with a handler Handler(Looper.getMainLooper()).post {} or RX AndroidSchedulers.mainThread().scheduleDirect {})

Eg :
Activity A with uiMode
Activity B with uiMode
Activity C with uiMode

Here is your stack : A > B > C (C on top)
Call AppCompatDelegate.setDefaultNightMode with a new mode on activity C (but not directly from this activity, ex with RX AndroidSchedulers.mainThread or an handler)
Activity C receives the onConfigurationChanged/onNightModeChanged, Activity B receives the onNightModeChanged but one time out of two activity A not receives the onNightModeChanged (and the resources are indeed not updated when back to A).

Process:
A > B > C > setDefaultNightMode OK > go back to A (B & C destroyed) > B > C > setDefaultNightMode KO (wrong configuration for A)
& so on (OK/KO/OK/KO...)

I think the fix for the

issue 169770885

leads to this issue.
Before the fix of the 169770885, i had a different issue (since 1.2.0) (onConfigurationChanged not called on Activity C after setDefaultNightMode)

Technically here is what I have seen :
- The AppCompatDelegate has 3 AppCompatDelegateImpl (for A,B and C) in sActivityDelegates (both when the setDefaultNightMode works well or not)
- When calculating the currentNightMode in AppCompatDelegateImpl.updateForNightMode, the Activity A has the wrong value so the changed is not applied

Regards,

Comments

al...@google.com <al...@google.com> #2May 20, 2021 01:04PM

Hi Ed, Thank you so much for these suggestions. I've been reviewing them and merging them in. Hopefully it should be live. I've included a thank you note too in the article.

al...@google.com <al...@google.com> #3May 20, 2021 02:01PM

Reassigned to cf...@coyote-group.com.

Great! Thanks a lot, I'll look for the live updates soon!

cf...@coyote-group.com <cf...@coyote-group.com> #4May 20, 2021 02:33PM

The issue is in our production project but i will try make a minimal sample (hoping to repro), but with a sample project the UI thread is not heavily loaded (if its a problem of thread synchronization).

I have checked the test case implementation and something seems strange:

// Activities A, B, and C should all receive configuration changes.
listOf(activityA, activityB, activityC).forEach { activity ->
activity.expectOnConfigurationChange(3000)
}

Its ensure that Activity onConfigurationChanged is called ? Because with the method overridden and logs added in our application, only activity C received onConfigurationChanged (and onNightModeChanged), the other activies received only onNightModeChanged.

al...@google.com <al...@google.com> #5May 20, 2021 04:38PM

I'm seeing varied results, which tracks with a thread synchronization issue, either of:

Activity A's effective configuration has night mode set expected:<32> but was:<16> from the initial call to setDefaultNightMode
pass

Interestingly, though, I'm unable to get it to fail from the second call. It's possible the initial call failure is masking it, though, in the case where it would have failed.

al...@google.com <al...@google.com> #6May 20, 2021 04:40PM

Its ensure that Activity onConfigurationChanged is called ? Because with the method overridden and logs added in our application, only activity C received onConfigurationChanged (and onNightModeChanged), the other activies received only onNightModeChanged.

Yes, it's tracking calls to Activity.onConfigurationChanged().

al...@google.com <al...@google.com> #7May 20, 2021 05:10PM

The variance in my test is due to Activity A reaching the STOPPED state slightly faster or slower than the test can call setDefaultNightMode(). We don't call the method if the activity is stopped:

                if (lifecycle.getCurrentState().isAtLeast(Lifecycle.State.STARTED)) {
                    activity.onConfigurationChanged(conf);
                }

Which doesn't seem right. Activities don't get new configurations unless they're restarted or onConfigueationChanged is called, so as long as the activity is CREATED and not DESTROYED then we ought to be calling the method.

al...@google.com <al...@google.com> #8May 20, 2021 05:39PM

Accepted by al...@google.com.

Alright, test is failing 100% once I have a sync point for hidden activities reaching STOPPED, and I think that's representative of the same issue that you're seeing. I've identified a safe fix that we can do in 1.3.1.

cf...@coyote-group.com <cf...@coyote-group.com> #9May 20, 2021 05:53PM

I have created a sample project with the strange behavior, i will check again and i will share it tomorrow.

cf...@coyote-group.com <cf...@coyote-group.com> #10May 21, 2021 01:02PM

Here the behavior with the sample:

appcompat version 1.3.0:

MainActivity -> ActivityB -> ActivityC -> "change night mode"

=>logs:
05-21 14:41:04.307 D/NightMode(14750): ActivityC onConfigurationChanged
05-21 14:41:04.307 D/NightMode(14750): ActivityC onNightModeChanged mode 2, resource configuration in dark mode : true

------------> no configurationChanged nor onNightModeChanged in previous activities, even when back)

then back -> back -> ActivityB -> ActivityC -> "change night mode"
=>logs:
05-21 14:43:41.317 D/NightMode(14750): ActivityC onConfigurationChanged
05-21 14:43:41.317 D/NightMode(14750): ActivityC onNightModeChanged mode 1, resource configuration in dark mode : false
05-21 14:43:41.319 D/NightMode(14750): MainActivity onNightModeChanged mode 1, resource configuration in dark mode : false

------------> no onNightModeChanged for ActivityB

the third time is even odd:
logs:
05-21 14:46:30.951 D/NightMode(14750): ActivityC setDefaultNightMode, previous mode 1, new mode 2
05-21 14:46:30.955 D/NightMode(14750): ActivityB onNightModeChanged mode 2, resource configuration in dark mode : true

------------> ActivityC even not received onConfigurationChanged ! and only ActivityB received onNightModeChanged

And so on, inconsistent behavior for activities receiving onNightModeChanged / onConfigurationChanged, depending of the AppCompatDelegateImpl order in sActivityDelegates

appcompat version 1.4.0-alpha01:

MainActivity -> ActivityB -> ActivityC -> "change night mode"

=>logs:
05-21 14:54:27.480 D/NightMode(15350): MainActivity onNightModeChanged mode 2, resource configuration in dark mode : true
05-21 14:54:27.482 D/NightMode(15350): ActivityC onConfigurationChanged
05-21 14:54:27.482 D/NightMode(15350): ActivityC onNightModeChanged mode 2, resource configuration in dark mode : true
05-21 14:54:27.482 D/NightMode(15350): ActivityB onNightModeChanged mode 2, resource configuration in dark mode : true

------------> ok but weird order

then back -> back -> ActivityB -> ActivityC -> "change night mode"
=>logs:
05-21 14:54:37.811 D/NightMode(15350): ActivityC setDefaultNightMode, previous mode 2, new mode 1
05-21 14:54:37.817 D/NightMode(15350): ActivityC onConfigurationChanged
05-21 14:54:37.817 D/NightMode(15350): ActivityC onNightModeChanged mode 1, resource configuration in dark mode : false
05-21 14:54:37.819 D/NightMode(15350): ActivityB onNightModeChanged mode 1, resource configuration in dark mode : false

------------> no onNightModeChanged for MainActivity

I hope it will help

TestNightMode.zip

201 KB

Download

cf...@coyote-group.com <cf...@coyote-group.com> #11May 21, 2021 01:08PM

With the last case in version 1.4.0-alpha01, when the MainActivity onNightModeChanged hasn't been triggered, indeed the uiMode hasn't changed (ActivityB and C uiMode OK)

al...@google.com <al...@google.com> #12May 21, 2021 05:39PM

Thanks! Okay, interesting, that means we're not getting updateResourcesConfigurationForNightMode at all and we may not be getting applyDayNight.

We could see that if Activity A was destroyed, but I would expect it to come back up from onCreate with the appropriate night mode.

Will do a bit of investigation using your sample app...

al...@google.com <al...@google.com> #13May 21, 2021 06:01PM

MainActivity -> ActivityB -> ActivityC -> "change night mode"

2018-08-17 14:53:28.945 10425-10425/com.example.testnightmode D/NightMode: ActivityC setDefaultNightMode, previous mode -100, new mode 2
2018-08-17 14:53:28.949 10425-10425/com.example.testnightmode D/NightMode: ActivityC onConfigurationChanged
2018-08-17 14:53:28.949 10425-10425/com.example.testnightmode D/NightMode: ActivityC onNightModeChanged mode 2, resource configuration in dark mode : true
2018-08-17 14:53:28.949 10425-10425/com.example.testnightmode D/NightMode: ActivityC color: -4487428
2018-08-17 14:53:28.950 10425-10425/com.example.testnightmode D/NightMode: MainActivity onNightModeChanged mode 2, resource configuration in dark mode : true
2018-08-17 14:53:28.950 10425-10425/com.example.testnightmode D/NightMode: MainActivity color: -4487428
2018-08-17 14:53:28.951 10425-10425/com.example.testnightmode D/NightMode: ActivityB onNightModeChanged mode 2, resource configuration in dark mode : true
2018-08-17 14:53:28.951 10425-10425/com.example.testnightmode D/NightMode: ActivityB color: -4487428

back -> back

2018-08-17 14:54:27.806 10425-10425/com.example.testnightmode D/NightMode: ActivityB onNightModeChanged mode 2, resource configuration in dark mode : true
2018-08-17 14:54:27.806 10425-10425/com.example.testnightmode D/NightMode: ActivityB color: -4487428
2018-08-17 14:54:27.807 10425-10425/com.example.testnightmode D/NightMode: ActivityB onResume - in dark mode : true
2018-08-17 14:54:28.304 10425-10425/com.example.testnightmode D/NightMode: ActivityC onStop - in dark mode : true
2018-08-17 14:54:28.305 10425-10425/com.example.testnightmode D/NightMode: ActivityC onDestroy - in dark mode : true
2018-08-17 14:54:28.387 10425-10425/com.example.testnightmode D/NightMode: MainActivity onNightModeChanged mode 2, resource configuration in dark mode : true
2018-08-17 14:54:28.387 10425-10425/com.example.testnightmode D/NightMode: MainActivity color: -4487428
2018-08-17 14:54:28.388 10425-10425/com.example.testnightmode D/NightMode: MainActivity onResume - in dark mode : true
2018-08-17 14:54:28.869 10425-10425/com.example.testnightmode D/NightMode: ActivityB onStop - in dark mode : true
2018-08-17 14:54:28.871 10425-10425/com.example.testnightmode D/NightMode: ActivityB onDestroy - in dark mode : true

-> B -> C

2018-08-17 14:55:05.050 10425-10425/com.example.testnightmode D/NightMode: ActivityB onCreate - in dark mode : true
2018-08-17 14:55:05.094 10425-10425/com.example.testnightmode D/NightMode: ActivityB onResume - in dark mode : true
2018-08-17 14:55:05.156 1196-1252/system_process I/ActivityManager: Displayed com.example.testnightmode/.ActivityB: +123ms
2018-08-17 14:55:05.583 10425-10425/com.example.testnightmode D/NightMode: MainActivity onStop - in dark mode : true
2018-08-17 14:55:06.018 1196-1741/system_process I/ActivityManager: START u0 {cmp=com.example.testnightmode/.ActivityC} from uid 10145
2018-08-17 14:55:06.063 10425-10425/com.example.testnightmode D/NightMode: ActivityC onCreate - in dark mode : true
2018-08-17 14:55:06.096 10425-10425/com.example.testnightmode D/NightMode: ActivityC onResume - in dark mode : true
2018-08-17 14:55:06.150 1196-1252/system_process I/ActivityManager: Displayed com.example.testnightmode/.ActivityC: +105ms
2018-08-17 14:55:06.608 10425-10425/com.example.testnightmode D/NightMode: ActivityB onStop - in dark mode : true

change night mode

2018-08-17 14:56:27.601 10425-10425/com.example.testnightmode D/NightMode: ActivityC calling scheduleDirect
2018-08-17 14:56:27.603 10425-10425/com.example.testnightmode D/NightMode: ActivityC setDefaultNightMode, previous mode 2, new mode 1
2018-08-17 14:56:27.606 10425-10425/com.example.testnightmode D/NightMode: ActivityC onConfigurationChanged
2018-08-17 14:56:27.606 10425-10425/com.example.testnightmode D/NightMode: ActivityC onNightModeChanged mode 1, resource configuration in dark mode : false
2018-08-17 14:56:27.606 10425-10425/com.example.testnightmode D/NightMode: ActivityC color: -10354450
2018-08-17 14:56:27.607 10425-10425/com.example.testnightmode D/NightMode: ActivityB onNightModeChanged mode 1, resource configuration in dark mode : false
2018-08-17 14:56:27.607 10425-10425/com.example.testnightmode D/NightMode: ActivityB color: -10354450

al...@google.com <al...@google.com> #14May 21, 2021 06:09PM

Yeah, this all comes down to STOPPED activities not receiving onConfigurationChanged. That's where mEffectiveConfiguration gets updated, which is how activities track whether they need to respond to changes. The proposed fix addresses this.

al...@google.com <al...@google.com> #15May 21, 2021 06:37PM

We'll do a 1.3.1 once the CL lands and gets through integration testing.

al...@google.com <al...@google.com> #16May 21, 2021 06:37PM

And it ought to be in before we cut 1.4.0-alpha02, as well.

al...@google.com <al...@google.com> #17May 26, 2021 05:58PM

One little snag -- we have existing tests ensuring that activities that are not started do not receive configuration changes; however, it's not clear from the test whether it was intended to cover activities that are created, started, and then stopped.

    @Test
    fun testOnConfigurationChangeNotCalledWhenNotStarted() {
        scenario.moveToState(Lifecycle.State.CREATED)
        // And clear any previous config changes
        scenario.onActivity { it.lastConfigurationChangeAndClear }

        // Set local night mode to YES
        scenario.onActivity { setNightMode(MODE_NIGHT_YES, it, setMode) }
        // Assert that the onConfigurationChange was not called with a new correct config
        scenario.onActivity {
            assertNull(it.lastConfigurationChangeAndClear)
        }

I think what we actually want here is State.INITIALIZED and/or State.DESTROYED, but I'm going to see if I can figure out how the platform handles onConfigurationChanged for stopped activities.

al...@google.com <al...@google.com> #18May 26, 2021 06:07PM

The platform will, in fact, send changes to stopped activities:

        // Set local night mode to MODE_NIGHT_YES and wait for state RESUMED.
        val scenario = ActivityScenario.launch(NightModeRotateDoesNotRecreateActivity::class.java)
        scenario.moveToState(Lifecycle.State.RESUMED)

        // Stop the activity.
        scenario.moveToState(Lifecycle.State.CREATED)

        // Now rotate the device. This should NOT result in a lifecycle event, just a call to
        // onConfigurationChanged.
        lateinit var activity: NightModeRotateDoesNotRecreateActivity
        scenario.onActivity { activity = it }

        activity.resetOnConfigurationChange()
        device.withOrientation(Orientation.LEFT) {
            activity.expectOnConfigurationChange(5000)
        }

ap...@google.com <ap...@google.com> #19May 26, 2021 07:21PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit 13a7752790ee585c733eb01a56ac94d3f196cf7a
Author: Alan Viverette <alanv@google.com>
Date: Thu May 20 09:59:34 2021

Ensure that all created activities receive onConfigurationChange

Also adds better test sync points around activity lifecycle.

Relnote: """Fix issue where stopped actvities did not receive configuration
changes from AppCompat-instrumented night mode changes."""
Fixes: 188681415
Test: NightModeStackedHandlingTestCase
Change-Id: I8fa8fbc2849499de9cd8af2b77726f7081a37139

M appcompat/appcompat/src/androidTest/AndroidManifest.xml
M appcompat/appcompat/src/androidTest/java/androidx/appcompat/app/NightModeStackedHandlingTestCase.kt
A appcompat/appcompat/src/androidTest/java/androidx/appcompat/app/NightModeUiModeConfigChangesActivityB.java
A appcompat/appcompat/src/androidTest/java/androidx/appcompat/app/NightModeUiModeConfigChangesActivityC.java
M appcompat/appcompat/src/androidTest/java/androidx/appcompat/app/NightModeUiModeConfigChangesTestCase.kt
M appcompat/appcompat/src/main/java/androidx/appcompat/app/AppCompatDelegateImpl.java

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