author Jan Henning <>
Tue, 16 Oct 2018 16:17:18 +0000
changeset 492965 defc169a1ee347ab4aabada9da44a859186448c8
parent 486163 78a98c31897acde7302d3250ddc74b1f75598ac4
permissions -rw-r--r--
Bug 1498854 - Rework dismissing of TabHistoryFragment. r=jchen, a=pascalc 1. The patch from bug 1476710 was nonsense and had the same effect as simply deleting that line, because the ChildFragmentManager is only of interest if the TabHistoryFragment loaded further Fragments itself. 2. The issue at hand is that under some circumstances the TabHistoryFragment will be trying to dismiss itself while its responsible FragmentManager is already busy transacting some Fragment state changes. More precisely, the fact that the Fragment is calling popBackStack*Immediately*, which isn't allowed if the FragmentManager is already handling some other transaction. 3. The dismiss() calls in response to the onClick() handlers are unproblematic, because they won't trigger any FragmentManager activity through any route other than the dismiss() call itself. 4. The dismiss() calls in onPause() *are* problematic because the Fragment- Manager will already be busy pausing the TabHistoryFragment, so triggering a further synchronous transaction is not allowed (and originally caused bug 1476710). 5. If the onPause() call happened because some external entity was attempting to remove the fragment (either BrowserApp directly, or indirectly by forwarding a back button press to the FragmentManager), then the Fragment trying to additionally remove itself is unnecessary. 6. If the onPause() call happens because the containing activity itself is being paused, then the Fragment being dismissed is the desired outcome (see bug 1093209), however the Fragment won't be able to remove itself because a) A synchronous transaction is illegal at that point in time. b) An async transaction would be possible, but might not complete until after onSaveInstanceState() had subsequently already been called, which again would be illegal because of state loss. c) An async transaction allowing state loss would succeed in any case, but would mean that if BrowserApp was subsequently destroyed while in back- ground and then later recreated from the savedInstanceState, the Tab- HistoryFragment would be recreated as well, which is undesired. 7. Therefore, the only way to dismiss the TabHistoryFragment when the containing activity is pausing is to synchronously dismiss the Fragment from inside the activity, *before* the onPause() call is forwarded to the FragmentManager. 8. Calling dismiss() in response to onDestroy() is unnecessary, because the Fragment is already irrevocably being removed as soon as we hit onPause(). 9. Because it doesn't make sense to have multiple TabHistoryFragments active at the same time, we also change the logic such that any attempt to show a new TabHistoryFragment will now replace the previous fragment. This is also useful in view of the fact that in order to close the Fragment, BrowserApp retrieves it by calling findFragmentByTag(), which simply returns the first matching Fragment, i.e. wouldn't properly handle things if we ever accidentally ended up with multiple Fragment instances active at the same time. Differential Revision:

/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at */

package org.mozilla.gecko.tabs;

import java.util.ArrayList;
import java.util.List;

import org.mozilla.gecko.EventDispatcher;
import org.mozilla.gecko.GeckoApplication;
import org.mozilla.gecko.R;
import org.mozilla.gecko.util.GeckoBundle;

import android.content.Context;
import android.content.DialogInterface;
import android.os.Bundle;
import android.os.Parcelable;
import android.util.Log;
import android.view.LayoutInflater;
import android.view.View;
import android.view.View.OnClickListener;
import android.view.ViewGroup;
import android.widget.AdapterView;
import android.widget.AdapterView.OnItemClickListener;
import android.widget.ArrayAdapter;
import android.widget.ListView;

public class TabHistoryFragment extends Fragment implements OnItemClickListener, OnClickListener {
    private static final String ARG_LIST = "historyPageList";
    private static final String ARG_INDEX = "index";
    private static final String ARG_PRIVATE_MODE = "private";
    private static final String BACK_STACK_ID = "backStateId";

    private List<TabHistoryPage> historyPageList;
    private int toIndex;
    private ListView dialogList;
    private int backStackId = -1;
    private ViewGroup parent;
    private boolean dismissed;

    public TabHistoryFragment() {


    public static TabHistoryFragment newInstance(List<TabHistoryPage> historyPageList, int toIndex, boolean isPrivate) {
        final TabHistoryFragment fragment = new TabHistoryFragment();
        final Bundle args = new Bundle();
        args.putParcelableArrayList(ARG_LIST, (ArrayList<? extends Parcelable>) historyPageList);
        args.putInt(ARG_INDEX, toIndex);
        args.putBoolean(ARG_PRIVATE_MODE, isPrivate);
        return fragment;

    public void onCreate(Bundle savedInstanceState) {
        if (savedInstanceState != null) {
            backStackId = savedInstanceState.getInt(BACK_STACK_ID, -1);

    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
        this.parent = container;
        View view = inflater.inflate(R.layout.tab_history_layout, container, false);
        dialogList = (ListView) view.findViewById(;
        return view;

    public void onActivityCreated(Bundle savedInstanceState) {
        Bundle bundle = getArguments();
        historyPageList = bundle.getParcelableArrayList(ARG_LIST);
        toIndex = bundle.getInt(ARG_INDEX);
        boolean isPrivate = bundle.getBoolean(ARG_PRIVATE_MODE);
        final ArrayAdapter<TabHistoryPage> urlAdapter = new TabHistoryAdapter(getActivity(), historyPageList, isPrivate);

    public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
        final GeckoBundle data = new GeckoBundle(1);
        data.putInt("index", toIndex - position);
        EventDispatcher.getInstance().dispatch("Session:Navigate", data);

    public void onClick(View v) {
        // Since the fragment view fills the entire screen, any clicks outside of the history
        // ListView will end up here.

    public void onPause() {

    public void onDestroy() {

        GeckoApplication.watchReference(getActivity(), this);

    public void onSaveInstanceState(Bundle outState) {
        if (backStackId >= 0) {
            outState.putInt(BACK_STACK_ID, backStackId);

    // Function to add this fragment to activity state with containerViewId as parent.
    // This similar in functionality to except that containerId is provided here.
    public void show(final int containerViewId, final FragmentTransaction transaction, final String tag) {
        dismissed = false;
        transaction.replace(containerViewId, this, tag);
        // Populating the tab history requires a gecko call (which can be slow) - therefore the app
        // state by the time we try to show this fragment is unknown, and we could be in the
        // middle of shutting down:
        backStackId = transaction.commitAllowingStateLoss();

    // Pop the fragment from backstack if it exists.
    public void dismiss() {
        if (backStackId >= 0) {
            getFragmentManager().popBackStackImmediate(backStackId, FragmentManager.POP_BACK_STACK_INCLUSIVE);
            backStackId = -1;

    private void onDismiss() {
        if (dismissed) {

        dismissed = true;

        if (parent != null) {

    private static class TabHistoryAdapter extends ArrayAdapter<TabHistoryPage> {
        private final List<TabHistoryPage> pages;
        private final Context context;
        private final boolean isPrivate;

        public TabHistoryAdapter(Context context, List<TabHistoryPage> pages, boolean isPrivate) {
            super(context, R.layout.tab_history_item_row, pages);
            this.context = context;
            this.pages = pages;
            this.isPrivate = isPrivate;

        public View getView(int position, View convertView, ViewGroup parent) {
            TabHistoryItemRow row = (TabHistoryItemRow) convertView;
            if (row == null) {
                row = new TabHistoryItemRow(context, null);

            row.update(pages.get(position), position == 0, position == pages.size() - 1, isPrivate);
            return row;