From 36924659f5ba108cdea323d15a4fbe97a9cca2e2 Mon Sep 17 00:00:00 2001 From: Fan Zhang Date: Fri, 7 Oct 2016 08:38:48 -0700 Subject: [PATCH] Refactor DashboardFragment. Merged refreshAllPreferences into DashboardFragment. This hopefully makes it more modular to manage preference display logic in each dashboardFragment, and makes it more efficient to monitor category changes. Now subclasses needs to implement 2 methods: - displayResourceTiles(): for 'static' preferences from xml - getDashboardTiles(): returns a list of dashboard tiles and superclass will wire it up to preference screen. If getDashboardTiles() return null (aka no dashboardCategory available), the fragment will not attempt to monitor category change. The edge case is that if a package starts to provide a tile for this category, we will not be notified. I have not seen this case coming up. If we indeed need to handle this case, the category listener needs to have a way to monitor specific category rather than globally. Bug: 31781480 Test: make RunSettingsRoboTests -j40 Change-Id: Ia9f9541b95816214df0d0bb27e3e41078c36c5ca --- .../settings/dashboard/DashboardFragment.java | 70 +++++++++++++++++-- .../settings/dashboard/SummaryLoader.java | 3 +- .../deviceinfo/StorageDashboardFragment.java | 27 +++---- .../system/SystemDashboardFragment.java | 38 ++++------ .../dashboard/DashboardFragmentTest.java | 40 ++++++++--- 5 files changed, 117 insertions(+), 61 deletions(-) diff --git a/src/com/android/settings/dashboard/DashboardFragment.java b/src/com/android/settings/dashboard/DashboardFragment.java index 8a0a753e7d0..1388be7c628 100644 --- a/src/com/android/settings/dashboard/DashboardFragment.java +++ b/src/com/android/settings/dashboard/DashboardFragment.java @@ -17,6 +17,7 @@ package com.android.settings.dashboard; import android.app.Activity; import android.content.Context; +import android.os.Bundle; import android.support.v7.preference.Preference; import android.support.v7.preference.PreferenceScreen; import android.text.TextUtils; @@ -45,6 +46,7 @@ public abstract class DashboardFragment extends SettingsPreferenceFragment new ArrayMap<>(); protected DashboardFeatureProvider mDashboardFeatureProvider; + private boolean mListeningToCategoryChange; @Override public void onAttach(Context context) { @@ -53,11 +55,32 @@ public abstract class DashboardFragment extends SettingsPreferenceFragment FeatureFactory.getFactory(context).getDashboardFeatureProvider(context); } + @Override + public void onCategoriesChanged() { + final DashboardCategory category = getDashboardTiles(); + if (category == null) { + return; + } + refreshAllPreferences(getLogTag()); + } + + @Override + public void onCreatePreferences(Bundle savedInstanceState, String rootKey) { + super.onCreatePreferences(savedInstanceState, rootKey); + refreshAllPreferences(getLogTag()); + } + @Override public void onStart() { super.onStart(); + final DashboardCategory category = getDashboardTiles(); + if (category == null) { + return; + } + final Activity activity = getActivity(); if (activity instanceof SettingsDrawerActivity) { + mListeningToCategoryChange = true; ((SettingsDrawerActivity) activity).addCategoryListener(this); } } @@ -77,9 +100,12 @@ public abstract class DashboardFragment extends SettingsPreferenceFragment @Override public void onStop() { super.onStop(); - final Activity activity = getActivity(); - if (activity instanceof SettingsDrawerActivity) { - ((SettingsDrawerActivity) activity).remCategoryListener(this); + if (mListeningToCategoryChange) { + final Activity activity = getActivity(); + if (activity instanceof SettingsDrawerActivity) { + ((SettingsDrawerActivity) activity).remCategoryListener(this); + } + mListeningToCategoryChange = false; } } @@ -92,9 +118,28 @@ public abstract class DashboardFragment extends SettingsPreferenceFragment mPreferenceControllers.put(controller.getClass(), controller); } - protected final void displayTilesAsPreference(String TAG, PreferenceScreen screen, - DashboardCategory category) { + /** + * Returns {@link DashboardCategory} for this fragment. + */ + protected abstract DashboardCategory getDashboardTiles(); + + /** + * Displays resource based tiles. + */ + protected abstract void displayResourceTiles(); + + protected abstract String getLogTag(); + + /** + * Displays dashboard tiles as preference. + */ + private final void displayDashboardTiles(final String TAG, PreferenceScreen screen) { final Context context = getContext(); + final DashboardCategory category = getDashboardTiles(); + if (category == null) { + Log.d(TAG, "NO dynamic tiles for " + TAG); + return; + } List tiles = category.tiles; if (tiles == null) { Log.d(TAG, "tile list is empty, skipping category " + category.title); @@ -123,4 +168,19 @@ public abstract class DashboardFragment extends SettingsPreferenceFragment screen.addPreference(pref); } } + + /** + * Refresh preference items using system category dashboard items. + */ + private void refreshAllPreferences(final String TAG) { + // First remove old preferences. + PreferenceScreen screen = getPreferenceScreen(); + if (screen != null) { + screen.removeAll(); + } + // Add resource based tiles. + displayResourceTiles(); + // Add dashboard tiles. + displayDashboardTiles(TAG, getPreferenceScreen()); + } } diff --git a/src/com/android/settings/dashboard/SummaryLoader.java b/src/com/android/settings/dashboard/SummaryLoader.java index d55ec06f10e..9a6c944ee6d 100644 --- a/src/com/android/settings/dashboard/SummaryLoader.java +++ b/src/com/android/settings/dashboard/SummaryLoader.java @@ -80,8 +80,7 @@ public class SummaryLoader { mWorker = new Worker(mWorkerThread.getLooper()); mActivity = activity; List tiles = categories.tiles; - for (int j = 0; j < tiles.size(); j++) { - Tile tile = tiles.get(j); + for (Tile tile :tiles) { mWorker.obtainMessage(Worker.MSG_GET_PROVIDER, tile).sendToTarget(); } } diff --git a/src/com/android/settings/deviceinfo/StorageDashboardFragment.java b/src/com/android/settings/deviceinfo/StorageDashboardFragment.java index cb6343a16f0..92de6b7488f 100644 --- a/src/com/android/settings/deviceinfo/StorageDashboardFragment.java +++ b/src/com/android/settings/deviceinfo/StorageDashboardFragment.java @@ -17,16 +17,14 @@ package com.android.settings.deviceinfo; import android.content.Context; -import android.os.Bundle; -import android.os.UserManager; import android.provider.SearchIndexableResource; -import android.support.v7.preference.PreferenceScreen; import com.android.settings.R; import com.android.settings.dashboard.DashboardFragment; import com.android.settings.overlay.FeatureFactory; import com.android.settings.search.BaseSearchIndexProvider; import com.android.settings.search.Indexable; +import com.android.settingslib.drawer.DashboardCategory; import java.util.ArrayList; import java.util.Arrays; @@ -41,6 +39,11 @@ public class StorageDashboardFragment extends DashboardFragment { return STORAGE_CATEGORY_FRAGMENT; } + @Override + protected String getLogTag() { + return TAG; + } + @Override public void onAttach(Context context) { super.onAttach(context); @@ -48,28 +51,16 @@ public class StorageDashboardFragment extends DashboardFragment { } @Override - public void onCreatePreferences(Bundle savedInstanceState, String rootKey) { - super.onCreatePreferences(savedInstanceState, rootKey); - refreshAllPreferences(); + protected DashboardCategory getDashboardTiles() { + return mDashboardFeatureProvider.getTilesForStorageCategory(); } @Override - public void onCategoriesChanged() { - refreshAllPreferences(); - } - - private void refreshAllPreferences() { - PreferenceScreen screen = getPreferenceScreen(); - if (screen != null) { - screen.removeAll(); - } + protected void displayResourceTiles() { addPreferencesFromResource(R.xml.storage_dashboard_fragment); getPreferenceController(ManageStoragePreferenceController.class) .displayPreference(getPreferenceScreen()); - - displayTilesAsPreference(TAG, getPreferenceScreen(), - mDashboardFeatureProvider.getTilesForStorageCategory()); } /** diff --git a/src/com/android/settings/system/SystemDashboardFragment.java b/src/com/android/settings/system/SystemDashboardFragment.java index 54bd3e63c32..a2e9152a207 100644 --- a/src/com/android/settings/system/SystemDashboardFragment.java +++ b/src/com/android/settings/system/SystemDashboardFragment.java @@ -16,10 +16,8 @@ package com.android.settings.system; import android.content.Context; -import android.os.Bundle; import android.os.UserManager; import android.provider.SearchIndexableResource; -import android.support.v7.preference.PreferenceScreen; import com.android.settings.R; import com.android.settings.dashboard.DashboardFragment; @@ -27,15 +25,14 @@ import com.android.settings.deviceinfo.SystemUpdatePreferenceController; import com.android.settings.overlay.FeatureFactory; import com.android.settings.search.BaseSearchIndexProvider; import com.android.settings.search.Indexable; -import com.android.settingslib.drawer.SettingsDrawerActivity; +import com.android.settingslib.drawer.DashboardCategory; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -public class SystemDashboardFragment extends DashboardFragment - implements SettingsDrawerActivity.CategoryListener, Indexable { +public class SystemDashboardFragment extends DashboardFragment { private static final String TAG = "SystemDashboardFrag"; @@ -44,6 +41,11 @@ public class SystemDashboardFragment extends DashboardFragment return SYSTEM_CATEGORY_FRAGMENT; } + @Override + protected String getLogTag() { + return TAG; + } + @Override public void onAttach(Context context) { super.onAttach(context); @@ -52,32 +54,16 @@ public class SystemDashboardFragment extends DashboardFragment } @Override - public void onCreatePreferences(Bundle savedInstanceState, String rootKey) { - super.onCreatePreferences(savedInstanceState, rootKey); - refreshAllPreferences(); - } - - @Override - public void onCategoriesChanged() { - refreshAllPreferences(); - } - - /** - * Refresh preference items using system category dashboard items. - */ - private void refreshAllPreferences() { - PreferenceScreen screen = getPreferenceScreen(); - if (screen != null) { - screen.removeAll(); - } - + protected void displayResourceTiles() { addPreferencesFromResource(R.xml.system_dashboard_fragment); getPreferenceController(SystemUpdatePreferenceController.class) .displayPreference(getPreferenceScreen()); + } - displayTilesAsPreference(TAG, getPreferenceScreen(), - mDashboardFeatureProvider.getTilesForSystemCategory()); + @Override + protected DashboardCategory getDashboardTiles() { + return mDashboardFeatureProvider.getTilesForSystemCategory(); } /** diff --git a/tests/robotests/src/com/android/settings/dashboard/DashboardFragmentTest.java b/tests/robotests/src/com/android/settings/dashboard/DashboardFragmentTest.java index de233667e2a..f4371a670ce 100644 --- a/tests/robotests/src/com/android/settings/dashboard/DashboardFragmentTest.java +++ b/tests/robotests/src/com/android/settings/dashboard/DashboardFragmentTest.java @@ -16,6 +16,7 @@ package com.android.settings.dashboard; import android.content.Context; +import android.os.Bundle; import android.support.v7.preference.Preference; import android.support.v7.preference.PreferenceScreen; @@ -41,6 +42,7 @@ import java.util.List; import static com.google.common.truth.Truth.assertThat; import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -55,8 +57,6 @@ public class DashboardFragmentTest { private DashboardCategory mDashboardCategory; @Mock private FakeFeatureFactory mFakeFeatureFactory; - @Mock - private PreferenceScreen mScreen; private TestFragment mTestFragment; @Before @@ -68,6 +68,7 @@ public class DashboardFragmentTest { mDashboardCategory.tiles.add(new Tile()); mTestFragment = new TestFragment(ShadowApplication.getInstance().getApplicationContext()); mTestFragment.onAttach(mContext); + mTestFragment.mCategory = mDashboardCategory; } @Test @@ -85,24 +86,24 @@ public class DashboardFragmentTest { public void displayTilesAsPreference_shouldAddTilesWithIntent() { when(mFakeFeatureFactory.dashboardFeatureProvider.getDashboardKeyForTile(any(Tile.class))) .thenReturn("test_key"); - mTestFragment.displayTilesAsPreference("TEST_FRAGMENT", mScreen, mDashboardCategory); + mTestFragment.onCreatePreferences(new Bundle(), "rootKey"); - verify(mScreen).addPreference(any(DashboardTilePreference.class)); + verify(mTestFragment.mScreen).addPreference(any(DashboardTilePreference.class)); } @Test public void displayTilesAsPreference_shouldNotAddTilesWithoutIntent() { - mTestFragment.displayTilesAsPreference("TEST_FRAGMENT", mScreen, mDashboardCategory); + mTestFragment.onCreatePreferences(new Bundle(), "rootKey"); - verify(mScreen, never()).addPreference(any(DashboardTilePreference.class)); + verify(mTestFragment.mScreen, never()).addPreference(any(DashboardTilePreference.class)); } @Test public void displayTilesAsPreference_withEmptyCategory_shouldNotAddTiles() { - mDashboardCategory.tiles = null; - mTestFragment.displayTilesAsPreference("TEST_FRAGMENT", mScreen, mDashboardCategory); + mTestFragment.mCategory.tiles = null; + mTestFragment.onCreatePreferences(new Bundle(), "rootKey"); - verify(mScreen, never()).addPreference(any(DashboardTilePreference.class)); + verify(mTestFragment.mScreen, never()).addPreference(any(DashboardTilePreference.class)); } public static class TestPreferenceController extends PreferenceController { @@ -130,9 +131,14 @@ public class DashboardFragmentTest { public static class TestFragment extends DashboardFragment { private final Context mContext; + @Mock + public PreferenceScreen mScreen; + public DashboardCategory mCategory; + public TestFragment(Context context) { mContext = context; + mScreen = mock(PreferenceScreen.class); } @Override @@ -146,8 +152,22 @@ public class DashboardFragmentTest { } @Override - public void onCategoriesChanged() { + protected DashboardCategory getDashboardTiles() { + return mCategory; + } + @Override + protected void displayResourceTiles() { + } + + @Override + public PreferenceScreen getPreferenceScreen() { + return mScreen; + } + + @Override + protected String getLogTag() { + return "TEST_FRAG"; } }