From 3c89d8e2cfe5bc338f3e285945b6836cd2f69b05 Mon Sep 17 00:00:00 2001 From: Gustav Sennton Date: Thu, 23 Feb 2017 18:32:20 +0000 Subject: [PATCH] Fix disabled WebView packages being shown as enabled in Dev Setting. WebView packages that cannot be chosen as WebView implementation should be shown as disabled in the 'WebView Implementation' Dev Setting. Simplify the logic in DefaultAppInfo a little bit to avoid special-casing for the WebView Dev setting. Also add tests to ensure we are correctly disabling WebView packages in the Dev setting and showing their disabled-reasons. Bug: 35707106 Test: 'make RunSettingsRoboTests' Test: Ensure disabled WebView packages show up as being disabled in the 'WebView Implementation' Dev Setting. Change-Id: Ic2f5265c3451b6396e55b7bc29689c3d889ddb5e --- .../defaultapps/DefaultAppInfo.java | 12 ++-- .../defaultapps/DefaultAppPickerFragment.java | 41 ++++++------ .../settings/webview/WebViewAppPicker.java | 11 +++- .../webview/WebViewAppPickerTest.java | 65 ++++++++++++++++++- 4 files changed, 97 insertions(+), 32 deletions(-) diff --git a/src/com/android/settings/applications/defaultapps/DefaultAppInfo.java b/src/com/android/settings/applications/defaultapps/DefaultAppInfo.java index 96512f5d54e..d276fd241c2 100644 --- a/src/com/android/settings/applications/defaultapps/DefaultAppInfo.java +++ b/src/com/android/settings/applications/defaultapps/DefaultAppInfo.java @@ -35,8 +35,6 @@ public class DefaultAppInfo { public final ComponentName componentName; public final PackageItemInfo packageItemInfo; public final String summary; - // Description for why this item is disabled, if null, the item is enabled. - public final String disabledDescription; public final boolean enabled; public DefaultAppInfo(int uid, ComponentName cn) { @@ -52,21 +50,19 @@ public class DefaultAppInfo { userId = uid; componentName = cn; this.summary = summary; - this.disabledDescription = null; this.enabled = enabled; } - public DefaultAppInfo(PackageItemInfo info, String description) { + public DefaultAppInfo(PackageItemInfo info, String summary, boolean enabled) { userId = UserHandle.myUserId(); packageItemInfo = info; componentName = null; - summary = null; - this.disabledDescription = description; - enabled = true; + this.summary = summary; + this.enabled = enabled; } public DefaultAppInfo(PackageItemInfo info) { - this(info, null); + this(info, null /* summary */, true /* enabled */); } public CharSequence loadLabel(PackageManager pm) { diff --git a/src/com/android/settings/applications/defaultapps/DefaultAppPickerFragment.java b/src/com/android/settings/applications/defaultapps/DefaultAppPickerFragment.java index e4bcf067e72..785f7add124 100644 --- a/src/com/android/settings/applications/defaultapps/DefaultAppPickerFragment.java +++ b/src/com/android/settings/applications/defaultapps/DefaultAppPickerFragment.java @@ -116,26 +116,9 @@ public abstract class DefaultAppPickerFragment extends InstrumentedPreferenceFra screen.addPreference(nonePref); } for (Map.Entry app : mCandidates.entrySet()) { - final RadioButtonPreference pref = new RadioButtonPreference(getPrefContext()); - final String appKey = app.getKey(); - final DefaultAppInfo info = app.getValue(); - pref.setTitle(info.loadLabel(mPm.getPackageManager())); - pref.setIcon(info.loadIcon(mPm.getPackageManager())); - pref.setKey(appKey); - if (TextUtils.equals(defaultAppKey, appKey)) { - pref.setChecked(true); - } - if (TextUtils.equals(systemDefaultAppKey, appKey)) { - pref.setSummary(R.string.system_app); - } else if (!TextUtils.isEmpty(info.summary)) { - pref.setSummary(info.summary); - } - if (!TextUtils.isEmpty(app.getValue().disabledDescription)) { - pref.setEnabled(false); - pref.setSummary(app.getValue().disabledDescription); - } - pref.setEnabled(info.enabled); - pref.setOnClickListener(this); + RadioButtonPreference pref = new RadioButtonPreference(getPrefContext()); + configurePreferenceFromAppInfo( + pref, app.getKey(), app.getValue(), defaultAppKey, systemDefaultAppKey); screen.addPreference(pref); } mayCheckOnlyRadioButton(); @@ -259,4 +242,22 @@ public abstract class DefaultAppPickerFragment extends InstrumentedPreferenceFra } } + @VisibleForTesting + public RadioButtonPreference configurePreferenceFromAppInfo(RadioButtonPreference pref, + String appKey, DefaultAppInfo info, String defaultAppKey, String systemDefaultAppKey) { + pref.setTitle(info.loadLabel(mPm.getPackageManager())); + pref.setIcon(info.loadIcon(mPm.getPackageManager())); + pref.setKey(appKey); + if (TextUtils.equals(defaultAppKey, appKey)) { + pref.setChecked(true); + } + if (TextUtils.equals(systemDefaultAppKey, appKey)) { + pref.setSummary(R.string.system_app); + } else if (!TextUtils.isEmpty(info.summary)) { + pref.setSummary(info.summary); + } + pref.setEnabled(info.enabled); + pref.setOnClickListener(this); + return pref; + } } diff --git a/src/com/android/settings/webview/WebViewAppPicker.java b/src/com/android/settings/webview/WebViewAppPicker.java index aa229ac05a0..586ee36a6c6 100644 --- a/src/com/android/settings/webview/WebViewAppPicker.java +++ b/src/com/android/settings/webview/WebViewAppPicker.java @@ -24,6 +24,7 @@ import android.content.pm.ApplicationInfo; import android.content.pm.PackageInfo; import android.content.Context; import android.support.annotation.VisibleForTesting; +import android.text.TextUtils; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.settings.R; @@ -58,7 +59,7 @@ public class WebViewAppPicker extends DefaultAppPickerFragment { List pkgs = getWebViewUpdateServiceWrapper().getValidWebViewApplicationInfos(getContext()); for (ApplicationInfo ai : pkgs) { - packageInfoList.add(new DefaultAppInfo(ai, + packageInfoList.add(createDefaultAppInfo(ai, getDisabledReason(getWebViewUpdateServiceWrapper(), getContext(), ai.packageName))); } @@ -92,7 +93,6 @@ public class WebViewAppPicker extends DefaultAppPickerFragment { } } - private WebViewUpdateServiceWrapper createDefaultWebViewUpdateServiceWrapper() { return new WebViewUpdateServiceWrapper(); } @@ -107,6 +107,13 @@ public class WebViewAppPicker extends DefaultAppPickerFragment { return MetricsEvent.WEBVIEW_IMPLEMENTATION; } + @VisibleForTesting + DefaultAppInfo createDefaultAppInfo( + ApplicationInfo applicationInfo, String disabledReason) { + return new DefaultAppInfo(applicationInfo, disabledReason, + TextUtils.isEmpty(disabledReason) /* enabled */); + } + /** * Returns the reason why a package cannot be used as WebView implementation. * This is either because of it being disabled, uninstalled, or hidden for any user. diff --git a/tests/robotests/src/com/android/settings/webview/WebViewAppPickerTest.java b/tests/robotests/src/com/android/settings/webview/WebViewAppPickerTest.java index 78839faeef8..c58ce37149f 100644 --- a/tests/robotests/src/com/android/settings/webview/WebViewAppPickerTest.java +++ b/tests/robotests/src/com/android/settings/webview/WebViewAppPickerTest.java @@ -35,11 +35,13 @@ import android.content.Intent; import android.content.pm.ApplicationInfo; import android.content.pm.UserInfo; import android.os.UserManager; +import android.support.v7.preference.PreferenceManager; import com.android.settings.SettingsRobolectricTestRunner; import com.android.settings.TestConfig; -import com.android.settings.widget.RadioButtonPreference; import com.android.settings.applications.PackageManagerWrapper; +import com.android.settings.applications.defaultapps.DefaultAppInfo; +import com.android.settings.widget.RadioButtonPreference; import java.util.Arrays; @@ -51,6 +53,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; +import org.robolectric.util.ReflectionHelpers; @RunWith(SettingsRobolectricTestRunner.class) @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) @@ -66,7 +69,7 @@ public class WebViewAppPickerTest { private Activity mActivity; @Mock private UserManager mUserManager; - @Mock + @Mock(answer = Answers.RETURNS_DEEP_STUBS) private PackageManagerWrapper mPackageManager; private WebViewAppPicker mPicker; @@ -89,6 +92,8 @@ public class WebViewAppPickerTest { doNothing().when(mPicker).updateCheckedState(any()); doReturn(mActivity).when(mPicker).getActivity(); + ReflectionHelpers.setField(mPicker, "mPm", mPackageManager); + mWvusWrapper = mock(WebViewUpdateServiceWrapper.class); mPicker.setWebViewUpdateServiceWrapper(mWvusWrapper); } @@ -151,6 +156,62 @@ public class WebViewAppPickerTest { verify(mWvusWrapper, times(1)).showInvalidChoiceToast(any()); } + @Test + public void testDisabledPackageShownAsDisabled() { + String disabledReason = "disabled"; + DefaultAppInfo webviewAppInfo = mPicker.createDefaultAppInfo( + createApplicationInfo(DEFAULT_PACKAGE_NAME), disabledReason); + + RadioButtonPreference mockPreference = mock(RadioButtonPreference.class); + mPicker.configurePreferenceFromAppInfo(mockPreference, + DEFAULT_PACKAGE_NAME, webviewAppInfo, null, null); + + verify(mockPreference, times(1)).setEnabled(eq(false)); + verify(mockPreference, never()).setEnabled(eq(true)); + } + + @Test + public void testEnabledPackageShownAsEnabled() { + String disabledReason = ""; + DefaultAppInfo webviewAppInfo = mPicker.createDefaultAppInfo( + createApplicationInfo(DEFAULT_PACKAGE_NAME), disabledReason); + + RadioButtonPreference mockPreference = mock(RadioButtonPreference.class); + mPicker.configurePreferenceFromAppInfo(mockPreference, + DEFAULT_PACKAGE_NAME, webviewAppInfo, null, null); + + verify(mockPreference, times(1)).setEnabled(eq(true)); + verify(mockPreference, never()).setEnabled(eq(false)); + } + + @Test + public void testDisabledPackageShowsDisabledReasonSummary() { + String disabledReason = "disabled"; + DefaultAppInfo webviewAppInfo = mPicker.createDefaultAppInfo( + createApplicationInfo(DEFAULT_PACKAGE_NAME), disabledReason); + + RadioButtonPreference mockPreference = mock(RadioButtonPreference.class); + mPicker.configurePreferenceFromAppInfo(mockPreference, + DEFAULT_PACKAGE_NAME, webviewAppInfo, null, null); + + verify(mockPreference, times(1)).setSummary(eq(disabledReason)); + // Ensure we haven't called setSummary several times. + verify(mockPreference, times(1)).setSummary(any()); + } + + @Test + public void testEnabledPackageShowsEmptySummary() { + String disabledReason = null; + DefaultAppInfo webviewAppInfo = mPicker.createDefaultAppInfo( + createApplicationInfo(DEFAULT_PACKAGE_NAME), disabledReason); + + RadioButtonPreference mockPreference = mock(RadioButtonPreference.class); + mPicker.configurePreferenceFromAppInfo(mockPreference, + DEFAULT_PACKAGE_NAME, webviewAppInfo, null, null); + + verify(mockPreference, never()).setSummary(any()); + } + @Test public void testFinishIfNotAdmin() { doReturn(false).when(mUserManager).isAdminUser();