From 9e50419c75a515ec484a9b526e97ce4dcf4995b1 Mon Sep 17 00:00:00 2001 From: Matthew Fritze Date: Thu, 21 Feb 2019 15:17:37 -0800 Subject: [PATCH] Remove NFC Slice jank The NFC Slice would jank on enable and disable, because of the intent filter it registered with SysUI. The intent filter would broadcast an update for four states: 1) On 2) Off 3) Turning On 4) Turning off The first two caused no problems. The third and fourth caused jank, since when clicked, the switch in the NFC slice would turn on / off asynchronously - that is, it turned on or off based on the previous state of the switch, rather than on the actual value of NFC. It does this to feel fluid in the app in which it is rendered. From the off state, the order of events is: 1. Switch clicked 2. Switch animates on 2. Background intent is fired to settings to turn on Nfc (happens at the same time as animation) 3. Settings calls the NFC enable API 4. A broadcast for Turning On is sent 5. The receiver in SysUI gets the broadcast and forwards it to settings 6. Settings tells the Slice to make sure it is up to date 7. The Slice checks for the current value - IMPORTANTLY - which is currently off, it is only in the process of being enabled. 8. The Slice flips back off 9. Nfc finishes getting enabled in the background 10. The framework pushes the NFC ON broadcast 11. SysUI gets the broadcast, and forwards it to settings 12. Settings tells the slice to update 13. The slice checks again and finds that NFC is on, flipping on. This CL creates a new background slice worker for NFC and registers the intent filter there, rather than in SysUI. When the background worker gets the broadcast, it checks if it is in state 3/4, and if so, it drops the update silently. Fixes: 115737701 Test: robotests Change-Id: I17043828ad3a67a2a5acdf5c75d9cc51ff7e91d0 --- res/layout/settings_panel.xml | 3 +- .../settings/nfc/NfcPreferenceController.java | 97 ++++++++++++++++--- .../settings/slices/CustomSliceable.java | 12 --- .../slices/SettingsSliceProvider.java | 9 +- .../slices/SliceBackgroundWorker.java | 5 +- .../android/settings/slices/Sliceable.java | 12 +++ .../nfc/NfcPreferenceControllerTest.java | 70 +++++++++++++ .../slices/SettingsSliceProviderTest.java | 15 +++ .../testutils/FakeToggleController.java | 28 ++++++ 9 files changed, 220 insertions(+), 31 deletions(-) diff --git a/res/layout/settings_panel.xml b/res/layout/settings_panel.xml index aec898c99c6..3405ef0d844 100644 --- a/res/layout/settings_panel.xml +++ b/res/layout/settings_panel.xml @@ -16,4 +16,5 @@ \ No newline at end of file + android:layout_width="match_parent" + android:animateLayoutChanges="true"/> \ No newline at end of file diff --git a/src/com/android/settings/nfc/NfcPreferenceController.java b/src/com/android/settings/nfc/NfcPreferenceController.java index 04f288d721b..2ca3b23daea 100644 --- a/src/com/android/settings/nfc/NfcPreferenceController.java +++ b/src/com/android/settings/nfc/NfcPreferenceController.java @@ -15,20 +15,26 @@ */ package com.android.settings.nfc; +import android.content.BroadcastReceiver; import android.content.Context; +import android.content.Intent; import android.content.IntentFilter; +import android.net.Uri; import android.nfc.NfcAdapter; import android.provider.Settings; - +import android.util.Log; import androidx.annotation.VisibleForTesting; import androidx.preference.PreferenceScreen; import androidx.preference.SwitchPreference; import com.android.settings.core.TogglePreferenceController; +import com.android.settings.slices.SliceBackgroundWorker; import com.android.settingslib.core.lifecycle.LifecycleObserver; import com.android.settingslib.core.lifecycle.events.OnPause; import com.android.settingslib.core.lifecycle.events.OnResume; +import java.io.IOException; + public class NfcPreferenceController extends TogglePreferenceController implements LifecycleObserver, OnResume, OnPause { @@ -51,8 +57,7 @@ public class NfcPreferenceController extends TogglePreferenceController return; } - final SwitchPreference switchPreference = - (SwitchPreference) screen.findPreference(getPreferenceKey()); + final SwitchPreference switchPreference = screen.findPreference(getPreferenceKey()); mNfcEnabler = new NfcEnabler(mContext, switchPreference); @@ -86,14 +91,6 @@ public class NfcPreferenceController extends TogglePreferenceController : UNSUPPORTED_ON_DEVICE; } - @Override - public IntentFilter getIntentFilter() { - final IntentFilter filter = new IntentFilter(); - filter.addAction(NfcAdapter.ACTION_ADAPTER_STATE_CHANGED); - filter.addAction(NfcAdapter.EXTRA_ADAPTER_STATE); - return filter; - } - @Override public boolean hasAsyncUpdate() { return true; @@ -104,6 +101,11 @@ public class NfcPreferenceController extends TogglePreferenceController return true; } + @Override + public Class getBackgroundWorkerClass() { + return NfcSliceWorker.class; + } + @Override public void onResume() { if (mAirplaneModeObserver != null) { @@ -135,4 +137,77 @@ public class NfcPreferenceController extends TogglePreferenceController Settings.Global.AIRPLANE_MODE_TOGGLEABLE_RADIOS); return toggleable != null && toggleable.contains(Settings.Global.RADIO_NFC); } + + /** + * Listener for background changes to NFC. + * + *

+ * Listen to broadcasts from {@link NfcAdapter}. The worker will call notify changed on the + * NFC Slice only when the following extras are present in the broadcast: + *

+ */ + public static class NfcSliceWorker extends SliceBackgroundWorker { + + private static final String TAG = "NfcSliceWorker"; + + private static final IntentFilter NFC_FILTER = + new IntentFilter(NfcAdapter.ACTION_ADAPTER_STATE_CHANGED); + + private NfcUpdateReceiver mUpdateObserver; + + public NfcSliceWorker(Context context, Uri uri) { + super(context, uri); + mUpdateObserver = new NfcUpdateReceiver(this); + } + + @Override + protected void onSlicePinned() { + getContext().registerReceiver(mUpdateObserver, NFC_FILTER); + } + + @Override + protected void onSliceUnpinned() { + getContext().unregisterReceiver(mUpdateObserver); + } + + @Override + public void close() throws IOException { + mUpdateObserver = null; + } + + public void updateSlice() { + notifySliceChange(); + } + + public class NfcUpdateReceiver extends BroadcastReceiver { + + private final int NO_EXTRA = -1; + + private final NfcSliceWorker mSliceBackgroundWorker; + + public NfcUpdateReceiver(NfcSliceWorker sliceWorker) { + mSliceBackgroundWorker = sliceWorker; + } + + @Override + public void onReceive(Context context, Intent intent) { + final int nfcStateExtra = intent.getIntExtra(NfcAdapter.EXTRA_ADAPTER_STATE, + NO_EXTRA); + + // Do nothing if state change is empty, or an intermediate step. + if ( (nfcStateExtra == NO_EXTRA) + || (nfcStateExtra == NfcAdapter.STATE_TURNING_ON) + || (nfcStateExtra == NfcAdapter.STATE_TURNING_OFF)) { + Log.d(TAG, "Transitional update, dropping broadcast"); + return; + } + + Log.d(TAG, "Nfc broadcast received, updating Slice."); + mSliceBackgroundWorker.updateSlice(); + } + } + } } diff --git a/src/com/android/settings/slices/CustomSliceable.java b/src/com/android/settings/slices/CustomSliceable.java index b48f22a5237..0b97bedc54f 100644 --- a/src/com/android/settings/slices/CustomSliceable.java +++ b/src/com/android/settings/slices/CustomSliceable.java @@ -83,18 +83,6 @@ public interface CustomSliceable extends Sliceable { */ Intent getIntent(); - /** - * Settings Slices which require background work, such as updating lists should implement a - * {@link SliceBackgroundWorker} and return it here. An example of background work is updating - * a list of Wifi networks available in the area. - * - * @return a {@link Class} to perform background work for the - * slice. - */ - default Class getBackgroundWorkerClass() { - return null; - } - /** * Standardize the intents returned to indicate actions by the Slice. *

diff --git a/src/com/android/settings/slices/SettingsSliceProvider.java b/src/com/android/settings/slices/SettingsSliceProvider.java index 5c662e5c7e0..397b2fc631c 100644 --- a/src/com/android/settings/slices/SettingsSliceProvider.java +++ b/src/com/android/settings/slices/SettingsSliceProvider.java @@ -153,7 +153,7 @@ public class SettingsSliceProvider extends SliceProvider { if (filter != null) { registerIntentToUri(filter, sliceUri); } - ThreadUtils.postOnMainThread(() -> startBackgroundWorker(sliceable)); + ThreadUtils.postOnMainThread(() -> startBackgroundWorker(sliceable, sliceUri)); return; } @@ -326,20 +326,19 @@ public class SettingsSliceProvider extends SliceProvider { } } - private void startBackgroundWorker(CustomSliceable sliceable) { + private void startBackgroundWorker(Sliceable sliceable, Uri uri) { final Class workerClass = sliceable.getBackgroundWorkerClass(); if (workerClass == null) { return; } - final Uri uri = sliceable.getUri(); if (mPinnedWorkers.containsKey(uri)) { return; } Log.d(TAG, "Starting background worker for: " + uri); final SliceBackgroundWorker worker = SliceBackgroundWorker.getInstance( - getContext(), sliceable); + getContext(), sliceable, uri); mPinnedWorkers.put(uri, worker); worker.onSlicePinned(); } @@ -397,6 +396,8 @@ public class SettingsSliceProvider extends SliceProvider { registerIntentToUri(filter, uri); } + ThreadUtils.postOnMainThread(() -> startBackgroundWorker(controller, uri)); + final List pinnedSlices = getContext().getSystemService( SliceManager.class).getPinnedSlices(); if (pinnedSlices.contains(uri)) { diff --git a/src/com/android/settings/slices/SliceBackgroundWorker.java b/src/com/android/settings/slices/SliceBackgroundWorker.java index 995394e7d41..559aa711e2a 100644 --- a/src/com/android/settings/slices/SliceBackgroundWorker.java +++ b/src/com/android/settings/slices/SliceBackgroundWorker.java @@ -80,13 +80,12 @@ public abstract class SliceBackgroundWorker implements Closeable { * Returns the singleton instance of the {@link SliceBackgroundWorker} for specified {@link * CustomSliceable} */ - static SliceBackgroundWorker getInstance(Context context, CustomSliceable sliceable) { - final Uri uri = sliceable.getUri(); + static SliceBackgroundWorker getInstance(Context context, Sliceable sliceable, Uri uri) { SliceBackgroundWorker worker = getInstance(uri); if (worker == null) { final Class workerClass = sliceable.getBackgroundWorkerClass(); - worker = createInstance(context, uri, workerClass); + worker = createInstance(context.getApplicationContext(), uri, workerClass); LIVE_WORKERS.put(uri, worker); } return worker; diff --git a/src/com/android/settings/slices/Sliceable.java b/src/com/android/settings/slices/Sliceable.java index b00ab8207eb..c1661f8392a 100644 --- a/src/com/android/settings/slices/Sliceable.java +++ b/src/com/android/settings/slices/Sliceable.java @@ -91,4 +91,16 @@ public interface Sliceable { final String toast = context.getString(R.string.copyable_slice_toast, messageTitle); Toast.makeText(context, toast, Toast.LENGTH_SHORT).show(); } + + /** + * Settings Slices which require background work, such as updating lists should implement a + * {@link SliceBackgroundWorker} and return it here. An example of background work is updating + * a list of Wifi networks available in the area. + * + * @return a {@link Class} to perform background work for the + * slice. + */ + default Class getBackgroundWorkerClass() { + return null; + } } diff --git a/tests/robotests/src/com/android/settings/nfc/NfcPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/nfc/NfcPreferenceControllerTest.java index 8883ddf7e3a..e3672c92dc9 100644 --- a/tests/robotests/src/com/android/settings/nfc/NfcPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/nfc/NfcPreferenceControllerTest.java @@ -19,10 +19,13 @@ package com.android.settings.nfc; import static com.google.common.truth.Truth.assertThat; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.content.Context; +import android.content.Intent; +import android.net.Uri; import android.nfc.NfcAdapter; import android.nfc.NfcManager; import android.os.UserManager; @@ -31,6 +34,10 @@ import android.provider.Settings; import androidx.preference.PreferenceScreen; import androidx.preference.SwitchPreference; +import com.android.settings.nfc.NfcPreferenceController.NfcSliceWorker; +import com.android.settings.nfc.NfcPreferenceController.NfcSliceWorker.NfcUpdateReceiver; +import com.android.settings.slices.SliceBuilderUtils; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -246,4 +253,67 @@ public class NfcPreferenceControllerTest { assertThat(mNfcController.mAirplaneModeObserver).isNull(); } + + @Test + public void ncfSliceWorker_nfcBroadcast_noExtra_sliceDoesntUpdate() { + final NfcSliceWorker worker = spy(new NfcSliceWorker(mContext, getDummyUri())); + final NfcUpdateReceiver receiver = worker.new NfcUpdateReceiver(worker); + final Intent triggerIntent = new Intent(NfcAdapter.ACTION_ADAPTER_STATE_CHANGED); + + receiver.onReceive(mContext, triggerIntent); + + verify(worker, times(0)).updateSlice(); + } + + @Test + public void ncfSliceWorker_nfcBroadcast_turningOn_sliceDoesntUpdate() { + final NfcSliceWorker worker = spy(new NfcSliceWorker(mContext, getDummyUri())); + final NfcUpdateReceiver receiver = worker.new NfcUpdateReceiver(worker); + final Intent triggerIntent = new Intent(NfcAdapter.ACTION_ADAPTER_STATE_CHANGED); + triggerIntent.putExtra(NfcAdapter.EXTRA_ADAPTER_STATE, NfcAdapter.STATE_TURNING_ON); + + receiver.onReceive(mContext, triggerIntent); + + verify(worker, times(0)).updateSlice(); + } + + @Test + public void ncfSliceWorker_nfcBroadcast_turningOff_sliceDoesntUpdate() { + final NfcSliceWorker worker = spy(new NfcSliceWorker(mContext, getDummyUri())); + final NfcUpdateReceiver receiver = worker.new NfcUpdateReceiver(worker); + final Intent triggerIntent = new Intent(NfcAdapter.ACTION_ADAPTER_STATE_CHANGED); + triggerIntent.putExtra(NfcAdapter.EXTRA_ADAPTER_STATE, NfcAdapter.STATE_TURNING_OFF); + + receiver.onReceive(mContext, triggerIntent); + + verify(worker, times(0)).updateSlice(); + } + + @Test + public void ncfSliceWorker_nfcBroadcast_nfcOn_sliceUpdates() { + final NfcSliceWorker worker = spy(new NfcSliceWorker(mContext, getDummyUri())); + final NfcUpdateReceiver receiver = worker.new NfcUpdateReceiver(worker); + final Intent triggerIntent = new Intent(NfcAdapter.ACTION_ADAPTER_STATE_CHANGED); + triggerIntent.putExtra(NfcAdapter.EXTRA_ADAPTER_STATE, NfcAdapter.STATE_ON); + + receiver.onReceive(mContext, triggerIntent); + + verify(worker).updateSlice(); + } + + @Test + public void ncfSliceWorker_nfcBroadcast_nfcOff_sliceUpdates() { + final NfcSliceWorker worker = spy(new NfcSliceWorker(mContext, getDummyUri())); + final NfcUpdateReceiver receiver = worker.new NfcUpdateReceiver(worker); + final Intent triggerIntent = new Intent(NfcAdapter.ACTION_ADAPTER_STATE_CHANGED); + triggerIntent.putExtra(NfcAdapter.EXTRA_ADAPTER_STATE, NfcAdapter.STATE_OFF); + + receiver.onReceive(mContext, triggerIntent); + + verify(worker).updateSlice(); + } + + private Uri getDummyUri() { + return SliceBuilderUtils.getUri("action/nfc", false); + } } diff --git a/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java b/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java index 726e4bdf300..a693f349894 100644 --- a/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java +++ b/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java @@ -64,6 +64,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.robolectric.Robolectric; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; @@ -183,6 +184,20 @@ public class SettingsSliceProviderTest { verify(mProvider).registerIntentToUri(eq(FakeToggleController.INTENT_FILTER), eq(uri)); } + @Test + public void loadSlice_registersBackgroundListener() { + insertSpecialCase(KEY); + final Uri uri = SliceBuilderUtils.getUri(INTENT_PATH, false); + + mProvider.loadSlice(uri); + + Robolectric.flushForegroundThreadScheduler(); + Robolectric.flushBackgroundThreadScheduler(); + + assertThat(mProvider.mPinnedWorkers.get(uri).getClass()) + .isEqualTo(FakeToggleController.TestWorker.class); + } + @Test public void testLoadSlice_doesNotCacheWithoutPin() { insertSpecialCase(KEY); diff --git a/tests/robotests/src/com/android/settings/testutils/FakeToggleController.java b/tests/robotests/src/com/android/settings/testutils/FakeToggleController.java index d1677cd7382..e7854874c54 100644 --- a/tests/robotests/src/com/android/settings/testutils/FakeToggleController.java +++ b/tests/robotests/src/com/android/settings/testutils/FakeToggleController.java @@ -19,10 +19,14 @@ package com.android.settings.testutils; import android.content.Context; import android.content.IntentFilter; +import android.net.Uri; import android.net.wifi.WifiManager; import android.provider.Settings; import com.android.settings.core.TogglePreferenceController; +import com.android.settings.slices.SliceBackgroundWorker; + +import java.io.IOException; public class FakeToggleController extends TogglePreferenceController { @@ -70,6 +74,11 @@ public class FakeToggleController extends TogglePreferenceController { return true; } + @Override + public Class getBackgroundWorkerClass() { + return TestWorker.class; + } + @Override public boolean hasAsyncUpdate() { return mIsAsyncUpdate; @@ -78,4 +87,23 @@ public class FakeToggleController extends TogglePreferenceController { public void setAsyncUpdate(boolean isAsyncUpdate) { mIsAsyncUpdate = isAsyncUpdate; } + + public static class TestWorker extends SliceBackgroundWorker { + + public TestWorker(Context context, Uri uri) { + super(context, uri); + } + + @Override + protected void onSlicePinned() { + } + + @Override + protected void onSliceUnpinned() { + } + + @Override + public void close() throws IOException { + } + } }