Bug 1359531 - Part 5 - Trigger activity switching from GeckoApp. r?sebastian,walkingice draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Sat, 29 Apr 2017 15:20:19 +0200
changeset 582513 6cfd49064140ab112e4de051d4a6142e1dc544f3
parent 582512 c11e0e22938853ef4cf19592b8b763c7ee082f03
child 582514 6526cb9fc61a427fa2ed4a6d0e19bc05531f7d92
push id60121
push usermozilla@buttercookie.de
push dateMon, 22 May 2017 19:37:24 +0000
reviewerssebastian, walkingice
bugs1359531
milestone55.0a1
Bug 1359531 - Part 5 - Trigger activity switching from GeckoApp. r?sebastian,walkingice To avoid the need for tracking the currently active activity, we just respond from the activity itself by using GeckoApp's tab event listener. As replacement for the "closeTabNoActivitySwitch()" function, we set a flag on the activity instead before closing the tab. MozReview-Commit-ID: LdmyJ5i10jZ
mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
mobile/android/base/java/org/mozilla/gecko/Tabs.java
mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
mobile/android/components/SessionStore.js
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ -208,17 +208,17 @@ public abstract class GeckoApp extends G
     private View mFullScreenPluginView;
 
     protected boolean mLastSessionCrashed;
     protected boolean mShouldRestore;
     private boolean mSessionRestoreParsingFinished = false;
 
     protected int mLastSelectedTabId = INVALID_TAB_ID;
     protected String mLastSessionUUID = null;
-    protected boolean mCheckTabSelectionOnResume = false;
+    protected boolean mSuppressActivitySwitch = false;
 
     private boolean foregrounded = false;
 
     private static final class LastSessionParser extends SessionParser {
         private JSONArray tabs;
         private JSONObject windowObject;
         private boolean loadingExternalURL;
 
@@ -416,35 +416,38 @@ public abstract class GeckoApp extends G
     public void removeAppStateListener(GeckoAppShell.AppStateListener listener) {
         mAppStateListeners.remove(listener);
     }
 
     @Override
     public void onTabChanged(Tab tab, Tabs.TabEvents msg, String data) {
         // When a tab is closed, it is always unselected first.
         // When a tab is unselected, another tab is always selected first.
-        // When we're switching activities because of differing tab types,
-        // the first statement is not true.
         switch (msg) {
             case UNSELECTED:
                 break;
 
             case LOCATION_CHANGE:
                 // We only care about location change for the selected tab.
                 if (Tabs.getInstance().isSelectedTab(tab)) {
                     resetOptionsMenu();
                     resetFormAssistPopup();
                 }
                 break;
 
             case SELECTED:
                 resetOptionsMenu();
                 resetFormAssistPopup();
 
-                if (saveAsLastSelectedTab(tab)) {
+                if (mLastSelectedTabId != INVALID_TAB_ID && foregrounded &&
+                        // mSuppressActivitySwitch implies that we want to defer a pending
+                        // activity switch because we're actually about to leave the app.
+                        !mSuppressActivitySwitch && !tab.matchesActivity(this)) {
+                    startActivity(IntentHelper.getTabSwitchIntent(tab));
+                } else if (saveAsLastSelectedTab(tab)) {
                     mLastSelectedTabId = tab.getId();
                     mLastSessionUUID = GeckoApplication.getSessionUUID();
                 }
                 break;
 
             case CLOSED:
                 if (saveAsLastSelectedTab(tab)) {
                     if (mLastSelectedTabId == tab.getId() &&
@@ -2219,21 +2222,17 @@ public abstract class GeckoApp extends G
         // Otherwise, we just try matching the tab ID with one of our open tabs.
         return tabToCheck != null && (!intent.hasExtra(INTENT_EXTRA_SESSION_UUID) ||
                 GeckoApplication.getSessionUUID().equals(intentSessionUUID));
     }
 
     protected void handleSelectTabIntent(SafeIntent intent) {
         final int tabId = intent.getIntExtra(INTENT_EXTRA_TAB_ID, INVALID_TAB_ID);
         final Tab selectedTab = Tabs.getInstance().selectTab(tabId);
-        // If the tab selection has been redirected to a different activity,
-        // the selectedTab within Tabs will not have been updated yet.
-        if (selectedTab == Tabs.getInstance().getSelectedTab()) {
-            onTabSelectFromIntent(selectedTab);
-        }
+        onTabSelectFromIntent(selectedTab);
     }
 
     /**
      * 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();
@@ -2279,19 +2278,24 @@ public abstract class GeckoApp extends G
             return;
         }
 
         foregrounded = true;
 
         GeckoAppShell.setGeckoInterface(this);
         GeckoAppShell.setScreenOrientationDelegate(this);
 
+        // When backing out of the app triggers a tab close and therefore selects another tab, we
+        // don't switch activities even if the new selected tab is of a different type, because
+        // doing so would bring us into the foreground again.
+        // As this means that the currently selected tab doesn't match the last active GeckoApp, we
+        // need to check mSuppressActivitySwitch as well here.
         if (mLastActiveGeckoApp == null || mLastActiveGeckoApp.get() != this ||
-                mCheckTabSelectionOnResume) {
-            mCheckTabSelectionOnResume = false;
+                mSuppressActivitySwitch) {
+            mSuppressActivitySwitch = false;
             restoreLastSelectedTab();
         }
         mIgnoreLastSelectedTab = false;
 
         int newOrientation = getResources().getConfiguration().orientation;
         if (GeckoScreenOrientation.getInstance().update(newOrientation)) {
             refreshChrome();
         }
@@ -2707,18 +2711,18 @@ public abstract class GeckoApp extends G
                     Tab nextSelectedTab = Tabs.getInstance().getNextTab(tab);
                     // Closing the tab will select the next tab. There's no need to unzombify it
                     // if we're really exiting - switching activities is a different matter, though.
                     if (nextSelectedTab != null && nextSelectedTab.getType() == tab.getType()) {
                         final GeckoBundle data = new GeckoBundle(1);
                         data.putInt("nextSelectedTabId", nextSelectedTab.getId());
                         EventDispatcher.getInstance().dispatch("Tab:KeepZombified", data);
                     }
-                    tabs.closeTabNoActivitySwitch(tab);
-                    mCheckTabSelectionOnResume = true;
+                    mSuppressActivitySwitch = true;
+                    tabs.closeTab(tab);
                     return;
                 }
 
                 final int parentId = tab.getParentId();
                 final Tab parent = tabs.getTab(parentId);
                 if (parent != null) {
                     // The back button should always return to the parent (not a sibling).
                     tabs.closeTab(tab, parent);
--- a/mobile/android/base/java/org/mozilla/gecko/Tabs.java
+++ b/mobile/android/base/java/org/mozilla/gecko/Tabs.java
@@ -7,18 +7,16 @@ package org.mozilla.gecko;
 
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.atomic.AtomicInteger;
 
-import android.app.Activity;
-import android.content.Intent;
 import android.content.SharedPreferences;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 
 import org.mozilla.gecko.annotation.JNITarget;
 import org.mozilla.gecko.annotation.RobocopTarget;
 import org.mozilla.gecko.db.BrowserDB;
 import org.mozilla.gecko.distribution.PartnerBrowserCustomizationsClient;
@@ -300,40 +298,28 @@ public class Tabs implements BundleEvent
             Tab tab = getTab(id);
             mOrder.remove(tab);
             mTabs.remove(id);
             tabPositionCache.mTabId = INVALID_TAB_ID;
         }
     }
 
     public synchronized Tab selectTab(int id) {
-        return selectTab(id, true);
-    }
-
-    public synchronized Tab selectTab(int id, boolean switchActivities) {
         if (!mTabs.containsKey(id))
             return null;
 
         final Tab oldTab = getSelectedTab();
         final Tab tab = mTabs.get(id);
 
         // This avoids a NPE below, but callers need to be careful to
         // handle this case.
         if (tab == null || oldTab == tab) {
             return tab;
         }
 
-        if (switchActivities && oldTab != null && oldTab.getType() != tab.getType() &&
-                !currentActivityMatchesTab(tab)) {
-            // We're in the wrong activity for this kind of tab, so launch the correct one
-            // and then try again.
-            launchActivityForTab(tab);
-            return tab;
-        }
-
         mSelectedTab = tab;
         notifyListeners(tab, TabEvents.SELECTED);
 
         if (mLayerView != null) {
             mLayerView.setClearColor(getTabColor(tab));
         }
 
         if (oldTab != null) {
@@ -342,29 +328,16 @@ public class Tabs implements BundleEvent
 
         // Pass a message to Gecko to update tab state in BrowserApp.
         final GeckoBundle data = new GeckoBundle(1);
         data.putInt("id", tab.getId());
         EventDispatcher.getInstance().dispatch("Tab:Selected", data);
         return tab;
     }
 
-    /**
-     * Check whether the currently active activity matches the tab type of the passed tab.
-     */
-    public boolean currentActivityMatchesTab(Tab tab) {
-        final Activity currentActivity = GeckoActivityMonitor.getInstance().getCurrentActivity();
-        return currentActivity != null && tab.matchesActivity(currentActivity);
-    }
-
-    private void launchActivityForTab(Tab tab) {
-        final Intent intent = IntentHelper.getTabSwitchIntent(tab);
-        mAppContext.startActivity(intent);
-    }
-
     public synchronized boolean selectLastTab() {
         if (mOrder.isEmpty()) {
             return false;
         }
 
         selectTab(mOrder.get(mOrder.size() - 1).getId());
         return true;
     }
@@ -464,43 +437,37 @@ public class Tabs implements BundleEvent
     }
 
     /** Close tab and then select the default next tab */
     @RobocopTarget
     public synchronized void closeTab(Tab tab) {
         closeTab(tab, getNextTab(tab));
     }
 
-    /** Don't switch activities even if the default next tab is of a different tab type */
-    public synchronized void closeTabNoActivitySwitch(Tab tab) {
-        closeTab(tab, getNextTab(tab), false, false);
-    }
-
     public synchronized void closeTab(Tab tab, Tab nextTab) {
-        closeTab(tab, nextTab, false, true);
+        closeTab(tab, nextTab, false);
     }
 
     public synchronized void closeTab(Tab tab, boolean showUndoToast) {
-        closeTab(tab, getNextTab(tab), showUndoToast, true);
+        closeTab(tab, getNextTab(tab), showUndoToast);
     }
 
     /** Close tab and then select nextTab */
-    public synchronized void closeTab(final Tab tab, Tab nextTab,
-                                      boolean showUndoToast, boolean switchActivities) {
+    public synchronized void closeTab(final Tab tab, Tab nextTab, boolean showUndoToast) {
         if (tab == null)
             return;
 
         int tabId = tab.getId();
         removeTab(tabId);
 
         if (nextTab == null) {
             nextTab = loadUrl(getHomepageForNewTab(mAppContext), LOADURL_NEW_TAB);
         }
 
-        selectTab(nextTab.getId(), switchActivities);
+        selectTab(nextTab.getId());
 
         tab.onDestroy();
 
         // Pass a message to Gecko to update tab state in BrowserApp
         final GeckoBundle data = new GeckoBundle(2);
         data.putInt("tabId", tabId);
         data.putBoolean("showUndoToast", showUndoToast);
         EventDispatcher.getInstance().dispatch("Tab:Closed", data);
--- a/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
+++ b/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
@@ -338,18 +338,18 @@ public class CustomTabsActivity extends 
 
     private void bindNavigationCallback(@NonNull final Toolbar toolbar) {
         toolbar.setNavigationOnClickListener(new View.OnClickListener() {
             @Override
             public void onClick(View v) {
                 onDone();
                 final Tabs tabs = Tabs.getInstance();
                 final Tab tab = tabs.getSelectedTab();
-                tabs.closeTabNoActivitySwitch(tab);
-                mCheckTabSelectionOnResume = true;
+                mSuppressActivitySwitch = true;
+                tabs.closeTab(tab);
             }
         });
     }
 
     private void performPendingIntent(@NonNull PendingIntent pendingIntent) {
         // bug 1337771: If intent-creator haven't set data url, call send() directly won't work.
         final Intent additional = new Intent();
         final Tab tab = Tabs.getInstance().getSelectedTab();
--- a/mobile/android/components/SessionStore.js
+++ b/mobile/android/components/SessionStore.js
@@ -682,20 +682,19 @@ SessionStore.prototype = {
 
   onTabClose: function ss_onTabClose(aWindow, aBrowser, aTabIndex) {
     let data = aBrowser.__SS_data;
     let tab = aWindow.BrowserApp.getTabForId(data.tabId);
 
     let windowData = this._windows[aWindow.__SSID];
     if (windowData.selectedTabId == tab.id) {
       // Normally, we will first select another tab anyway before closing the previous tab, which
-      // would make this logic moot. However
-      // - we only update the selected tab when selecting a normal BROWSING-type tab, and
-      // - in conjunction with switching between activities, the event order as we see it can
-      //   become reversed.
+      // would make this logic moot. However we only update the selected tab when selecting a normal
+      // BROWSING-type tab, so we include this just to be on the safe side - although normally there
+      // should always be at least one BROWSING-type tab open.
       windowData.selectedTabId = INVALID_TAB_ID;
     }
 
     if (this._maxTabsUndo == 0 || this._sessionDataIsEmpty(data) || tab.type != "BROWSING") {
       this._lastClosedTabIndex = INVALID_TAB_INDEX;
       return;
     }