Bug 1302936 - Ignore tab clicks when the tab has already been removed. r=sebastian a=jcristau
authorTom Klein <twointofive@gmail.com>
Sat, 12 Nov 2016 09:09:17 -0600
changeset 352729 76cf4e84939e3258e654c7cc756503bcc807ff7e
parent 352728 8ec3755143534917ca172b8e922d21700a107fe1
child 352730 f51d3ab82dc60c54403cc6f64f4e0a5a87a5f5ff
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian, jcristau
bugs1302936
milestone52.0a2
Bug 1302936 - Ignore tab clicks when the tab has already been removed. r=sebastian a=jcristau MozReview-Commit-ID: K0eUnMHI9j1
mobile/android/base/java/org/mozilla/gecko/Tabs.java
mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayout.java
mobile/android/base/java/org/mozilla/gecko/tabs/TabsLayout.java
--- a/mobile/android/base/java/org/mozilla/gecko/Tabs.java
+++ b/mobile/android/base/java/org/mozilla/gecko/Tabs.java
@@ -260,17 +260,17 @@ public class Tabs implements GeckoEventL
             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 null;
+            return tab;
         }
 
         mSelectedTab = tab;
         notifyListeners(tab, TabEvents.SELECTED);
 
         if (mLayerView != null) {
             mLayerView.setClearColor(getTabColor(tab));
         }
--- a/mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayout.java
+++ b/mobile/android/base/java/org/mozilla/gecko/tabs/TabsGridLayout.java
@@ -97,23 +97,24 @@ class TabsGridLayout extends GridView
         // Your demise, GridView, cannot come fast enough.
         final int paddingBottom = paddingTop * 2;
 
         setPadding(padding, paddingTop, padding, paddingBottom);
 
         setOnItemClickListener(new OnItemClickListener() {
             @Override
             public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
-                final TabsLayoutItemView tab = (TabsLayoutItemView) view;
-                final int tabId = tab.getTabId();
-                Tabs.getInstance().selectTab(tabId);
+                final TabsLayoutItemView tabView = (TabsLayoutItemView) view;
+                final int tabId = tabView.getTabId();
+                final Tab tab = Tabs.getInstance().selectTab(tabId);
+                if (tab == null) {
+                    return;
+                }
                 autoHidePanel();
-                Tabs.getInstance().notifyListeners(
-                        Tabs.getInstance().getTab(tabId), Tabs.TabEvents.OPENED_FROM_TABS_TRAY
-                );
+                Tabs.getInstance().notifyListeners(tab, Tabs.TabEvents.OPENED_FROM_TABS_TRAY);
             }
         });
 
         TabSwipeGestureListener mSwipeListener = new TabSwipeGestureListener();
         setOnTouchListener(mSwipeListener);
         setOnScrollListener(mSwipeListener.makeScrollListener());
     }
 
@@ -572,21 +573,21 @@ class TabsGridLayout extends GridView
                     }
 
                     cancelCheckForTap();
                     mSwipeView.setPressed(false);
 
                     if (!mSwiping) {
                         final TabsLayoutItemView item = (TabsLayoutItemView) mSwipeView;
                         final int tabId = item.getTabId();
-                        Tabs.getInstance().selectTab(tabId);
-                        autoHidePanel();
-                        Tabs.getInstance().notifyListeners(
-                                Tabs.getInstance().getTab(tabId), Tabs.TabEvents.OPENED_FROM_TABS_TRAY
-                        );
+                        final Tab tab = Tabs.getInstance().selectTab(tabId);
+                        if (tab != null) {
+                            autoHidePanel();
+                            Tabs.getInstance().notifyListeners(tab, Tabs.TabEvents.OPENED_FROM_TABS_TRAY);
+                        }
 
                         mVelocityTracker.recycle();
                         mVelocityTracker = null;
                         break;
                     }
 
                     mVelocityTracker.addMovement(e);
                     mVelocityTracker.computeCurrentVelocity(1000, mMaxFlingVelocity);
--- a/mobile/android/base/java/org/mozilla/gecko/tabs/TabsLayout.java
+++ b/mobile/android/base/java/org/mozilla/gecko/tabs/TabsLayout.java
@@ -127,20 +127,25 @@ public abstract class TabsLayout extends
     // Addition of a tab at selected positions (dependent on LayoutManager) will result in a tab
     // being added out of view - return true if index is such a position.
     abstract protected boolean addAtIndexRequiresScroll(int index);
 
     @Override
     public void onItemClicked(RecyclerView recyclerView, int position, View v) {
         final TabsLayoutItemView item = (TabsLayoutItemView) v;
         final int tabId = item.getTabId();
-        final Tabs tabs = Tabs.getInstance();
-        tabs.selectTab(tabId);
+        final Tab tab = Tabs.getInstance().selectTab(tabId);
+        if (tab == null) {
+            // The tab that was clicked no longer exists in the tabs list (which can happen if you
+            // tap on a tab while its remove animation is running), so ignore the click.
+            return;
+        }
+
         autoHidePanel();
-        tabs.notifyListeners(tabs.getTab(tabId), Tabs.TabEvents.OPENED_FROM_TABS_TRAY);
+        Tabs.getInstance().notifyListeners(tab, Tabs.TabEvents.OPENED_FROM_TABS_TRAY);
     }
 
     // Updates the selected position in the list so that it will be scrolled to the right place.
     private void updateSelectedPosition() {
         final int selected = tabsAdapter.getPositionForTab(Tabs.getInstance().getSelectedTab());
         if (selected != NO_POSITION) {
             scrollToPosition(selected);
         }
@@ -160,19 +165,24 @@ public abstract class TabsLayout extends
 
         tabsAdapter.setTabs(tabData);
         updateSelectedPosition();
     }
 
     private void closeTab(View view) {
         final TabsLayoutItemView itemView = (TabsLayoutItemView) view;
         final Tab tab = getTabForView(itemView);
+        if (tab == null) {
+            // We can be null here if this is the second closeTab call resulting from a sufficiently
+            // fast double tap on the close tab button.
+            return;
+        }
+
         final boolean closingLastTab = tabsAdapter.getItemCount() == 1;
         Tabs.getInstance().closeTab(tab, true);
-
         if (closingLastTab) {
             autoHidePanel();
         }
     }
 
     protected void closeAllTabs() {
         final Iterable<Tab> tabs = Tabs.getInstance().getTabsInOrder();
         for (final Tab tab : tabs) {