From 7e1aeb791e45177440de811363a587f9f95a63bc Mon Sep 17 00:00:00 2001
From: Alan Viverette <alanv@google.com>
Date: Wed, 22 Mar 2017 11:06:59 -0400
Subject: [PATCH] Avoid NPE when PopupWindow is shown while dismissing

Bug: 36468675
Test: PopupWindowTest#testEnterExitInterruption
Change-Id: I03bd297c4e7f7653dfff801ec0adfd7ab869abb5
---
 core/java/android/widget/PopupWindow.java | 46 +++++++++++++++++++------------
 1 file changed, 28 insertions(+), 18 deletions(-)

diff --git a/core/java/android/widget/PopupWindow.java b/core/java/android/widget/PopupWindow.java
index d096baf3c943..e4e79664aec3 100644
--- a/core/java/android/widget/PopupWindow.java
+++ b/core/java/android/widget/PopupWindow.java
@@ -2267,7 +2267,8 @@ public class PopupWindow {
     }
 
     private class PopupDecorView extends FrameLayout {
-        private TransitionListenerAdapter mPendingExitListener;
+        /** Runnable used to clean up listeners after exit transition. */
+        private Runnable mCleanupAfterExit;
 
         public PopupDecorView(Context context) {
             super(context);
@@ -2378,7 +2379,7 @@ public class PopupWindow {
          * <p>
          * <strong>Note:</strong> The transition listener is guaranteed to have
          * its {@code onTransitionEnd} method called even if the transition
-         * never starts; however, it may be called with a {@code null} argument.
+         * never starts.
          */
         public void startExitTransition(@NonNull Transition transition,
                 @Nullable final View anchorRoot, @Nullable final Rect epicenter,
@@ -2394,25 +2395,32 @@ public class PopupWindow {
                 anchorRoot.addOnAttachStateChangeListener(mOnAnchorRootDetachedListener);
             }
 
-            // The exit listener MUST be called for cleanup, even if the
-            // transition never starts or ends. Stash it for later.
-            mPendingExitListener = new TransitionListenerAdapter() {
-                @Override
-                public void onTransitionEnd(Transition t) {
-                    if (anchorRoot != null) {
-                        anchorRoot.removeOnAttachStateChangeListener(mOnAnchorRootDetachedListener);
-                    }
-
-                    listener.onTransitionEnd(t);
+            // The cleanup runnable MUST be called even if the transition is
+            // canceled before it starts (and thus can't call onTransitionEnd).
+            mCleanupAfterExit = () -> {
+                listener.onTransitionEnd(transition);
 
-                    // The listener was called. Our job here is done.
-                    mPendingExitListener = null;
-                    t.removeListener(this);
+                if (anchorRoot != null) {
+                    anchorRoot.removeOnAttachStateChangeListener(mOnAnchorRootDetachedListener);
                 }
+
+                // The listener was called. Our job here is done.
+                mCleanupAfterExit = null;
             };
 
             final Transition exitTransition = transition.clone();
-            exitTransition.addListener(mPendingExitListener);
+            exitTransition.addListener(new TransitionListenerAdapter() {
+                @Override
+                public void onTransitionEnd(Transition t) {
+                    t.removeListener(this);
+
+                    // This null check shouldn't be necessary, but it's easier
+                    // to check here than it is to test every possible case.
+                    if (mCleanupAfterExit != null) {
+                        mCleanupAfterExit.run();
+                    }
+                }
+            });
             exitTransition.setEpicenterCallback(new EpicenterCallback() {
                 @Override
                 public Rect onGetEpicenter(Transition transition) {
@@ -2440,8 +2448,10 @@ public class PopupWindow {
         public void cancelTransitions() {
             TransitionManager.endTransitions(this);
 
-            if (mPendingExitListener != null) {
-                mPendingExitListener.onTransitionEnd(null);
+            // If the cleanup runnable is still around, that means the
+            // transition never started. We should run it now to clean up.
+            if (mCleanupAfterExit != null) {
+                mCleanupAfterExit.run();
             }
         }
 
-- 
2.11.0