From 6a256e77d624967470f60c68dab6d5447e8a24cb Mon Sep 17 00:00:00 2001 From: Daniel Nishi Date: Thu, 2 Mar 2017 12:52:40 -0800 Subject: [PATCH] Fix a crash when transferring an app. This crash occurs because the background loader is loading the storage for an app which is currently being moved off of the device. In the native code which is calculating the storage, it throws an InstallerException which is caught and rethrown as an IllegalStateException. We handle this in the view by reporting that we could not calculate the size of the app. Change-Id: I109b1be60ae60f8ef31b08cb4392b576261fa806 Fixes: 35922033 Test: Robotest --- res/xml/app_storage_settings.xml | 12 -- .../applications/AppStorageSettings.java | 69 ++------ .../AppStorageSizesController.java | 155 ++++++++++++++++++ .../FetchPackageStorageAsyncLoader.java | 10 +- .../AppStorageSizesControllerTest.java | 95 +++++++++++ .../FetchPackageStorageAsyncLoaderTest.java | 15 +- 6 files changed, 285 insertions(+), 71 deletions(-) create mode 100644 src/com/android/settings/applications/AppStorageSizesController.java create mode 100644 tests/robotests/src/com/android/settings/applications/AppStorageSizesControllerTest.java diff --git a/res/xml/app_storage_settings.xml b/res/xml/app_storage_settings.xml index 0254b137ed9..6bd8ae3314b 100644 --- a/res/xml/app_storage_settings.xml +++ b/res/xml/app_storage_settings.xml @@ -50,24 +50,12 @@ android:selectable="false" android:layout="@layout/horizontal_preference" /> - - - - loader, AppStorageStats result) { - mLastResult = result; + mSizeController.setResult(result); updateUiWithSize(result); } @@ -545,39 +524,15 @@ public class AppStorageSettings extends AppInfoWithHeader } private void updateUiWithSize(AppStorageStats result) { + mSizeController.updateUi(getContext()); + if (result == null) { - mLastCodeSize = mLastDataSize = mLastCacheSize = mLastTotalSize = -1; - if (!mHaveSizes) { - mAppSize.setSummary(mComputingStr); - mDataSize.setSummary(mComputingStr); - mCacheSize.setSummary(mComputingStr); - mTotalSize.setSummary(mComputingStr); - } mClearDataButton.setEnabled(false); mClearCacheButton.setEnabled(false); } else { - mHaveSizes = true; long codeSize = result.getCodeBytes(); long dataSize = result.getDataBytes(); - if (mLastCodeSize != codeSize) { - mLastCodeSize = codeSize; - mAppSize.setSummary(getSizeStr(codeSize)); - } - if (mLastDataSize != dataSize) { - mLastDataSize = dataSize; - mDataSize.setSummary(getSizeStr(dataSize)); - } long cacheSize = result.getCacheBytes(); - if (mLastCacheSize != cacheSize) { - mLastCacheSize = cacheSize; - mCacheSize.setSummary(getSizeStr(cacheSize)); - } - - long totalSize = codeSize + dataSize + cacheSize; - if (mLastTotalSize != totalSize) { - mLastTotalSize = totalSize; - mTotalSize.setSummary(getSizeStr(totalSize)); - } if (dataSize <= 0 || !mCanClearData) { mClearDataButton.setEnabled(false); diff --git a/src/com/android/settings/applications/AppStorageSizesController.java b/src/com/android/settings/applications/AppStorageSizesController.java new file mode 100644 index 00000000000..bc8f680cc2c --- /dev/null +++ b/src/com/android/settings/applications/AppStorageSizesController.java @@ -0,0 +1,155 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +package com.android.settings.applications; + +import android.content.Context; +import android.support.annotation.Nullable; +import android.support.annotation.StringRes; +import android.support.v7.preference.Preference; +import android.text.format.Formatter; + +import com.android.internal.util.Preconditions; +import com.android.settingslib.applications.StorageStatsSource; + +/** + * Handles setting the sizes for the app info screen. + */ +public class AppStorageSizesController { + private final Preference mTotalSize; + private final Preference mAppSize; + private final Preference mDataSize; + private final Preference mCacheSize; + private final @StringRes int mComputing; + private final @StringRes int mError; + + @Nullable + private StorageStatsSource.AppStorageStats mLastResult; + private boolean mLastResultFailed; + private long mLastCodeSize = -1; + private long mLastDataSize = -1; + private long mLastCacheSize = -1; + private long mLastTotalSize = -1; + + private AppStorageSizesController(Preference total, Preference app, + Preference data, Preference cache, @StringRes int computing, @StringRes int error) { + mTotalSize = total; + mAppSize = app; + mDataSize = data; + mCacheSize = cache; + mComputing = computing; + mError = error; + } + + /** + * Updates the UI using storage stats. + * @param context Context to use to fetch strings + */ + public void updateUi(Context context) { + if (mLastResult == null) { + int errorRes = mLastResultFailed ? mError : mComputing; + + mAppSize.setSummary(errorRes); + mDataSize.setSummary(errorRes); + mCacheSize.setSummary(errorRes); + mTotalSize.setSummary(errorRes); + } else { + long codeSize = mLastResult.getCodeBytes(); + long dataSize = mLastResult.getDataBytes(); + if (mLastCodeSize != codeSize) { + mLastCodeSize = codeSize; + mAppSize.setSummary(getSizeStr(context, codeSize)); + } + if (mLastDataSize != dataSize) { + mLastDataSize = dataSize; + mDataSize.setSummary(getSizeStr(context, dataSize)); + } + long cacheSize = mLastResult.getCacheBytes(); + if (mLastCacheSize != cacheSize) { + mLastCacheSize = cacheSize; + mCacheSize.setSummary(getSizeStr(context, cacheSize)); + } + + long totalSize = codeSize + dataSize + cacheSize; + if (mLastTotalSize != totalSize) { + mLastTotalSize = totalSize; + mTotalSize.setSummary(getSizeStr(context, totalSize)); + } + } + } + + /** + * Sets a result for the controller to use to update the UI. + * @param result A result for the UI. If null, count as a failed calculation. + */ + public void setResult(StorageStatsSource.AppStorageStats result) { + mLastResult = result; + mLastResultFailed = result == null; + } + + private String getSizeStr(Context context, long size) { + return Formatter.formatFileSize(context, size); + } + + public static class Builder { + private Preference mTotalSize; + private Preference mAppSize; + private Preference mDataSize; + private Preference mCacheSize; + private @StringRes int mComputing; + private @StringRes int mError; + + public Builder setAppSizePreference(Preference preference) { + mAppSize = preference; + return this; + } + + public Builder setDataSizePreference(Preference preference) { + mDataSize = preference; + return this; + } + + public Builder setCacheSizePreference(Preference preference) { + mCacheSize = preference; + return this; + } + + public Builder setTotalSizePreference(Preference preference) { + mTotalSize = preference; + return this; + } + + public Builder setComputingString(@StringRes int sequence) { + mComputing = sequence; + return this; + } + + public Builder setErrorString(@StringRes int sequence) { + mError = sequence; + return this; + } + + public AppStorageSizesController build() { + return new AppStorageSizesController( + Preconditions.checkNotNull(mTotalSize), + Preconditions.checkNotNull(mAppSize), + Preconditions.checkNotNull(mDataSize), + Preconditions.checkNotNull(mCacheSize), + mComputing, + mError); + } + } +} diff --git a/src/com/android/settings/applications/FetchPackageStorageAsyncLoader.java b/src/com/android/settings/applications/FetchPackageStorageAsyncLoader.java index 347729928eb..97e5b7bf9d3 100644 --- a/src/com/android/settings/applications/FetchPackageStorageAsyncLoader.java +++ b/src/com/android/settings/applications/FetchPackageStorageAsyncLoader.java @@ -20,6 +20,7 @@ import android.annotation.NonNull; import android.content.Context; import android.content.pm.ApplicationInfo; import android.os.UserHandle; +import android.util.Log; import com.android.internal.util.Preconditions; import com.android.settings.utils.AsyncLoader; @@ -30,6 +31,7 @@ import com.android.settingslib.applications.StorageStatsSource.AppStorageStats; * Fetches the storage stats using the StorageStatsManager for a given package and user tuple. */ public class FetchPackageStorageAsyncLoader extends AsyncLoader { + private static final String TAG = "FetchPackageStorage"; private final StorageStatsSource mSource; private final ApplicationInfo mInfo; private final UserHandle mUser; @@ -44,7 +46,13 @@ public class FetchPackageStorageAsyncLoader extends AsyncLoader @Override public AppStorageStats loadInBackground() { - return mSource.getStatsForPackage(mInfo.volumeUuid, mInfo.packageName, mUser); + AppStorageStats result = null; + try { + result = mSource.getStatsForPackage(mInfo.volumeUuid, mInfo.packageName, mUser); + } catch (IllegalStateException e) { + Log.w(TAG, "Package may have been removed during query, failing gracefully", e); + } + return result; } @Override diff --git a/tests/robotests/src/com/android/settings/applications/AppStorageSizesControllerTest.java b/tests/robotests/src/com/android/settings/applications/AppStorageSizesControllerTest.java new file mode 100644 index 00000000000..7204bd13f70 --- /dev/null +++ b/tests/robotests/src/com/android/settings/applications/AppStorageSizesControllerTest.java @@ -0,0 +1,95 @@ +package com.android.settings.applications; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import android.content.Context; +import android.support.v7.preference.Preference; + +import com.android.settings.SettingsRobolectricTestRunner; +import com.android.settings.TestConfig; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowApplication; + +import com.android.settings.R; +import com.android.settingslib.applications.StorageStatsSource.AppStorageStats; + +@RunWith(SettingsRobolectricTestRunner.class) +@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) +public class AppStorageSizesControllerTest { + private static final String COMPUTING = "Computing…"; + private static final String INVALID_SIZE = "Couldn’t compute package size."; + private AppStorageSizesController mController; + private Context mContext; + + private Preference mAppPreference; + private Preference mCachePreference; + private Preference mDataPreference; + private Preference mTotalPreference; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mContext = RuntimeEnvironment.application; + mAppPreference = new Preference(mContext); + mCachePreference = new Preference(mContext); + mDataPreference = new Preference(mContext); + mTotalPreference = new Preference(mContext); + + mController = new AppStorageSizesController.Builder() + .setAppSizePreference(mAppPreference) + .setCacheSizePreference(mCachePreference) + .setDataSizePreference(mDataPreference) + .setTotalSizePreference(mTotalPreference) + .setErrorString(R.string.invalid_size_value) + .setComputingString(R.string.computing_size) + .build(); + } + + @Test + public void requestingUpdateBeforeValuesSetIsComputing() { + mController.updateUi(mContext); + + assertThat(mAppPreference.getSummary()).isEqualTo(COMPUTING); + assertThat(mCachePreference.getSummary()).isEqualTo(COMPUTING); + assertThat(mDataPreference.getSummary()).isEqualTo(COMPUTING); + assertThat(mTotalPreference.getSummary()).isEqualTo(COMPUTING); + } + + @Test + public void requestingUpdateAfterFailureHasErrorText() { + mController.setResult(null); + mController.updateUi(mContext); + + assertThat(mAppPreference.getSummary()).isEqualTo(INVALID_SIZE); + assertThat(mCachePreference.getSummary()).isEqualTo(INVALID_SIZE); + assertThat(mDataPreference.getSummary()).isEqualTo(INVALID_SIZE); + assertThat(mTotalPreference.getSummary()).isEqualTo(INVALID_SIZE); + } + + @Test + public void properlyPopulatedAfterValidEntry() { + AppStorageStats result = mock(AppStorageStats.class); + when(result.getCodeBytes()).thenReturn(1L); + when(result.getCacheBytes()).thenReturn(10L); + when(result.getDataBytes()).thenReturn(100L); + when(result.getTotalBytes()).thenReturn(111L); + + mController.setResult(result); + mController.updateUi(mContext); + + assertThat(mAppPreference.getSummary()).isEqualTo("1.00B"); + assertThat(mCachePreference.getSummary()).isEqualTo("10.00B"); + assertThat(mDataPreference.getSummary()).isEqualTo("100B"); + assertThat(mTotalPreference.getSummary()).isEqualTo("111B"); + } +} diff --git a/tests/robotests/src/com/android/settings/applications/FetchPackageStorageAsyncLoaderTest.java b/tests/robotests/src/com/android/settings/applications/FetchPackageStorageAsyncLoaderTest.java index 0b1d1aacb30..04eeb022e3c 100644 --- a/tests/robotests/src/com/android/settings/applications/FetchPackageStorageAsyncLoaderTest.java +++ b/tests/robotests/src/com/android/settings/applications/FetchPackageStorageAsyncLoaderTest.java @@ -43,6 +43,7 @@ import org.robolectric.annotation.Config; @RunWith(SettingsRobolectricTestRunner.class) @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) public class FetchPackageStorageAsyncLoaderTest { + private static final String PACKAGE_NAME = "com.test.package"; @Mock(answer = Answers.RETURNS_DEEP_STUBS) private Context mContext; @Mock @@ -63,10 +64,22 @@ public class FetchPackageStorageAsyncLoaderTest { when(mSource.getStatsForPackage(anyString(), anyString(), any(UserHandle.class))) .thenReturn(stats); ApplicationInfo info = new ApplicationInfo(); - info.packageName = "com.test.package"; + info.packageName = PACKAGE_NAME; FetchPackageStorageAsyncLoader task = new FetchPackageStorageAsyncLoader( mContext, mSource, info, new UserHandle(0)); assertThat(task.loadInBackground()).isEqualTo(stats); } + + @Test + public void installerExceptionHandledCleanly() { + when(mSource.getStatsForPackage(anyString(), anyString(), any(UserHandle.class))). + thenThrow(new IllegalStateException("intentional failure")); + ApplicationInfo info = new ApplicationInfo(); + info.packageName = PACKAGE_NAME; + FetchPackageStorageAsyncLoader task = new FetchPackageStorageAsyncLoader( + mContext, mSource, info, new UserHandle(0)); + + assertThat(task.loadInBackground()).isNull(); + } }