Bug 1352997 - Part 3 - Re-implement tracking of last selected tab for BrowserApp. r?sebastian,walkingice draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Sat, 08 Apr 2017 22:55:10 +0200
changeset 569641 61527ba16b8f8fef9356fa164bd4505dd337dff3
parent 569640 51dddfa30a15840f32578c397244f0cb0373d69f
child 569642 9008f40418255ca88a2faf303bf60d10445f6d5d
push id56240
push usermozilla@buttercookie.de
push dateThu, 27 Apr 2017 18:44:39 +0000
reviewerssebastian, walkingice
bugs1352997
milestone55.0a1
Bug 1352997 - Part 3 - Re-implement tracking of last selected tab for BrowserApp. r?sebastian,walkingice Currently, we basically take a snapshot of the currently selected tab when pausing an activity and then later re-select that tab ID when switching back from another activity within our application. In practice, this doesn't seem entirely fool-proof, so when switching between our normal UI (BrowserApp) and custom tabs or web apps we can eventually end up with the wrong tab being selected in the wrong activity. In this part, we'll rip out the current code and replace it by a new implementation for BrowserApp - following parts will then cover custom tabs and web apps. As BrowserApp is our normal tabbed browsing interface, we can simply track all tab switches for BROWSING-type tabs as they happen, which ensures that our data is always up-to-date. Because tab IDs remain unique only within the same application session and are reused if we're terminated and then later restart, we need to take additional precautions to make sure we're really selecting the correct tab object - the savedInstanceState can carry even across (OOM-)kills. Therefore we now additionally also store and compare the current per-session UUID to make sure that the tab we're trying to select is really the same one it was when the activity was last running. For BrowserApp caring about this is less important because on a full startup, the selection behaviour will be overridden by session restore anyway (although we can still hit it if only BrowserApp gets destroyed while Gecko keeps running, or if BrowserApp is launched after some other activity has already loaded Gecko), but it'll be quite relevant for web apps and custom tabs which don't have that benefit. As it stands, this patch temporarily breaks behaviour around activity restoring for custom tabs/web apps, but tearing the old implementation out in one go was easier and the patch needs to be split somewhere. MozReview-Commit-ID: I0Tq9XZGvpV
mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
mobile/android/base/java/org/mozilla/gecko/Tabs.java
mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -179,16 +179,18 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Vector;
 import java.util.regex.Pattern;
 
+import static org.mozilla.gecko.Tab.TabType;
+
 public class BrowserApp extends GeckoApp
                         implements TabsPanel.TabsLayoutChangeListener,
                                    PropertyAnimator.PropertyAnimationListener,
                                    View.OnKeyListener,
                                    DynamicToolbarAnimator.MetricsListener,
                                    DynamicToolbarAnimator.ToolbarChromeProxy,
                                    BrowserSearch.OnSearchListener,
                                    BrowserSearch.OnEditSuggestionListener,
@@ -433,16 +435,21 @@ public class BrowserApp extends GeckoApp
 
         if (HardwareUtils.isTablet() && msg == TabEvents.SELECTED) {
             updateEditingModeForTab(tab);
         }
 
         super.onTabChanged(tab, msg, data);
     }
 
+    @Override
+    protected boolean saveAsLastSelectedTab(Tab tab) {
+        return tab.getType() == TabType.BROWSING;
+    }
+
     private void updateEditingModeForTab(final Tab selectedTab) {
         // (bug 1086983 comment 11) Because the tab may be selected from the gecko thread and we're
         // running this code on the UI thread, the selected tab argument may not still refer to the
         // selected tab. However, that means this code should be run again and the initial state
         // changes will be overridden. As an optimization, we can skip this update, but it may have
         // unknown side-effects so we don't.
         if (!Tabs.getInstance().isSelectedTab(selectedTab)) {
             Log.w(LOGTAG, "updateEditingModeForTab: Given tab is expected to be selected tab");
@@ -1149,16 +1156,36 @@ public class BrowserApp extends GeckoApp
         processTabQueue();
 
         for (BrowserAppDelegate delegate : delegates) {
             delegate.onResume(this);
         }
     }
 
     @Override
+    protected void restoreLastSelectedTab() {
+        if (mResumingAfterOnCreate && !mIsRestoringActivity) {
+            // We're the first activity to run, so our startup code will (have) handle(d) tab selection.
+            return;
+        }
+
+        final Tabs tabs = Tabs.getInstance();
+        final Tab tabToSelect = tabs.getTab(mLastSelectedTabId);
+
+        if (tabToSelect != null && GeckoApplication.getSessionUUID().equals(mLastSessionUUID) &&
+                tabToSelect.getType() == TabType.BROWSING) {
+            tabs.selectTab(mLastSelectedTabId);
+        } else {
+            if (!tabs.selectLastTab(TabType.BROWSING)) {
+                tabs.loadUrl(Tabs.getHomepageForStartupTab(this), Tabs.LOADURL_NEW_TAB);
+            }
+        }
+    }
+
+    @Override
     public void onPause() {
         super.onPause();
         if (mIsAbortingAppLaunch) {
             return;
         }
 
         if (mHasResumed) {
             // Register for Prompt:ShowTop so we can foreground this activity even if it's hidden.
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ -155,17 +155,18 @@ public abstract class GeckoApp
     public static final String ACTION_LOAD                 = "org.mozilla.gecko.LOAD";
     public static final String ACTION_INIT_PW              = "org.mozilla.gecko.INIT_PW";
     public static final String ACTION_SWITCH_TAB           = "org.mozilla.gecko.SWITCH_TAB";
 
     public static final String INTENT_REGISTER_STUMBLER_LISTENER = "org.mozilla.gecko.STUMBLER_REGISTER_LOCAL_LISTENER";
 
     public static final String EXTRA_STATE_BUNDLE          = "stateBundle";
 
-    public static final String LAST_SELECTED_TAB           = "lastSelectedTab";
+    protected static final String LAST_SELECTED_TAB        = "lastSelectedTab";
+    protected static final String LAST_SESSION_UUID        = "lastSessionUUID";
 
     public static final String PREFS_ALLOW_STATE_BUNDLE    = "allowStateBundle";
     public static final String PREFS_FLASH_USAGE           = "playFlashCount";
     public static final String PREFS_VERSION_CODE          = "versionCode";
     public static final String PREFS_WAS_STOPPED           = "wasStopped";
     public static final String PREFS_CRASHED_COUNT         = "crashedCount";
     public static final String PREFS_CLEANUP_TEMP_FILES    = "cleanupTempFiles";
 
@@ -177,17 +178,18 @@ public abstract class GeckoApp
     public static final String SAVED_STATE_PRIVATE_SESSION = "privateSession";
 
     // Delay before running one-time "cleanup" tasks that may be needed
     // after a version upgrade.
     private static final int CLEANUP_DEFERRAL_SECONDS = 15;
 
     private static boolean sAlreadyLoaded;
 
-    private static WeakReference<GeckoApp> lastActiveGeckoApp;
+    protected boolean mResumingAfterOnCreate;
+    protected static WeakReference<GeckoApp> mLastActiveGeckoApp;
 
     protected RelativeLayout mRootLayout;
     protected RelativeLayout mMainLayout;
 
     protected RelativeLayout mGeckoLayout;
     private OrientationEventListener mCameraOrientationEventListener;
     public List<GeckoAppShell.AppStateListener> mAppStateListeners = new LinkedList<GeckoAppShell.AppStateListener>();
     protected MenuPanel mMenuPanel;
@@ -211,17 +213,18 @@ public abstract class GeckoApp
     private View mFullScreenPluginView;
 
     private final HashMap<String, PowerManager.WakeLock> mWakeLocks = new HashMap<String, PowerManager.WakeLock>();
 
     protected boolean mLastSessionCrashed;
     protected boolean mShouldRestore;
     private boolean mSessionRestoreParsingFinished = false;
 
-    protected int lastSelectedTabId = INVALID_TAB_ID;
+    protected int mLastSelectedTabId = INVALID_TAB_ID;
+    protected String mLastSessionUUID = null;
 
     private boolean foregrounded = false;
 
     private static final class LastSessionParser extends SessionParser {
         private JSONArray tabs;
         private JSONObject windowObject;
         private boolean isExternalURL;
 
@@ -422,16 +425,31 @@ public abstract class GeckoApp
                     resetOptionsMenu();
                     resetFormAssistPopup();
                 }
                 break;
 
             case SELECTED:
                 resetOptionsMenu();
                 resetFormAssistPopup();
+
+                if (saveAsLastSelectedTab(tab)) {
+                    mLastSelectedTabId = tab.getId();
+                    mLastSessionUUID = GeckoApplication.getSessionUUID();
+                }
+                break;
+
+            case CLOSED:
+                if (saveAsLastSelectedTab(tab)) {
+                    if (mLastSelectedTabId == tab.getId() &&
+                            GeckoApplication.getSessionUUID().equals(mLastSessionUUID)) {
+                        mLastSelectedTabId = Tabs.INVALID_TAB_ID;
+                        mLastSessionUUID = null;
+                    }
+                }
                 break;
 
             case DESKTOP_MODE_CHANGE:
                 if (Tabs.getInstance().isSelectedTab(tab))
                     resetOptionsMenu();
                 break;
         }
     }
@@ -443,16 +461,24 @@ public abstract class GeckoApp
     }
 
     private void resetFormAssistPopup() {
         if (mInitialized && mFormAssistPopup != null) {
             mFormAssistPopup.hide();
         }
     }
 
+    /**
+     * Called on tab selection and tab close - return true to allow updating of this activity's
+     * last selected tab.
+     */
+    protected boolean saveAsLastSelectedTab(Tab tab) {
+        return false;
+    }
+
     public void refreshChrome() { }
 
     @Override
     public void invalidateOptionsMenu() {
         if (mMenu == null) {
             return;
         }
 
@@ -648,22 +674,18 @@ public abstract class GeckoApp
     }
 
     @Override
     protected void onSaveInstanceState(Bundle outState) {
         super.onSaveInstanceState(outState);
 
         outState.putBoolean(SAVED_STATE_IN_BACKGROUND, isApplicationInBackground());
         outState.putString(SAVED_STATE_PRIVATE_SESSION, mPrivateBrowsingSession);
-        outState.putInt(LAST_SELECTED_TAB, lastSelectedTabId);
-    }
-
-    @Override
-    protected void onRestoreInstanceState(final Bundle inState) {
-        lastSelectedTabId = inState.getInt(LAST_SELECTED_TAB);
+        outState.putInt(LAST_SELECTED_TAB, mLastSelectedTabId);
+        outState.putString(LAST_SESSION_UUID, mLastSessionUUID);
     }
 
     public void addTab() { }
 
     public void addPrivateTab() { }
 
     public void showNormalTabs() { }
 
@@ -1203,16 +1225,21 @@ public abstract class GeckoApp
             finish();
             return;
         }
 
         // The clock starts...now. Better hurry!
         mJavaUiStartupTimer = new Telemetry.UptimeTimer("FENNEC_STARTUP_TIME_JAVAUI");
         mGeckoReadyStartupTimer = new Telemetry.UptimeTimer("FENNEC_STARTUP_TIME_GECKOREADY");
 
+        if (savedInstanceState != null) {
+            mLastSelectedTabId = savedInstanceState.getInt(LAST_SELECTED_TAB);
+            mLastSessionUUID = savedInstanceState.getString(LAST_SESSION_UUID);
+        }
+
         final SafeIntent intent = new SafeIntent(getIntent());
 
         earlyStartJavaSampler(intent);
 
         // GeckoLoader wants to dig some environment variables out of the
         // incoming intent, so pass it in here. GeckoLoader will do its
         // business later and dispose of the reference.
         GeckoLoader.setLastIntent(intent);
@@ -1250,16 +1277,18 @@ public abstract class GeckoApp
         // restart, and will be propagated to Gecko accordingly, so there's
         // no need to touch that here.
         if (BrowserLocaleManager.getInstance().systemLocaleDidChange()) {
             Log.i(LOGTAG, "System locale changed. Restarting.");
             doRestart();
             return;
         }
 
+        mResumingAfterOnCreate = true;
+
         if (sAlreadyLoaded) {
             // This happens when the GeckoApp activity is destroyed by Android
             // without killing the entire application (see Bug 769269).
             // Now that we've got multiple GeckoApp-based activities, this can
             // also happen if we're not the first activity to run within a session.
             mIsRestoringActivity = true;
             Telemetry.addToHistogram("FENNEC_RESTORING_ACTIVITY", 1);
 
@@ -2223,30 +2252,28 @@ public abstract class GeckoApp
             passedUri = null;
         }
 
         if (hasGeckoTab(intent)) {
             // This also covers ACTION_SWITCH_TAB.
             handleSelectTabIntent(intent);
         } else if (ACTION_LOAD.equals(action)) {
             Tabs.getInstance().loadUrl(intent.getDataString());
-            lastSelectedTabId = INVALID_TAB_ID;
         } else if (Intent.ACTION_VIEW.equals(action)) {
             processActionViewIntent(new Runnable() {
                 @Override
                 public void run() {
                     final String url = intent.getDataString();
                     int flags = Tabs.LOADURL_NEW_TAB | Tabs.LOADURL_USER_ENTERED | Tabs.LOADURL_EXTERNAL;
                     if (isFirstTab) {
                         flags |= Tabs.LOADURL_FIRST_AFTER_ACTIVITY_UNHIDDEN;
                     }
                     Tabs.getInstance().loadUrlWithIntentExtras(url, intent, flags);
                 }
             });
-            lastSelectedTabId = INVALID_TAB_ID;
         } else if (ACTION_HOMESCREEN_SHORTCUT.equals(action)) {
             mLayerView.loadUri(uri, GeckoView.LOAD_SWITCH_TAB);
         } else if (Intent.ACTION_SEARCH.equals(action)) {
             mLayerView.loadUri(uri, GeckoView.LOAD_NEW_TAB);
         } else if (NotificationHelper.HELPER_BROADCAST_ACTION.equals(action)) {
             NotificationHelper.getInstance(getApplicationContext()).handleNotificationIntent(intent);
         } else if (ACTION_LAUNCH_SETTINGS.equals(action)) {
             // Check if launched from data reporting notification.
@@ -2262,17 +2289,16 @@ public abstract class GeckoApp
     protected boolean hasGeckoTab(SafeIntent intent) {
         final int tabId = intent.getIntExtra(Tabs.INTENT_EXTRA_TAB_ID, INVALID_TAB_ID);
         return Tabs.getInstance().getTab(tabId) != null;
     }
 
     protected void handleSelectTabIntent(SafeIntent intent) {
         final int tabId = intent.getIntExtra(Tabs.INTENT_EXTRA_TAB_ID, INVALID_TAB_ID);
         Tabs.getInstance().selectTab(tabId);
-        lastSelectedTabId = INVALID_TAB_ID;
     }
 
     /**
      * Handles getting a URI from an intent in a way that is backwards-
      * compatible with our previous implementations.
      */
     protected String getURIFromIntent(SafeIntent intent) {
         final String action = intent.getAction();
@@ -2306,17 +2332,19 @@ public abstract class GeckoApp
             return;
         }
 
         foregrounded = true;
 
         GeckoAppShell.setGeckoInterface(this);
         GeckoAppShell.setScreenOrientationDelegate(this);
 
-        restoreLastSelectedTab();
+        if (mLastActiveGeckoApp == null || mLastActiveGeckoApp.get() != this) {
+            restoreLastSelectedTab();
+        }
 
         int newOrientation = getResources().getConfiguration().orientation;
         if (GeckoScreenOrientation.getInstance().update(newOrientation)) {
             refreshChrome();
         }
 
         if (mAppStateListeners != null) {
             for (GeckoAppShell.AppStateListener listener : mAppStateListeners) {
@@ -2356,23 +2384,25 @@ public abstract class GeckoApp
                     rec.processDelayed();
                 } else {
                     Log.w(LOGTAG, "Can't record session: rec is null.");
                 }
             }
         });
 
         Restrictions.update(this);
+
+        mResumingAfterOnCreate = false;
     }
 
-    protected void restoreLastSelectedTab() {
-        if (lastSelectedTabId >= 0 && (lastActiveGeckoApp == null || lastActiveGeckoApp.get() != this)) {
-            Tabs.getInstance().selectTab(lastSelectedTabId);
-        }
-    }
+    /**
+     * Called on activity resume if a different (or no) GeckoApp-based activity was previously
+     * active within our application.
+     */
+    protected void restoreLastSelectedTab() { }
 
     @Override
     public void onWindowFocusChanged(boolean hasFocus) {
         super.onWindowFocusChanged(hasFocus);
 
         if (!mWindowFocusInitialized && hasFocus) {
             mWindowFocusInitialized = true;
             // XXX our editor tests require the GeckoView to have focus to pass, so we have to
@@ -2392,21 +2422,17 @@ public abstract class GeckoApp
 
         if (mIsAbortingAppLaunch) {
             super.onPause();
             return;
         }
 
         foregrounded = false;
 
-        final Tab selectedTab = Tabs.getInstance().getSelectedTab();
-        if (selectedTab != null) {
-            lastSelectedTabId = selectedTab.getId();
-        }
-        lastActiveGeckoApp = new WeakReference<GeckoApp>(this);
+        mLastActiveGeckoApp = new WeakReference<GeckoApp>(this);
 
         final HealthRecorder rec = mHealthRecorder;
         final Context context = this;
 
         // In some way it's sad that Android will trigger StrictMode warnings
         // here as the whole point is to save to disk while the activity is not
         // interacting with the user.
         ThreadUtils.postToBackgroundThread(new Runnable() {
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
@@ -28,33 +28,37 @@ import org.mozilla.gecko.notifications.N
 import org.mozilla.gecko.notifications.NotificationHelper;
 import org.mozilla.gecko.preferences.DistroSharedPrefsImport;
 import org.mozilla.gecko.util.BundleEventListener;
 import org.mozilla.gecko.util.EventCallback;
 import org.mozilla.gecko.util.GeckoBundle;
 import org.mozilla.gecko.util.HardwareUtils;
 import org.mozilla.gecko.util.PRNGFixes;
 import org.mozilla.gecko.util.ThreadUtils;
+import org.mozilla.gecko.util.UUIDUtil;
 
 import java.io.File;
 import java.lang.reflect.Method;
+import java.util.UUID;
 
 public class GeckoApplication extends Application
     implements ContextGetter {
     private static final String LOG_TAG = "GeckoApplication";
     private static final String MEDIA_DECODING_PROCESS_CRASH = "MEDIA_DECODING_PROCESS_CRASH";
 
     private boolean mInBackground;
     private boolean mPausedGecko;
     private boolean mIsInitialResume;
 
     private LightweightTheme mLightweightTheme;
 
     private RefWatcher mRefWatcher;
 
+    private static String sSessionUUID = null;
+
     public GeckoApplication() {
         super();
     }
 
     public static RefWatcher getRefWatcher(Context context) {
         GeckoApplication app = (GeckoApplication) context.getApplicationContext();
         return app.mRefWatcher;
     }
@@ -62,16 +66,23 @@ public class GeckoApplication extends Ap
     public static void watchReference(Context context, Object object) {
         if (context == null) {
             return;
         }
 
         getRefWatcher(context).watch(object);
     }
 
+    /**
+     * @return The string representation of an UUID that changes on each application startup.
+     */
+    public static String getSessionUUID() {
+        return sSessionUUID;
+    }
+
     @Override
     public Context getContext() {
         return this;
     }
 
     @Override
     public SharedPreferences getSharedPreferences() {
         return GeckoSharedPrefs.forApp(this);
@@ -161,16 +172,18 @@ public class GeckoApplication extends Ap
             // Not much to be done here: it was weak before, so it's weak now.  Not worth aborting.
             Log.e(LOG_TAG, "Got exception applying PRNGFixes! Cryptographic data produced on this device may be weak. Ignoring.", e);
         }
 
         mIsInitialResume = true;
 
         mRefWatcher = LeakCanary.install(this);
 
+        sSessionUUID = UUID.randomUUID().toString();
+
         registerActivityLifecycleCallbacks(GeckoActivityMonitor.getInstance());
 
         final Context context = getApplicationContext();
         GeckoAppShell.setApplicationContext(context);
         HardwareUtils.init(context);
         FilePicker.init(context);
         DownloadsIntegration.init();
         HomePanelsManager.getInstance().init(context);
--- a/mobile/android/base/java/org/mozilla/gecko/Tabs.java
+++ b/mobile/android/base/java/org/mozilla/gecko/Tabs.java
@@ -429,16 +429,33 @@ public class Tabs implements BundleEvent
         if (mOrder.isEmpty()) {
             return false;
         }
 
         selectTab(mOrder.get(mOrder.size() - 1).getId());
         return true;
     }
 
+    public synchronized boolean selectLastTab(Tab.TabType targetType) {
+        if (mOrder.isEmpty()) {
+            return false;
+        }
+
+        Tab tabToSelect = mOrder.get(mOrder.size() - 1);
+        if (tabToSelect.getType() != targetType) {
+            tabToSelect = getPreviousTabFrom(tabToSelect, false, targetType);
+            if (tabToSelect == null) {
+                return false;
+            }
+        }
+
+        selectTab(tabToSelect.getId());
+        return true;
+    }
+
     private int getIndexOf(Tab tab) {
         return mOrder.lastIndexOf(tab);
     }
 
     private Tab getNextTabFrom(Tab tab, boolean getPrivate, TabType type) {
         int numTabs = mOrder.size();
         int index = getIndexOf(tab);
         for (int i = index + 1; i < numTabs; i++) {
--- a/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
+++ b/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
@@ -230,28 +230,17 @@ public class CustomTabsActivity extends 
     @Override
     protected void onSaveInstanceState(Bundle outState) {
         super.onSaveInstanceState(outState);
         outState.putParcelable(SAVED_START_INTENT, startIntent.getUnsafe());
     }
 
     @Override
     public void onResume() {
-        if (lastSelectedTabId >= 0) {
-            final Tabs tabs = Tabs.getInstance();
-            final Tab tab = tabs.getTab(lastSelectedTabId);
-            if (tab == null) {
-                finish();
-            } else {
-                // we are restoring
-                actionBarPresenter.update(tab);
-            }
-        }
         super.onResume();
-
         mLayerView.getDynamicToolbarAnimator().setPinned(true, PinReason.CUSTOM_TAB);
     }
 
     @Override
     public void onPause() {
         super.onPause();
         mLayerView.getDynamicToolbarAnimator().setPinned(false, PinReason.CUSTOM_TAB);
     }