drgreen/android_frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Don't hold old host callback for fragments on the back stack

Browse Source 
LoaderManagers configure their host callback lazily as their
associated fragment is brought up through its lifecycle states. In the
case of fragments on the fragment back stack this could happen very
late, if at all. As a LoaderManager's host callback references the
host Activity, this means that a LoaderManager could keep a destroyed
Activity reference alive.

Update the host callbacks of all LoaderManagers eagerly during the
restore non-configuration instance phase.

Bug: 30653222
Test: core/tests/coretests/src/android/app/LoaderLifecycleTest.java
Change-Id: I5d2b81daae5e7cae429fcf4934e64b3ce281140c
This commit is contained in:
Adam Powell
2016-09-21 15:41:47 -07:00
parent 56a70e3865
commit 180202f221
5 changed files with 258 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
core/java/android/app/FragmentHostCallback.java
View File

@ -340,6 +340,9 @@ public abstract class FragmentHostCallback<E> extends FragmentContainer {
}
void restoreLoaderNonConfig(ArrayMap<String, LoaderManager> loaderManagers) {
for (int i = 0, N = loaderManagers.size(); i < N; i++) {
((LoaderManagerImpl) loaderManagers.valueAt(i)).updateHostController(this);
}
mAllLoaderManagers = loaderManagers;
}

7
core/java/android/app/LoaderManager.java
View File

@ -195,6 +195,9 @@ public abstract class LoaderManager {
public static void enableDebugLogging(boolean enabled) {
LoaderManagerImpl.DEBUG = enabled;
}
/** @hide for internal testing only */
public FragmentHostCallback getFragmentHostCallback() { return null; }
}
class LoaderManagerImpl extends LoaderManager {
@ -542,6 +545,10 @@ class LoaderManagerImpl extends LoaderManager {
void updateHostController(FragmentHostCallback host) {
mHost = host;
}
public FragmentHostCallback getFragmentHostCallback() {
return mHost;
}
private LoaderInfo createLoader(int id, Bundle args,
LoaderManager.LoaderCallbacks<Object> callback) {

2
core/tests/coretests/AndroidManifest.xml
View File

@ -1147,6 +1147,8 @@
</activity>
<activity android:name="com.android.internal.policy.PhoneWindowActionModeTestActivity">
</activity>
<activity android:name="android.app.EmptyActivity">
</activity>
<receiver android:name="android.app.activity.AbortReceiver">
<intent-filter android:priority="1">

21
core/tests/coretests/src/android/app/EmptyActivity.java Normal file
View File

@ -0,0 +1,21 @@
/*
* Copyright (C) 2016 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 android.app;
public class EmptyActivity extends Activity {
}

225
core/tests/coretests/src/android/app/LoaderLifecycleTest.java Normal file
View File

@ -0,0 +1,225 @@
/*
* Copyright (C) 2016 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 android.app;
import android.content.Context;
import android.os.Handler;
import android.os.Parcelable;
import android.support.test.filters.MediumTest;
import android.support.test.rule.ActivityTestRule;
import android.support.test.runner.AndroidJUnit4;
import android.util.ArrayMap;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import static junit.framework.TestCase.assertNotNull;
import static junit.framework.TestCase.assertNotSame;
import static junit.framework.TestCase.assertSame;
@RunWith(AndroidJUnit4.class)
public class LoaderLifecycleTest {
@Rule
public ActivityTestRule<EmptyActivity> mActivityRule =
new ActivityTestRule<>(EmptyActivity.class);
@Test
@MediumTest
public void loaderIdentityTest() throws Throwable{
mActivityRule.runOnUiThread(() -> {
final Handler h = new Handler();
final FragmentController fc1 = FragmentController.createController(
new TestFragmentHostCallback(mActivityRule.getActivity(), h, 0));
fc1.attachHost(null);
fc1.dispatchCreate();
final FragmentManager fm1 = fc1.getFragmentManager();
final Fragment f1 = new Fragment();
fm1.beginTransaction().add(f1, "one").commitNow();
// Removing and re-adding a fragment completely will destroy its LoaderManager.
// Keep the first one here to confirm this later.
final LoaderManager lm1 = f1.getLoaderManager();
// Remove the fragment, add a second one, and re-add the first to
// force its internal index to change. The tests below should still remain consistent.
final Fragment f2 = new Fragment();
fm1.beginTransaction().remove(f1).commitNow();
fm1.beginTransaction().add(f2, "two").commitNow();
fm1.beginTransaction().add(f1, "one").commitNow();
// We'll check this to see if we get the same instance back later
// as passed through NonConfigurationInstance. If the keys stay consistent
// across fragment remove/re-add, this will be consistent.
final LoaderManager lm12 = f1.getLoaderManager();
assertNotSame("fully removed and re-added fragment got same LoaderManager", lm1, lm12);
fc1.dispatchActivityCreated();
fc1.noteStateNotSaved();
fc1.execPendingActions();
fc1.doLoaderStart();
fc1.dispatchStart();
fc1.reportLoaderStart();
fc1.dispatchResume();
fc1.execPendingActions();
// Bring the state back down to destroyed, simulating an activity restart
fc1.dispatchPause();
final Parcelable savedState = fc1.saveAllState();
fc1.doLoaderStop(true);
fc1.dispatchStop();
final FragmentManagerNonConfig nonconf = fc1.retainNestedNonConfig();
final ArrayMap<String, LoaderManager> loaderNonConfig = fc1.retainLoaderNonConfig();
assertNotNull("loaderNonConfig was null", loaderNonConfig);
fc1.dispatchDestroy();
// Create the new controller and restore state
final FragmentController fc2 = FragmentController.createController(
new TestFragmentHostCallback(mActivityRule.getActivity(), h, 0));
final FragmentManager fm2 = fc2.getFragmentManager();
fc2.attachHost(null);
fc2.restoreLoaderNonConfig(loaderNonConfig);
fc2.restoreAllState(savedState, nonconf);
fc2.dispatchCreate();
fc2.dispatchActivityCreated();
fc2.noteStateNotSaved();
fc2.execPendingActions();
fc2.doLoaderStart();
fc2.dispatchStart();
fc2.reportLoaderStart();
fc2.dispatchResume();
fc2.execPendingActions();
// Test that the fragments are in the configuration we expect
final Fragment restoredOne = fm2.findFragmentByTag("one");
final LoaderManager lm2 = restoredOne.getLoaderManager();
assertSame("didn't get same LoaderManager instance back", lm2, lm12);
// Bring the state back down to destroyed before we finish the test
fc2.dispatchPause();
fc2.saveAllState();
fc2.dispatchStop();
fc2.dispatchDestroy();
});
}
@Test
@MediumTest
public void backStackLoaderIdentityTest() throws Throwable{
mActivityRule.runOnUiThread(() -> {
final Handler h = new Handler();
final FragmentHostCallback host1 =
new TestFragmentHostCallback(mActivityRule.getActivity(), h, 0);
final FragmentController fc1 = FragmentController.createController(host1);
fc1.attachHost(null);
fc1.dispatchCreate();
final FragmentManager fm1 = fc1.getFragmentManager();
final Fragment f1 = new Fragment();
fm1.beginTransaction().add(f1, "one").commitNow();
final LoaderManager lm1 = f1.getLoaderManager();
// Put the fragment on the back stack.
fm1.beginTransaction().remove(f1).addToBackStack("backentry").commit();
fm1.executePendingTransactions();
fc1.dispatchActivityCreated();
fc1.noteStateNotSaved();
fc1.execPendingActions();
fc1.doLoaderStart();
fc1.dispatchStart();
fc1.reportLoaderStart();
fc1.dispatchResume();
fc1.execPendingActions();
// Bring the state back down to destroyed, simulating an activity restart
fc1.dispatchPause();
final Parcelable savedState = fc1.saveAllState();
fc1.doLoaderStop(true);
fc1.dispatchStop();
final FragmentManagerNonConfig nonconf = fc1.retainNestedNonConfig();
final ArrayMap<String, LoaderManager> loaderNonConfig = fc1.retainLoaderNonConfig();
assertNotNull("loaderNonConfig was null", loaderNonConfig);
fc1.dispatchDestroy();
// Create the new controller and restore state
final FragmentHostCallback host2 =
new TestFragmentHostCallback(mActivityRule.getActivity(), h, 0);
final FragmentController fc2 = FragmentController.createController(host2);
final FragmentManager fm2 = fc2.getFragmentManager();
fc2.attachHost(null);
fc2.restoreLoaderNonConfig(loaderNonConfig);
fc2.restoreAllState(savedState, nonconf);
fc2.dispatchCreate();
fc2.dispatchActivityCreated();
fc2.noteStateNotSaved();
fc2.execPendingActions();
fc2.doLoaderStart();
fc2.dispatchStart();
fc2.reportLoaderStart();
fc2.dispatchResume();
fc2.execPendingActions();
assertNotSame("LoaderManager kept reference to old FragmentHostCallback",
host1, lm1.getFragmentHostCallback());
assertSame("LoaderManager did not refrence new FragmentHostCallback",
host2, lm1.getFragmentHostCallback());
// Test that the fragments are in the configuration we expect
final Fragment restoredOne = fm2.findFragmentByTag("one");
final LoaderManager lm2 = restoredOne.getLoaderManager();
assertSame("didn't get same LoaderManager instance back", lm2, lm1);
// Bring the state back down to destroyed before we finish the test
fc2.dispatchPause();
fc2.saveAllState();
fc2.dispatchStop();
fc2.dispatchDestroy();
});
}
public class TestFragmentHostCallback extends FragmentHostCallback<LoaderLifecycleTest> {
public TestFragmentHostCallback(Context context, Handler handler, int windowAnimations) {
super(context, handler, windowAnimations);
}
@Override
public LoaderLifecycleTest onGetHost() {
return LoaderLifecycleTest.this;
}
}
}