Fix Bluetooth switch status in Connected devices screen
The symptom observed is that the Bluetooth master switch on the
Connected devices page doesn't properly respond to Bluetooth turning off
via quicksettings - either turning on airplane mode or just toggling
Bluetooth.
The root cause was that MasterSwitchPreference's isChecked method would
not return the true value of whether the switch was checked - if the
control is disabled, it always just returns false. This interacts badly
with code in BluetoothEnabler - we disable the switch when the Bluetooth
state is in transition (eg becomes STATE_TURNING_OFF), and we also
attempt to avoid calling setChecked if the switch is already in the
desired state. So the switch would be checked but disabled, and we'd
avoid ever calling setChecked(false) on it.
A thorough fix would be to remove the code from MasterSwitchPreference's
isChecked method that looks at the enabled state, since enabled and
checked really should be treated as separate concerns. But given the
timeframe of MR1, we're opting for a more conservative fix of directly
accessing the switch and checking it's state, to avoid introducing bugs
in other consumers that might be depending on the current
behavior. We'll then do the thorough fix on the master branch which will
give a lot more time for any unexpected issues to be found (I audited
other usages and none seemed likely to be a problem, but it's better to
be safe than sorry).
Change-Id: I19a6c6b71e74595be3ef32a9718a430b67a89d53
Bug: 64940731
Test: make RunSettingsRoboTests
diff --git a/src/com/android/settings/bluetooth/BluetoothEnabler.java b/src/com/android/settings/bluetooth/BluetoothEnabler.java
index 1bda130..f95145d 100644
--- a/src/com/android/settings/bluetooth/BluetoothEnabler.java
+++ b/src/com/android/settings/bluetooth/BluetoothEnabler.java
@@ -157,7 +157,9 @@
}
private void setChecked(boolean isChecked) {
- if (isChecked != mSwitchWidget.isChecked()) {
+ final boolean currentState =
+ (mSwitchWidget.getSwitch() != null) && mSwitchWidget.getSwitch().isChecked();
+ if (isChecked != currentState) {
// set listener to null, so onCheckedChanged won't be called
// if the checked status on Switch isn't changed by user click
if (mValidListener) {
diff --git a/tests/robotests/src/com/android/settings/bluetooth/BluetoothEnablerTest.java b/tests/robotests/src/com/android/settings/bluetooth/BluetoothEnablerTest.java
index 74c47e2..8fa8f06 100644
--- a/tests/robotests/src/com/android/settings/bluetooth/BluetoothEnablerTest.java
+++ b/tests/robotests/src/com/android/settings/bluetooth/BluetoothEnablerTest.java
@@ -15,14 +15,24 @@
*/
package com.android.settings.bluetooth;
+import android.bluetooth.BluetoothAdapter;
+import android.content.BroadcastReceiver;
import android.content.ComponentName;
import android.content.Context;
+import android.content.Intent;
+import android.content.IntentFilter;
import android.os.UserManager;
+import android.support.v7.preference.PreferenceViewHolder;
+import android.view.View;
+import android.widget.Switch;
+import com.android.settings.R;
import com.android.settings.testutils.SettingsRobolectricTestRunner;
import com.android.settings.TestConfig;
import com.android.settings.core.instrumentation.MetricsFeatureProvider;
+import com.android.settings.testutils.shadow.SettingsShadowResources;
import com.android.settings.widget.MasterSwitchController;
+import com.android.settings.widget.MasterSwitchPreference;
import com.android.settingslib.RestrictedLockUtils.EnforcedAdmin;
import com.android.settingslib.bluetooth.LocalBluetoothAdapter;
import com.android.settingslib.bluetooth.LocalBluetoothManager;
@@ -30,22 +40,26 @@
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
+import org.robolectric.RuntimeEnvironment;
import org.robolectric.annotation.Config;
import static com.google.common.truth.Truth.assertThat;
-import static junit.framework.TestCase.assertNotNull;
-import static junit.framework.TestCase.assertEquals;
+
import static org.mockito.Mockito.mock;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@RunWith(SettingsRobolectricTestRunner.class)
-@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION)
+@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION, shadows = {
+ SettingsShadowResources.class, SettingsShadowResources.SettingsShadowTheme.class
+})
public class BluetoothEnablerTest {
private static final EnforcedAdmin FAKE_ENFORCED_ADMIN =
@@ -54,22 +68,27 @@
@Mock
private MetricsFeatureProvider mMetricsFeatureProvider;
@Mock
- private Context mContext;
- @Mock
- private MasterSwitchController mMasterSwitchController;
- @Mock
private RestrictionUtils mRestrictionUtils;
@Mock
private LocalBluetoothManager mBluetoothManager;
@Mock
private LocalBluetoothAdapter mBluetoothAdapter;
+ private Context mContext;
+ Switch mSwitch;
+ private MasterSwitchPreference mMasterSwitchPreference;
+ private MasterSwitchController mMasterSwitchController;
private BluetoothEnabler mBluetoothEnabler;
@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
+ mContext = spy(RuntimeEnvironment.application);
when(mBluetoothManager.getBluetoothAdapter()).thenReturn(mBluetoothAdapter);
+
+ mSwitch = new Switch(mContext);
+ mMasterSwitchPreference = new MasterSwitchPreference(mContext);
+ mMasterSwitchController = spy(new MasterSwitchController(mMasterSwitchPreference));
mBluetoothEnabler = new BluetoothEnabler(
mContext,
mMasterSwitchController,
@@ -77,6 +96,9 @@
mBluetoothManager,
123,
mRestrictionUtils);
+ PreferenceViewHolder holder = PreferenceViewHolder.createInstanceForTests(mock(View.class));
+ when(holder.findViewById(R.id.switchWidget)).thenReturn(mSwitch);
+ mMasterSwitchPreference.onBindViewHolder(holder);
}
@Test
@@ -155,4 +177,71 @@
verify(mMasterSwitchController, never()).setEnabled(true);
}
+ @Test
+ public void startWithBluetoothOff_switchIsOff() {
+ when(mBluetoothAdapter.getBluetoothState()).thenReturn(BluetoothAdapter.STATE_OFF);
+ verify(mMasterSwitchController, never()).setChecked(anyBoolean());
+ mBluetoothEnabler.resume(mContext);
+ verify(mMasterSwitchController, never()).setChecked(true);
+ }
+
+ @Test
+ public void startWithBluetoothOn_switchIsOn() {
+ when(mBluetoothAdapter.getBluetoothState()).thenReturn(BluetoothAdapter.STATE_ON);
+ verify(mMasterSwitchController, never()).setChecked(anyBoolean());
+ mBluetoothEnabler.resume(mContext);
+ verify(mMasterSwitchController, never()).setChecked(false);
+ verify(mMasterSwitchController).setChecked(true);
+ }
+
+ @Test
+ public void bluetoothTurnsOff_switchTurnsOff() {
+ // Start up with bluetooth turned on. The switch should get turned on.
+ assertThat(mSwitch.isChecked()).isFalse();
+ ArgumentCaptor<BroadcastReceiver> captor = ArgumentCaptor.forClass(BroadcastReceiver.class);
+ when(mContext.registerReceiver(captor.capture(), any(IntentFilter.class))).thenReturn(null);
+ when(mBluetoothAdapter.getBluetoothState()).thenReturn(BluetoothAdapter.STATE_ON);
+ verify(mMasterSwitchController, never()).setChecked(anyBoolean());
+ mBluetoothEnabler.resume(mContext);
+ verify(mMasterSwitchController, never()).setChecked(false);
+ verify(mMasterSwitchController).setChecked(true);
+
+ // Now simulate bluetooth being turned off via an event.
+ BroadcastReceiver receiver = captor.getValue();
+ Intent turningOff = new Intent(BluetoothAdapter.ACTION_STATE_CHANGED);
+ turningOff.putExtra(BluetoothAdapter.EXTRA_STATE, BluetoothAdapter.STATE_TURNING_OFF);
+ receiver.onReceive(mContext, turningOff);
+ Intent off = new Intent(BluetoothAdapter.ACTION_STATE_CHANGED);
+ off.putExtra(BluetoothAdapter.EXTRA_STATE, BluetoothAdapter.STATE_OFF);
+ receiver.onReceive(mContext, off);
+
+ // Make sure the switch was turned off.
+ verify(mMasterSwitchController).setChecked(false);
+ assertThat(mSwitch.isChecked()).isFalse();
+ }
+
+ @Test
+ public void bluetoothTurnsOn_switchTurnsOn() {
+ // Start up with bluetooth turned on. The switch should be left off.
+ assertThat(mSwitch.isChecked()).isFalse();
+ ArgumentCaptor<BroadcastReceiver> captor = ArgumentCaptor.forClass(BroadcastReceiver.class);
+ when(mContext.registerReceiver(captor.capture(), any(IntentFilter.class))).thenReturn(null);
+ when(mBluetoothAdapter.getBluetoothState()).thenReturn(BluetoothAdapter.STATE_OFF);
+ verify(mMasterSwitchController, never()).setChecked(anyBoolean());
+ mBluetoothEnabler.resume(mContext);
+ verify(mMasterSwitchController, never()).setChecked(anyBoolean());
+
+ // Now simulate bluetooth being turned on via an event.
+ BroadcastReceiver receiver = captor.getValue();
+ Intent turningOn = new Intent(BluetoothAdapter.ACTION_STATE_CHANGED);
+ turningOn.putExtra(BluetoothAdapter.EXTRA_STATE, BluetoothAdapter.STATE_TURNING_ON);
+ receiver.onReceive(mContext, turningOn);
+ Intent on = new Intent(BluetoothAdapter.ACTION_STATE_CHANGED);
+ on.putExtra(BluetoothAdapter.EXTRA_STATE, BluetoothAdapter.STATE_ON);
+ receiver.onReceive(mContext, on);
+
+ // Make sure the switch was turned on.
+ verify(mMasterSwitchController).setChecked(true);
+ assertThat(mSwitch.isChecked()).isTrue();
+ }
}