Skip to content
This repository has been archived by the owner on Mar 16, 2021. It is now read-only.

Bugfix: Presenter not destroyed when Fragment is popped off the backstack #78

Merged
merged 83 commits into from
Apr 25, 2017
Merged
Show file tree
Hide file tree
Changes from 62 commits
Commits
Show all changes
83 commits
Select commit Hold shift + click to select a range
f0b5a3d
Adds a TiFragmentDelegateBuilder.
Mar 10, 2017
d482f01
Changes the visibility of the SAVED_PRESENTER_ID class variable in th…
Mar 12, 2017
b6e0f67
Adds a TiFragmentPresenterDestroyTest class with a first test impleme…
Mar 12, 2017
34b803e
Merge branch 'master' into bugfix/fragment_backstack_presenter_not_de…
Mar 15, 2017
c41603c
Inject savior with fragment delegate builder
Mar 15, 2017
81323af
Adds more tests to the TiFragmentPresenterDestroyTest class.
Mar 16, 2017
0f87776
Refactors method names in the TiFragmentDelegateBuilder class.
Mar 16, 2017
b1c9b70
Changes the back stack test case.
Mar 16, 2017
788f4c4
Adds a fragment lifecycle test to the sample application.
Mar 17, 2017
edf1db7
Logs delegate method callbacks in the TestFragment.
Mar 17, 2017
a7cac19
Changes tests according to the real Fragment lifecycle.
Mar 20, 2017
92b3c68
Adds more tests to the TiFragmentPresenterDestroyTest.
Mar 20, 2017
89c7a25
Change name of sample app
Mar 20, 2017
c957464
Add all test cases stubs
Mar 20, 2017
ccbb6c6
Moves the basic Bundle initialization into the setUp() method of the …
Mar 20, 2017
831fc34
Logs if onCreate_afterSuper should be called with null or with a non …
Mar 20, 2017
0758938
Adds an option to retain a fragment instance in the FragmentLifecycle…
Mar 20, 2017
c9557c0
Improves logging in the TestFragment.
Mar 20, 2017
04179dc
Implements all tests for the combination savior = false and retain = …
Mar 20, 2017
4ff89db
Fixes false test implementations.
Mar 20, 2017
803b7b3
Add HostingActivity which can change it’s state of isChangingConfigur…
Mar 20, 2017
1f32468
Fixes test cases so they only test the scope that is implied by the t…
Mar 22, 2017
1a8b791
Adds more test case implementations.
Mar 22, 2017
c5f831d
Adds missing test implementations.
Mar 22, 2017
2497ca7
Improve logging
Mar 22, 2017
5dda802
Splits the tests into multiple files.
Mar 22, 2017
fd16f88
Adds more missing test cases.
Mar 22, 2017
05f2541
Adds more missing test cases.
Mar 22, 2017
838ab48
Implement equals correctly for the configuration
Mar 22, 2017
ea700d8
Add more logging
Mar 22, 2017
8c0df9c
Verified and improved the single fragment default cases
Mar 22, 2017
6e3476e
Verify retain false tests with savior true for single fragments
Mar 22, 2017
ac09f78
Improve logging
Mar 22, 2017
1821b6d
Verify dontKeepActivitiesFalse_activityFinishing tests
Mar 22, 2017
0c1c340
Verify saviorFalse_retainFalse_dontKeepActivitiesFalse_activityChangi…
Mar 22, 2017
423d886
Adjust comments and rename HostingActivity reset function
Mar 22, 2017
ec3d582
Verify first dontKeepActivitiesFalse cases
Mar 22, 2017
f82dc45
Finish verifying Single Fragment cases
Mar 22, 2017
b5dd9e7
Add moveToBackground_moveToForeground tests
Mar 22, 2017
81a81be
Add more move to background -> foreground cases
Mar 23, 2017
7efab6b
Add more move to background -> foreground cases
Mar 23, 2017
549bce8
Finalize moveToBackground -> moveToForeground tests
Mar 23, 2017
ac569f5
Mark default cases for muti fragment tests
Mar 23, 2017
95958ff
Rename “Default case” to “Default config”
Mar 23, 2017
8c4d7a2
Changes code formatting.
Mar 24, 2017
65f601b
Allow removing FragmentA
Apr 13, 2017
8e2de39
Move don’t keep Activities cases to a sparate file
Apr 13, 2017
49ffd78
Merge branch 'master' into bugfix/fragment_backstack_presenter_not_de…
Apr 16, 2017
7bf7910
Remove verification comments because all test have to be rechecked an…
Apr 16, 2017
0ac8f79
Future TiFragments use the savior or don’t retain. savior false but r…
Apr 16, 2017
178f9f3
Allow set Fragment#isRemoving in tests
Apr 17, 2017
5daf108
Add don’t keep activities tests
Apr 17, 2017
e01d246
Remove savior=false tests, savior false is deprecated
Apr 17, 2017
88f2985
Finish all current fragment tests
Apr 18, 2017
bfc8a2d
Also remove static savior setting in Activity
Apr 18, 2017
23c41a5
Observe activity scopes and destory presenters accordingly
Apr 18, 2017
00dc083
Make PresenterSavior testable
Apr 18, 2017
a758265
Fix recursive call
Apr 18, 2017
4e71f20
Remove deprecated android tests, replaced with far better jvm tests
Apr 18, 2017
7ab1143
Harden providePresenter() to only return INITIALIZED state instances
Apr 19, 2017
6dfa2ca
Print the current presenter state when failing
Apr 19, 2017
b5a0281
Create a new Presenter when the Presenter is destroyed
Apr 19, 2017
b767f63
Extract ActivityLifecycleCallbacks to a separate class
Apr 21, 2017
933e215
Move methods for testing to a TestPresenterSavior class
Apr 21, 2017
f10f606
Make fragment presenter retain adjustable
Apr 23, 2017
096444e
Add mulitple variants for the previous presenter leaking case
Apr 23, 2017
a8b6c61
Test to reuse of fragments
Apr 23, 2017
52e02c6
Remove todo which is now done
Apr 23, 2017
96c5f93
Cleanup logging
Apr 23, 2017
f49152a
Remove all deprecated usages of setUseStaticSaviorToRetain()
Apr 23, 2017
e46621e
Add documentation
Apr 23, 2017
3fac777
Don’t allow the usage of setRetainInstance(true)
Apr 23, 2017
a6dbf0d
Simplify destroy logic in TiFragment and TiActivity
Apr 24, 2017
c3997c8
Rename isInBackstack -> isFragmentInBackstack
Apr 24, 2017
191bb5d
Improve providePresenter documentation
Apr 24, 2017
6d67cbf
Remove unused class (was moved to sample)
Apr 25, 2017
ab8f873
Review improvements
Apr 25, 2017
9cd36cb
Add detach/attach feature to sample
Apr 25, 2017
43aa76a
Don’t print a hint as warning
Apr 25, 2017
a89f8de
Use same visibility for Listener as the using class
Apr 25, 2017
cf4d1d4
Add sample/test for viewpager
StefMa Apr 25, 2017
143a378
Better starting of Activities
StefMa Apr 25, 2017
3880e06
Merge pull request #84 from StefMa/bugfix/fragment_backstack_presente…
passsy Apr 25, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public class TiActivityPlugin<P extends TiPresenter<V>, V extends TiView> extend
*/
public TiActivityPlugin(@NonNull final TiPresenterProvider<P> presenterProvider) {
mDelegate = new TiActivityDelegate<>(this, this, presenterProvider, this,
PresenterSavior.INSTANCE);
PresenterSavior.getInstance());
}

@NonNull
Expand Down Expand Up @@ -156,6 +156,11 @@ public final boolean isDontKeepActivitiesEnabled() {
return AndroidDeveloperOptions.isDontKeepActivitiesEnabled(getActivity());
}

@Override
public Activity getHostingActivity() {
return getActivity();
}

@CallSuper
@Override
public void onConfigurationChanged(final Configuration newConfig) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@
import net.grandcentrix.thirtyinch.util.AndroidDeveloperOptions;
import net.grandcentrix.thirtyinch.util.AnnotationUtil;

import android.app.Activity;
import android.os.Bundle;
import android.support.annotation.CallSuper;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.v4.app.BackstackReader;
import android.support.v4.app.Fragment;
import android.view.LayoutInflater;
import android.view.View;
Expand Down Expand Up @@ -78,7 +80,7 @@ public class TiFragmentPlugin<P extends TiPresenter<V>, V extends TiView> extend
*/
public TiFragmentPlugin(@NonNull final TiPresenterProvider<P> presenterProvider) {
mDelegate = new TiFragmentDelegate<>(this, this, presenterProvider, this,
PresenterSavior.INSTANCE);
PresenterSavior.getInstance());
}

@NonNull
Expand All @@ -87,12 +89,17 @@ public final Removable addBindViewInterceptor(@NonNull final BindViewInterceptor
return mDelegate.addBindViewInterceptor(interceptor);
}

@Override
public Activity getHostingActivity() {
return getFragment().getActivity();
}

/**
* @return the cached result of {@link BindViewInterceptor#intercept(TiView)}
*/
@Nullable
@Override
public final V getInterceptedViewOf(@NonNull final BindViewInterceptor interceptor) {
public final V getInterceptedViewOf(@NonNull final BindViewInterceptor interceptor) {
return mDelegate.getInterceptedViewOf(interceptor);
}

Expand Down Expand Up @@ -146,6 +153,11 @@ public final boolean isFragmentDetached() {
return getFragment().isDetached();
}

@Override
public boolean isFragmentRemoving() {
return getFragment().isRemoving();
}

@Override
public final boolean isHostingActivityChangingConfigurations() {
return getFragment().getActivity().isChangingConfigurations();
Expand All @@ -156,6 +168,11 @@ public final boolean isHostingActivityFinishing() {
return getFragment().getActivity().isFinishing();
}

@Override
public boolean isInBackstack() {
return BackstackReader.isInBackStack(getFragment());
}

@CallSuper
@Override
public void onCreate(final Bundle savedInstanceState) {
Expand Down Expand Up @@ -236,11 +253,6 @@ public V provideView() {
}
}

@Override
public final void setFragmentRetainInstance(final boolean retain) {
getFragment().setRetainInstance(retain);
}

@Override
public String toString() {
String presenter = mDelegate.getPresenter() == null ? "null" :
Expand Down
2 changes: 2 additions & 0 deletions sample/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
</intent-filter>
</activity>

<activity android:name=".fragmentlifecycle.FragmentLifecycleActivity" />

</application>

</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,13 @@
import com.jakewharton.rxbinding.view.RxView;

import net.grandcentrix.thirtyinch.TiActivity;
import net.grandcentrix.thirtyinch.sample.fragmentlifecycle.FragmentLifecycleActivity;

import android.content.Intent;
import android.os.Bundle;
import android.support.annotation.NonNull;
import android.view.Menu;
import android.view.MenuItem;
import android.view.View;
import android.widget.Button;
import android.widget.TextView;
Expand All @@ -42,6 +46,12 @@ public Observable<Void> onButtonClicked() {
return RxView.clicks(mButton);
}

@Override
public boolean onCreateOptionsMenu(final Menu menu) {
getMenuInflater().inflate(R.menu.menu_hello_world, menu);
return true;
}

@NonNull
@Override
public HelloWorldPresenter providePresenter() {
Expand All @@ -58,6 +68,11 @@ public void showText(final String text) {
mOutput.setText(text);
}

public void startFragmentLifecycleTest(MenuItem item) {
final Intent intent = new Intent(this, FragmentLifecycleActivity.class);
startActivity(intent);
}

@Override
protected void onCreate(final Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
package net.grandcentrix.thirtyinch.sample.fragmentlifecycle;

import net.grandcentrix.thirtyinch.sample.R;

import android.os.Bundle;
import android.provider.Settings;
import android.support.v4.app.Fragment;
import android.support.v4.app.FragmentManager;
import android.support.v4.app.FragmentTransaction;
import android.support.v7.app.AppCompatActivity;
import android.support.v7.widget.SwitchCompat;
import android.util.Log;
import android.view.View;
import android.widget.TextView;

import static android.provider.Settings.Global.ALWAYS_FINISH_ACTIVITIES;
import static net.grandcentrix.thirtyinch.sample.fragmentlifecycle.TestFragment.testFragmentInstanceCount;

public class FragmentLifecycleActivity extends AppCompatActivity {

static int fragmentLifecycleActivityInstanceCount = -1;

private final String TAG = this.getClass().getSimpleName()
+ "@" + Integer.toHexString(this.hashCode());

private SwitchCompat mSwitchAddToBackStack;

private SwitchCompat mSwitchRetainFragmentInstance;

public void addFragmentA(View view) {
final TestFragmentA fragment = new TestFragmentA();
Log.v(TAG, "adding FragmentA");
addFragment(fragment);
}

public void addFragmentB(View view) {
final TestFragmentB fragment = new TestFragmentB();
Log.v(TAG, "adding FragmentB");
addFragment(fragment);
}

@Override
public void finish() {
super.finish();
Log.v(TAG, "// When the Activity finishes");
}

public void finishActivity(View view) {
finish();
}

@Override
public void onBackPressed() {
Log.v(TAG, "// When the back button gets pressed");
Log.v(TAG, "// When the top most fragment gets popped");
final FragmentManager fragmentManager = getSupportFragmentManager();
if (fragmentManager.getBackStackEntryCount() > 0) {
fragmentManager.popBackStack();
} else {
super.onBackPressed();
}
}

public void recreateActivity(View view) {
Log.v(TAG, "// And when the Activity is changing its configurations.");
recreate();
}

public void removeFragmentA(View view) {
final Fragment fragment = getSupportFragmentManager()
.findFragmentById(R.id.fragment_placeholder);
if (fragment instanceof TestFragmentA) {
getSupportFragmentManager().beginTransaction()
.remove(fragment)
.commitNow();
}
}

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
if (savedInstanceState == null) {
//started for the first time, reset all counters
fragmentLifecycleActivityInstanceCount = -1;
TestFragment.testFragmentInstanceCount = -1;
}

fragmentLifecycleActivityInstanceCount++;
setContentView(R.layout.activity_fragment_lifecycle);
FragmentManager.enableDebugLogging(true);
Log.v(TAG, "onCreate of " + this);

mSwitchAddToBackStack = (SwitchCompat) findViewById(R.id.switch_add_back_stack);
mSwitchRetainFragmentInstance = (SwitchCompat) findViewById(
R.id.switch_retain_fragment_instance);
final TextView textDontKeepActivities = (TextView) findViewById(
R.id.text_dont_keep_activities);
textDontKeepActivities.setText(
isDontKeepActivities() ? R.string.dont_keep_activities_enabled
: R.string.dont_keep_activities_disabled);

Log.v(TAG, "// A new Activity gets created by the Android Framework.");
Log.v(TAG, "final HostingActivity hostingActivity" + fragmentLifecycleActivityInstanceCount
+ " = new HostingActivity();");
}

@Override
protected void onDestroy() {
super.onDestroy();

Log.v(TAG, "onDestroy of " + this);

Log.v(TAG, "hostingActivity" + fragmentLifecycleActivityInstanceCount
+ ".setChangingConfiguration(" + isChangingConfigurations() + ");");
Log.v(TAG, "hostingActivity" + fragmentLifecycleActivityInstanceCount
+ ".setFinishing(" + isFinishing() + ");");
Log.v(TAG, "// hostingActivity" + fragmentLifecycleActivityInstanceCount
+ " got destroyed.");
}

@Override
protected void onPause() {
super.onPause();
Log.d(TAG, "onPause()");
Log.v(TAG, "hostingActivity" + fragmentLifecycleActivityInstanceCount + ""
+ ".setChangingConfiguration(" + isChangingConfigurations() + ");");
Log.v(TAG, "hostingActivity" + fragmentLifecycleActivityInstanceCount + ""
+ ".setFinishing(" + isFinishing() + ");");
}

@Override
protected void onSaveInstanceState(final Bundle outState) {
super.onSaveInstanceState(outState);
Log.d(TAG, "onSaveInstanceState(Bundle)");
Log.v(TAG, "hostingActivity" + fragmentLifecycleActivityInstanceCount + ""
+ ".setChangingConfiguration(" + isChangingConfigurations() + ");");
Log.v(TAG, "hostingActivity" + fragmentLifecycleActivityInstanceCount + ""
+ ".setFinishing(" + isFinishing() + ");");
}

private void addFragment(final Fragment fragment) {
final FragmentManager fragmentManager = getSupportFragmentManager();
final FragmentTransaction fragmentTransaction = fragmentManager.beginTransaction();
if (isRetainFragmentInstance()) {
Log.v(TAG, "retaining fragment instance");
fragment.setRetainInstance(true);
}
fragmentTransaction.replace(R.id.fragment_placeholder, fragment);
if (isAddToBackStack()) {
Log.v(TAG, "adding transaction to the back stack");
fragmentTransaction.addToBackStack(null);
}
final int backStackId = fragmentTransaction.commit();
Log.v(TAG, "\n// Given a Presenter ...");
// (testFragmentInstanceCount + 1) because it will be created after executing this code
Log.v(TAG, "final TestPresenter presenter" + (testFragmentInstanceCount + 1) + " ="
+ " new TestPresenter(new TiConfiguration.Builder()\n"
+ " .setUseStaticSaviorToRetain(/*TODO set*/)\n"
+ " .setRetainPresenterEnabled(" + isRetainFragmentInstance() + ")\n"
+ " .build());");

Log.v(TAG, "\n// And given a Fragment.");
Log.v(TAG, "final TiFragmentDelegate<TiPresenter<TiView>, TiView> "
+ "delegate" + (testFragmentInstanceCount + 1) + "\n"
+ " = new TiFragmentDelegateBuilder()\n"
+ " .setDontKeepActivitiesEnabled(" + isDontKeepActivities() + ")\n"
+ " .setHostingActivity(hostingActivity)\n"
+ " .setSavior(mSavior)\n"
+ " .setPresenter(presenter" + (testFragmentInstanceCount + 1)
+ ")\n"
+ " .build();");

if (backStackId >= 0) {
Log.v(TAG, "Back stack ID: " + String.valueOf(backStackId));
}
}

private boolean isAddToBackStack() {
return mSwitchAddToBackStack.isChecked();
}

private boolean isDontKeepActivities() {
// default behaviour
int dontKeepActivities = 0;
try {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use your util here now. Otherwise AndroidDeveloperOptions is not used and can removed

dontKeepActivities = Settings.Global
.getInt(getContentResolver(), ALWAYS_FINISH_ACTIVITIES);
} catch (Settings.SettingNotFoundException e) {
e.printStackTrace();
}
return dontKeepActivities != 0;
}

private boolean isRetainFragmentInstance() {
return mSwitchRetainFragmentInstance.isChecked();
}
}
Loading