Skip to content

Commit

Permalink
Fix fragment added crash on Android native stack (#399)
Browse files Browse the repository at this point in the history
Thi change fixes "fragment already added crash". The crash was caused by us calling onUpdate twice without fragment manager transaction queue getting flushed in between. This could've happened because in dismiss method we'd directly call onUpdate instead of going through markUpdated mechanism that enqueues onUpdate calls using choreographer and threfore guarantees that the method is called only once per frame. On top of changing dismiss behavior we also added call to flush fragment manager transactions queue in case there are some enqueued transactions at the point when onUpdate is called.
  • Loading branch information
kmagiera authored Feb 27, 2020
1 parent c4f31b0 commit bef5803
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,22 @@ private void updateIfNeeded() {
onUpdate();
}

protected void onUpdate() {
private final void onUpdate() {
if (mFragmentManager != null) {
// We double check if fragment manager have any pending transactions to run.
// In performUpdate we often check whether some fragments are added to
// manager to avoid adding them for the second time (which result in crash).
// By design performUpdate should be called at most once per frame, so this
// should never happen, but in case there are some pending transaction we
// need to flush them here such that Fragment#isAdded checks reflect the
// reality and that we don't have enqueued fragment add commands that will
// execute shortly and cause "Fragment already added" crash.
mFragmentManager.executePendingTransactions();
}
performUpdate();
}

protected void performUpdate() {
// detach screens that are no longer active
Set<Fragment> orphaned = new HashSet<>(mFragmentManager.getFragments());
for (int i = 0, size = mScreenFragments.size(); i < size; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public ScreenStack(Context context) {

public void dismiss(ScreenStackFragment screenFragment) {
mDismissed.add(screenFragment);
onUpdate();
markUpdated();
}

public Screen getTopScreen() {
Expand Down Expand Up @@ -142,7 +142,7 @@ protected boolean hasScreen(ScreenFragment screenFragment) {
}

@Override
protected void onUpdate() {
protected void performUpdate() {
// remove all screens previously on stack
for (ScreenStackFragment screen : mStack) {
if (!mScreenFragments.contains(screen) || mDismissed.contains(screen)) {
Expand Down

0 comments on commit bef5803

Please sign in to comment.