From 9955db636016e7f18358f7b5d24a14b55df4dabf Mon Sep 17 00:00:00 2001 From: Matthew Fritze Date: Tue, 20 Dec 2016 09:37:21 -0800 Subject: [PATCH] SearchFeatureProvider's context field is removed SearchFeatureProvider was holding on to a context and outlived the SettingsActivity, thus leaking the activity. The context was passed into most methods, and thus it makes more sense to pass it in to every method. Bug: 33677967 Test: Run MakeSettingsRoboTests Change-Id: Ia82f30e7e0b83587b4baeef28e81da6b8e4303fe --- src/com/android/settings/SettingsActivity.java | 8 ++++---- src/com/android/settings/overlay/FeatureFactory.java | 2 +- .../android/settings/overlay/FeatureFactoryImpl.java | 4 ++-- .../settings/search2/SearchFeatureProvider.java | 2 +- .../settings/search2/SearchFeatureProviderImpl.java | 11 +++-------- src/com/android/settings/search2/SearchFragment.java | 2 +- .../dashboard/DashboardFeatureProviderImplTest.java | 2 +- .../search/SearchFeatureProviderImplTest.java | 10 +++++----- .../settings/testutils/FakeFeatureFactory.java | 2 +- 9 files changed, 19 insertions(+), 24 deletions(-) diff --git a/src/com/android/settings/SettingsActivity.java b/src/com/android/settings/SettingsActivity.java index aa765171ad4..cc203f7fd28 100644 --- a/src/com/android/settings/SettingsActivity.java +++ b/src/com/android/settings/SettingsActivity.java @@ -329,7 +329,7 @@ public class SettingsActivity extends SettingsDrawerActivity } MenuInflater inflater = getMenuInflater(); - if (mSearchFeatureProvider.isEnabled()) { + if (mSearchFeatureProvider.isEnabled(this)) { mSearchFeatureProvider.setUpSearchMenu(menu, this); return true; } @@ -405,7 +405,7 @@ public class SettingsActivity extends SettingsDrawerActivity final FeatureFactory factory = FeatureFactory.getFactory(this); mDashboardFeatureProvider = factory.getDashboardFeatureProvider(this); - mSearchFeatureProvider = factory.getSearchFeatureProvider(this); + mSearchFeatureProvider = factory.getSearchFeatureProvider(); // Should happen before any call to getIntent() getMetaData(); @@ -1097,7 +1097,7 @@ public class SettingsActivity extends SettingsDrawerActivity @Deprecated @Override public boolean onQueryTextSubmit(String query) { - if (mSearchFeatureProvider.isEnabled()) { + if (mSearchFeatureProvider.isEnabled(this)) { return false; } mSearchQuery = query; @@ -1109,7 +1109,7 @@ public class SettingsActivity extends SettingsDrawerActivity @Override public boolean onQueryTextChange(String newText) { mSearchQuery = newText; - if (mSearchFeatureProvider.isEnabled() || mSearchResultsFragment == null) { + if (mSearchFeatureProvider.isEnabled(this) || mSearchResultsFragment == null) { return false; } return mSearchResultsFragment.onQueryTextChange(newText); diff --git a/src/com/android/settings/overlay/FeatureFactory.java b/src/com/android/settings/overlay/FeatureFactory.java index 364d5ed31e8..0b8ee8e1a98 100644 --- a/src/com/android/settings/overlay/FeatureFactory.java +++ b/src/com/android/settings/overlay/FeatureFactory.java @@ -81,7 +81,7 @@ public abstract class FeatureFactory { public abstract EnterprisePrivacyFeatureProvider getEnterprisePrivacyFeatureProvider( Context context); - public abstract SearchFeatureProvider getSearchFeatureProvider(Context context); + public abstract SearchFeatureProvider getSearchFeatureProvider(); public abstract SurveyFeatureProvider getSurveyFeatureProvider(Context context); diff --git a/src/com/android/settings/overlay/FeatureFactoryImpl.java b/src/com/android/settings/overlay/FeatureFactoryImpl.java index 2aa9b34ca86..c2d5d791cad 100644 --- a/src/com/android/settings/overlay/FeatureFactoryImpl.java +++ b/src/com/android/settings/overlay/FeatureFactoryImpl.java @@ -104,9 +104,9 @@ public class FeatureFactoryImpl extends FeatureFactory { } @Override - public SearchFeatureProvider getSearchFeatureProvider(Context context) { + public SearchFeatureProvider getSearchFeatureProvider() { if (mSearchFeatureProvider == null) { - mSearchFeatureProvider = new SearchFeatureProviderImpl(context); + mSearchFeatureProvider = new SearchFeatureProviderImpl(); } return mSearchFeatureProvider; } diff --git a/src/com/android/settings/search2/SearchFeatureProvider.java b/src/com/android/settings/search2/SearchFeatureProvider.java index ad26eae06d8..8a616a7d399 100644 --- a/src/com/android/settings/search2/SearchFeatureProvider.java +++ b/src/com/android/settings/search2/SearchFeatureProvider.java @@ -27,7 +27,7 @@ public interface SearchFeatureProvider { /** * @return true to use the new version of search */ - boolean isEnabled(); + boolean isEnabled(Context context); /** * Inserts the Menu items into Settings activity. diff --git a/src/com/android/settings/search2/SearchFeatureProviderImpl.java b/src/com/android/settings/search2/SearchFeatureProviderImpl.java index e2d25ad83ea..acb90b72ba2 100644 --- a/src/com/android/settings/search2/SearchFeatureProviderImpl.java +++ b/src/com/android/settings/search2/SearchFeatureProviderImpl.java @@ -31,16 +31,11 @@ import com.android.settings.applications.PackageManagerWrapperImpl; * FeatureProvider for the refactored search code. */ public class SearchFeatureProviderImpl implements SearchFeatureProvider { - protected Context mContext; private DatabaseIndexingManager mDatabaseIndexingManager; - public SearchFeatureProviderImpl(Context context) { - mContext = context; - } - @Override - public boolean isEnabled() { + public boolean isEnabled(Context context) { return false; } @@ -49,7 +44,7 @@ public class SearchFeatureProviderImpl implements SearchFeatureProvider { if (menu == null || activity == null) { return; } - String menuTitle = mContext.getString(R.string.search_menu); + String menuTitle = activity.getString(R.string.search_menu); MenuItem menuItem = menu.add(Menu.NONE, Menu.NONE, Menu.NONE, menuTitle) .setIcon(R.drawable.abc_ic_search_api_material) .setOnMenuItemClickListener(new MenuItem.OnMenuItemClickListener() { @@ -86,7 +81,7 @@ public class SearchFeatureProviderImpl implements SearchFeatureProvider { @Override public void updateIndex(Context context) { - if (isEnabled()) { + if (isEnabled(context)) { getIndexingManager(context).update(); } else { Index.getInstance(context).update(); diff --git a/src/com/android/settings/search2/SearchFragment.java b/src/com/android/settings/search2/SearchFragment.java index fca52e9cb15..1a4e6dcdbd7 100644 --- a/src/com/android/settings/search2/SearchFragment.java +++ b/src/com/android/settings/search2/SearchFragment.java @@ -66,7 +66,7 @@ public class SearchFragment extends InstrumentedFragment implements public void onAttach(Context context) { super.onAttach(context); mSearchFeatureProvider = FeatureFactory.getFactory(context) - .getSearchFeatureProvider(context); + .getSearchFeatureProvider(); } @Override diff --git a/tests/robotests/src/com/android/settings/dashboard/DashboardFeatureProviderImplTest.java b/tests/robotests/src/com/android/settings/dashboard/DashboardFeatureProviderImplTest.java index 3ffd059d75c..e4a988f7e40 100644 --- a/tests/robotests/src/com/android/settings/dashboard/DashboardFeatureProviderImplTest.java +++ b/tests/robotests/src/com/android/settings/dashboard/DashboardFeatureProviderImplTest.java @@ -124,7 +124,7 @@ public class DashboardFeatureProviderImplTest { tile.intent = new Intent(); tile.intent.setComponent(new ComponentName("pkg", "class")); - when(mActivity.getApplicationContext().getSystemService(Context.USER_SERVICE)) + when(mActivity.getSystemService(Context.USER_SERVICE)) .thenReturn(mUserManager); mImpl.bindPreferenceToTile(mActivity, preference, tile, "123", Preference.DEFAULT_ORDER); diff --git a/tests/robotests/src/com/android/settings/search/SearchFeatureProviderImplTest.java b/tests/robotests/src/com/android/settings/search/SearchFeatureProviderImplTest.java index d9e2dd6b195..c0b1b3d7b40 100644 --- a/tests/robotests/src/com/android/settings/search/SearchFeatureProviderImplTest.java +++ b/tests/robotests/src/com/android/settings/search/SearchFeatureProviderImplTest.java @@ -61,7 +61,7 @@ public class SearchFeatureProviderImplTest { public void setUp() { MockitoAnnotations.initMocks(this); mActivity = Robolectric.buildActivity(Activity.class).create().visible().get(); - mProvider = new SearchFeatureProviderImpl(mActivity); + mProvider = new SearchFeatureProviderImpl(); } @Test @@ -78,8 +78,8 @@ public class SearchFeatureProviderImplTest { @Test public void testUpdateIndexNewSearch_UsesDatabaseIndexingManager() { - mProvider = spy(new SearchFeatureProviderImpl(mActivity)); - when(mProvider.isEnabled()).thenReturn(true); + mProvider = spy(new SearchFeatureProviderImpl()); + when(mProvider.isEnabled(mActivity)).thenReturn(true); mProvider.updateIndex(mActivity); verify(mProvider).getIndexingManager(any(Context.class)); @@ -87,8 +87,8 @@ public class SearchFeatureProviderImplTest { @Test public void testUpdateIndexNewSearch_UsesIndex() { - mProvider = spy(new SearchFeatureProviderImpl(mActivity)); - when(mProvider.isEnabled()).thenReturn(false); + mProvider = spy(new SearchFeatureProviderImpl()); + when(mProvider.isEnabled(mActivity)).thenReturn(false); mProvider.updateIndex(mActivity); verify(mProvider, never()).getIndexingManager(any(Context.class)); diff --git a/tests/robotests/src/com/android/settings/testutils/FakeFeatureFactory.java b/tests/robotests/src/com/android/settings/testutils/FakeFeatureFactory.java index a9c1764129a..1625f352223 100644 --- a/tests/robotests/src/com/android/settings/testutils/FakeFeatureFactory.java +++ b/tests/robotests/src/com/android/settings/testutils/FakeFeatureFactory.java @@ -116,7 +116,7 @@ public class FakeFeatureFactory extends FeatureFactory { } @Override - public SearchFeatureProvider getSearchFeatureProvider(Context context) { + public SearchFeatureProvider getSearchFeatureProvider() { return searchFeatureProvider; }