fix callback NPEs when WebView is destroyed

    Bug: 6141489

change destroy flow for WebViewCore and block callback
messages
Change-Id: Ib15a0e016508bc6df031996adde150dd37c195c4

3 files changed, 33 insertions, 20 deletions

diff --git a/core/java/android/webkit/CallbackProxy.java b/core/java/android/webkit/CallbackProxy.java
index 2afb841..800ebc8 100644
--- a/core/java/android/webkit/CallbackProxy.java
+++ b/core/java/android/webkit/CallbackProxy.java
@@ -76,6 +76,8 @@ class CallbackProxy extends Handler {
     private volatile WebBackForwardListClient mWebBackForwardListClient;
     // Used to call startActivity during url override.
     private final Context mContext;
+    // block messages flag for destroy
+    private boolean mBlockMessages;
 
     // Message IDs
     private static final int PAGE_STARTED                         = 100;
@@ -155,10 +157,18 @@ class CallbackProxy extends Handler {
         mBackForwardList = new WebBackForwardList(this);
     }
 
+    protected synchronized void blockMessages() {
+        mBlockMessages = true;
+    }
+
+    protected synchronized boolean messagesBlocked() {
+        return mBlockMessages;
+    }
+
     protected void shutdown() {
+        removeCallbacksAndMessages(null);
         setWebViewClient(null);
         setWebChromeClient(null);
-        removeCallbacksAndMessages(null);
     }
 
     /**
@@ -265,6 +275,7 @@ class CallbackProxy extends Handler {
         // in the UI thread. The WebViewClient and WebChromeClient functions
         // that check for a non-null callback are ok because java ensures atomic
         // 32-bit reads and writes.
+        if (messagesBlocked()) return;
         switch (msg.what) {
             case PAGE_STARTED:
                 String startedUrl = msg.getData().getString("url");
diff --git a/core/java/android/webkit/WebViewClassic.java b/core/java/android/webkit/WebViewClassic.java
index af011a1b..5db2431 100644
--- a/core/java/android/webkit/WebViewClassic.java
+++ b/core/java/android/webkit/WebViewClassic.java
@@ -2070,6 +2070,7 @@ public final class WebViewClassic implements WebViewProvider, WebViewProvider.Sc
     }
 
     private void destroyImpl() {
+        mCallbackProxy.blockMessages();
         clearHelpers();
         if (mListBoxDialog != null) {
             mListBoxDialog.dismiss();
diff --git a/core/java/android/webkit/WebViewCore.java b/core/java/android/webkit/WebViewCore.java
index 09aa286..de30755 100644
--- a/core/java/android/webkit/WebViewCore.java
+++ b/core/java/android/webkit/WebViewCore.java
@@ -1244,6 +1244,23 @@ public final class WebViewCore {
                                 + " arg1=" + msg.arg1 + " arg2=" + msg.arg2
                                 + " obj=" + msg.obj);
                     }
+                    switch (msg.what) {
+                    case PAUSE_TIMERS:
+                        mSavedPriority = Process.getThreadPriority(mTid);
+                        Process.setThreadPriority(mTid,
+                            Process.THREAD_PRIORITY_BACKGROUND);
+                        pauseTimers();
+                        if (mNativeClass != 0) {
+                            nativeCloseIdleConnections(mNativeClass);
+                        }
+                        return;
+
+                    case RESUME_TIMERS:
+                        Process.setThreadPriority(mTid, mSavedPriority);
+                        resumeTimers();
+                        return;
+                    }
+
                     if (mWebView == null || mNativeClass == 0) {
                         if (DebugFlags.WEB_VIEW_CORE) {
                             Log.w(LOGTAG, "Rejecting message " + msg.what
@@ -1252,8 +1269,6 @@ public final class WebViewCore {
                         return;
                     }
                     if (mDestroying == true
-                            && msg.what != EventHub.RESUME_TIMERS
-                            && msg.what != EventHub.PAUSE_TIMERS
                             && msg.what != EventHub.DESTROY) {
                         if (DebugFlags.WEB_VIEW_CORE) {
                             Log.v(LOGTAG, "Rejecting message " + msg.what
@@ -1419,18 +1434,6 @@ public final class WebViewCore {
                             restoreState(msg.arg1);
                             break;
 
-                        case PAUSE_TIMERS:
-                            mSavedPriority = Process.getThreadPriority(mTid);
-                            Process.setThreadPriority(mTid,
-                                    Process.THREAD_PRIORITY_BACKGROUND);
-                            pauseTimers();
-                            nativeCloseIdleConnections(mNativeClass);
-                            break;
-
-                        case RESUME_TIMERS:
-                            Process.setThreadPriority(mTid, mSavedPriority);
-                            resumeTimers();
-                            break;
 
                         case ON_PAUSE:
                             nativePause(mNativeClass);
@@ -1961,12 +1964,10 @@ public final class WebViewCore {
      */
     void destroy() {
         synchronized (mEventHub) {
-            // Do not call removeMessages as then we risk removing PAUSE_TIMERS
-            // or RESUME_TIMERS messages, which we must still handle as they
-            // are per process. DESTROY will instead trigger a white list in
-            // mEventHub, skipping any remaining messages in the queue
+            // send DESTROY to front of queue
+            // PAUSE/RESUME timers will still be processed even if they get handled later
             mEventHub.mDestroying = true;
-            mEventHub.sendMessage(
+            mEventHub.sendMessageAtFrontOfQueue(
                     Message.obtain(null, EventHub.DESTROY));
             mEventHub.blockMessages();
         }