Bug 1129168 - Avoid fragment load flicker by explicilty managing back-stack. r=nalexander
authorvivek <vivekb.balakrishnan@gmail.com>
Thu, 05 Mar 2015 14:12:00 -0800
changeset 249023 ff97bae1edfcfa1636223d9fff681110ca8b1723
parent 249022 ef482a54a912a9804f1326858d7e23de3a5ff6c6
child 249024 d0e907163a24a898dafe8f8ed6b484a72e76c49d
push id966
push usermleibovic@mozilla.com
push dateTue, 10 Mar 2015 01:36:33 +0000
reviewersnalexander
bugs1129168
milestone39.0a1
Bug 1129168 - Avoid fragment load flicker by explicilty managing back-stack. r=nalexander
mobile/android/base/home/RemoteTabsPanel.java
--- a/mobile/android/base/home/RemoteTabsPanel.java
+++ b/mobile/android/base/home/RemoteTabsPanel.java
@@ -1,34 +1,32 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
  * 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 http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.home;
 
-import java.util.EnumMap;
-import java.util.Map;
-
 import org.mozilla.gecko.GeckoScreenOrientation;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.fxa.AccountLoader;
 import org.mozilla.gecko.fxa.FirefoxAccounts;
 import org.mozilla.gecko.fxa.FxAccountConstants;
 import org.mozilla.gecko.fxa.login.State;
 import org.mozilla.gecko.fxa.login.State.Action;
 import org.mozilla.gecko.sync.SyncConstants;
 import org.mozilla.gecko.util.HardwareUtils;
 
 import android.accounts.Account;
 import android.content.res.Configuration;
 import android.os.Bundle;
 import android.support.v4.app.Fragment;
 import android.support.v4.app.LoaderManager.LoaderCallbacks;
 import android.support.v4.content.Loader;
+import android.support.v4.util.Pair;
 import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
 
 /**
  * A <code>HomeFragment</code> that, depending on the state of accounts on the
  * device:
@@ -38,85 +36,104 @@ import android.view.ViewGroup;
  * <li>offers to create a new Firefox Account.</li>
  * </ul>
  */
 public class RemoteTabsPanel extends HomeFragment {
     private static final String LOGTAG = "GeckoRemoteTabsPanel";
 
     // Loader ID for Android Account loader.
     private static final int LOADER_ID_ACCOUNT = 0;
+    private static final String FRAGMENT_ACTION = "FRAGMENT_ACTION";
+    private static final String FRAGMENT_ORIENTATION = "FRAGMENT_ORIENTATION";
+    private static final String FRAGMENT_TAG = "FRAGMENT_TAG";
+    private static final String NO_ACCOUNT = "NO_ACCOUNT";
 
     // Callback for loaders.
     private AccountLoaderCallbacks mAccountLoaderCallbacks;
 
-    // The current fragment being shown to reflect the system account state. We
-    // don't want to detach and re-attach panels unnecessarily, because that
-    // causes flickering.
-    private Fragment mCurrentFragment;
-
-    // A lazily-populated cache of fragments corresponding to the possible
-    // system account states. We don't want to re-create panels unnecessarily,
-    // because that can cause flickering. `null` is not a valid key.
-    private final Map<Action, Fragment> mFragmentCache = new EnumMap<>(Action.class);
-
-    // The fragment that corresponds to the null action -- "no Account,
-    // neither Firefox nor Legacy Sync."
-    // Lazily populated.
-    private Fragment mFallbackFragment;
-
     @Override
     public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
         return inflater.inflate(R.layout.home_remote_tabs_panel, container, false);
     }
 
     @Override
     public void onActivityCreated(Bundle savedInstanceState) {
         super.onActivityCreated(savedInstanceState);
 
         // Create callbacks before the initial loader is started.
         mAccountLoaderCallbacks = new AccountLoaderCallbacks();
         loadIfVisible();
     }
 
     @Override
     protected void loadIfVisible() {
-        // Force load the child fragment if fragment is displayed in Tablet with a valid account.
-        if (canLoad() && HardwareUtils.isTablet() && (mCurrentFragment instanceof RemoteTabsBaseFragment)) {
-            load();
-            return;
+        // Force reload fragment only in tablets when there is valid account and the orientation has changed.
+        Pair<String, Integer> actionOrientationPair;
+        if (canLoad() && HardwareUtils.isTablet() && (actionOrientationPair = getActionAndOrientationForFragmentInBackStack()) != null) {
+            if (actionOrientationPair.first.equals(Action.None.name()) && actionOrientationPair.second != GeckoScreenOrientation.getInstance().getAndroidOrientation()) {
+                // As the fragment becomes visible only after onStart callback, we can safely remove it from the back-stack.
+                // If a portrait fragment is in the back-stack and then a landscape fragment should be shown, there can
+                // be a brief flash as the fragment as replaced.
+                getChildFragmentManager()
+                        .beginTransaction()
+                        .addToBackStack(null)
+                        .remove(getChildFragmentManager().findFragmentByTag(FRAGMENT_TAG))
+                        .commitAllowingStateLoss();
+                getChildFragmentManager().executePendingTransactions();
+
+                load();
+                return;
+            }
         }
-
         super.loadIfVisible();
     }
 
     @Override
     public void load() {
         getLoaderManager().initLoader(LOADER_ID_ACCOUNT, null, mAccountLoaderCallbacks);
     }
 
-    private void showSubPanel(Fragment subPanel) {
-        if (mCurrentFragment == subPanel) {
+    private void showSubPanel(Account account) {
+        final Action actionNeeded = getActionNeeded(account);
+        final String actionString = actionNeeded != null ? actionNeeded.name() : NO_ACCOUNT;
+        final int orientation = HardwareUtils.isTablet() ? GeckoScreenOrientation.getInstance().getAndroidOrientation()
+                : Configuration.ORIENTATION_UNDEFINED;
+
+        // Check if fragment for given action and orientation is in the back-stack.
+        final Pair<String, Integer> actionOrientationPair = getActionAndOrientationForFragmentInBackStack();
+        if (actionOrientationPair != null && actionOrientationPair.first.equals(actionString) && (actionOrientationPair.second == orientation)) {
             return;
         }
-        mCurrentFragment = subPanel;
 
-        Bundle args = subPanel.getArguments();
-        if (args == null) {
-            args = new Bundle();
-        }
+        // Instantiate the fragment for the action and update the arguments.
+        Fragment subPanel = makeFragmentForAction(actionNeeded);
+        final Bundle args = new Bundle();
         args.putBoolean(HomePager.CAN_LOAD_ARG, getCanLoadHint());
+        args.putString(FRAGMENT_ACTION, actionString);
+        args.putInt(FRAGMENT_ORIENTATION, orientation);
         subPanel.setArguments(args);
 
+        // Add the fragment to the back-stack.
         getChildFragmentManager()
             .beginTransaction()
             .addToBackStack(null)
-            .replace(R.id.remote_tabs_container, subPanel)
+            .replace(R.id.remote_tabs_container, subPanel, FRAGMENT_TAG)
             .commitAllowingStateLoss();
     }
 
+    private Pair<String, Integer> getActionAndOrientationForFragmentInBackStack() {
+        final Fragment currentFragment = getChildFragmentManager().findFragmentByTag(FRAGMENT_TAG);
+        if (currentFragment != null && currentFragment.getArguments() != null) {
+            final String fragmentAction  = currentFragment.getArguments().getString(FRAGMENT_ACTION);
+            final int fragmentOrientation = currentFragment.getArguments().getInt(FRAGMENT_ORIENTATION);
+            return Pair.create(fragmentAction, fragmentOrientation);
+        }
+        return null;
+    }
+
     /**
      * Get whatever <code>Action</code> is required to continue healthy syncing
      * of Remote Tabs.
      * <p>
      * A Firefox Account can be in many states, from healthy to requiring a
      * Fennec upgrade to continue use. If we have a Firefox Account, but the
      * state seems corrupt, the best we can do is ask for a password, which
      * resets most of the Account state. The health of a Sync account is
@@ -182,46 +199,16 @@ public class RemoteTabsPanel extends Hom
             // Account at this point, so let's show the needs password screen.
             // That's our best hope of righting the ship.
             Log.wtf(LOGTAG, "Got unexpected action needed; offering needs password.");
             return RemoteTabsStaticFragment.newInstance(R.layout.remote_tabs_needs_password);
         }
     }
 
     /**
-     * Get the <code>Fragment</code> that reflects the given
-     * <code>Account</code> and its state.
-     * <p>
-     * A null Account means there is no Account (Sync or Firefox) on the device.
-     *
-     * @param account
-     *            Android Account (Sync or Firefox); may be null.
-     */
-    private Fragment getFragmentNeeded(Account account) {
-        final Action actionNeeded = getActionNeeded(account);
-
-        if (actionNeeded == null) {
-            if (mFallbackFragment == null) {
-                mFallbackFragment = makeFragmentForAction(null);
-            }
-            return mFallbackFragment;
-        }
-
-        Fragment fragment = mFragmentCache.get(actionNeeded);
-        // On a tablet devices with accounts authenticated, create a new fragment based on the current orientation.
-        // The cached fragment in the above case may not be the valid fragment for the current orientation.
-        if (fragment == null || (HardwareUtils.isTablet() && actionNeeded == Action.None)) {
-            fragment = makeFragmentForAction(actionNeeded);
-            mFragmentCache.put(actionNeeded, fragment);
-        }
-
-        return fragment;
-    }
-
-    /**
      * Update the UI to reflect the given <code>Account</code> and its state.
      * <p>
      * A null Account means there is no Account (Sync or Firefox) on the device.
      *
      * @param account
      *            Android Account (Sync or Firefox); may be null.
      */
     protected void updateUiFromAccount(Account account) {
@@ -229,17 +216,17 @@ public class RemoteTabsPanel extends Hom
             // Early abort. When the fragment is detached, we get a loader
             // reset, which calls this with a null account parameter. A null
             // account is valid (it means there is no account, either Sync or
             // Firefox), and so we start to offer the setup flow. But this all
             // happens after the view has been destroyed, which means inserting
             // the setup flow fails. In this case, just abort.
             return;
         }
-        showSubPanel(getFragmentNeeded(account));
+        showSubPanel(account);
     }
 
     private class AccountLoaderCallbacks implements LoaderCallbacks<Account> {
         @Override
         public Loader<Account> onCreateLoader(int id, Bundle args) {
             return new AccountLoader(getActivity());
         }