Implement the plumbing to handle messages in a separate handler

Provide the mechanism to allow applications to provide their own handler
for receiving postmessages on a thread separate than UI thread.

BUG=393291

Review URL: https://codereview.chromium.org/961393002

Cr-Commit-Position: refs/heads/master@{#318589}

android_webview/java/src/org/chromium/android_webview/AwMessagePortService.java

@@ -4,9 +4,6 @@
 
 package org.chromium.android_webview;
 
-import android.os.Handler;
-import android.os.Looper;
-import android.os.Message;
 import android.util.SparseArray;
 
 import org.chromium.base.CalledByNative;
@@ -39,8 +36,6 @@ public class AwMessagePortService {
 
     private static final String TAG = "AwMessagePortService";
 
-    private static final int POST_MESSAGE = 1;
-
     /**
      * Observer for MessageChannel events.
      */
@@ -48,45 +43,6 @@ public class AwMessagePortService {
         void onMessageChannelCreated();
     }
 
-    private static class PostMessageFromWeb {
-        public MessagePort port;
-        public String message;
-
-        public PostMessageFromWeb(MessagePort port, String message) {
-            this.port = port;
-            this.message = message;
-        }
-    }
-
-    // The messages from JS to Java are posted to a message port on a background thread.
-    // We do this to make any potential synchronization between Java and JS
-    // easier for user programs.
-    private static class MessageHandlerThread extends Thread {
-        private static class MessageHandler extends Handler {
-            @Override
-            public void handleMessage(Message msg) {
-                if (msg.what == POST_MESSAGE) {
-                    PostMessageFromWeb m = (PostMessageFromWeb) msg.obj;
-                    m.port.onMessage(m.message);
-                    return;
-                }
-                throw new IllegalStateException("undefined message");
-            }
-        }
-
-        private MessageHandler mHandler;
-
-        public Handler getHandler() {
-            return mHandler;
-        }
-
-        public void run() {
-            Looper.prepare();
-            mHandler = new MessageHandler();
-            Looper.loop();
-        }
-    }
-
     // A thread safe storage for Message Ports.
     private static class MessagePortStorage {
         private SparseArray<MessagePort> mMessagePorts = new SparseArray<MessagePort>();
@@ -112,13 +68,11 @@ public class AwMessagePortService {
 
     private long mNativeMessagePortService;
     private MessagePortStorage mPortStorage = new MessagePortStorage();
-    private MessageHandlerThread mMessageHandlerThread = new MessageHandlerThread();
     private ObserverList<MessageChannelObserver> mObserverList =
             new ObserverList<MessageChannelObserver>();
 
     AwMessagePortService() {
         mNativeMessagePortService = nativeInitAwMessagePortService();
-        mMessageHandlerThread.start();
     }
 
     public void addObserver(MessageChannelObserver observer) {
@@ -181,16 +135,11 @@ public class AwMessagePortService {
         }
     }
 
+    // Called on IO thread.
     @CalledByNative
-    private void onPostMessage(int portId, String message, int[] ports) {
-        PostMessageFromWeb m = new PostMessageFromWeb(mPortStorage.get(portId), message);
-        Handler handler = mMessageHandlerThread.getHandler();
-        if (handler == null) {
-            // TODO(sgurun) handler could be null. But this logic will be removed.
-            return;
-        }
-        Message msg = handler.obtainMessage(POST_MESSAGE, m);
-        handler.sendMessage(msg);
+    private void onReceivedMessage(int portId, String message, int[] ports) {
+        // TODO(sgurun) implement receiving ports from native
+        mPortStorage.get(portId).onReceivedMessage(message);
     }
 
     @CalledByNative

android_webview/java/src/org/chromium/android_webview/MessagePort.java

@@ -4,6 +4,9 @@
 
 package org.chromium.android_webview;
 
+import android.os.Handler;
+import android.os.Looper;
+import android.os.Message;
 import android.util.Log;
 
 /**
@@ -32,7 +35,10 @@ import android.util.Log;
  * since the ownership is also transferred during the transfer. Closing a transferred port will
  * throw an exception.
  *
- * TODO(sgurun) implement queueing messages while a port is in transfer
+ * The fact that messages can be handled on a separate thread means that thread
+ * synchronization is important. All methods are called on UI thread except as noted.
+ *
+ * TODO(sgurun) implement queueing messages while a port is in transfer.
  */
 public class MessagePort implements PostMessageSender.PostMessageSenderDelegate {
 
@@ -41,16 +47,53 @@ public class MessagePort implements PostMessageSender.PostMessageSenderDelegate
      */
     public abstract static class WebEventHandler {
         public abstract void onMessage(String message);
-    };
+    }
 
     private static final String TAG = "MessagePort";
     private static final int PENDING = -1;
+
+    // the what value for POST_MESSAGE
+    private static final int POST_MESSAGE = 1;
+
+    private static class PostMessageFromWeb {
+        public MessagePort port;
+        public String message;
+
+        public PostMessageFromWeb(MessagePort port, String message) {
+            this.port = port;
+            this.message = message;
+        }
+    }
+
+    // Implements the handler to handle messageport messages received from web.
+    // These messages are received on IO thread and normally handled in main
+    // thread however, alternatively application can pass a handler to execute them.
+    private static class MessageHandler extends Handler {
+        public MessageHandler(Looper looper) {
+            super(looper);
+        }
+        @Override
+        public void handleMessage(Message msg) {
+            if (msg.what == POST_MESSAGE) {
+                PostMessageFromWeb m = (PostMessageFromWeb) msg.obj;
+                m.port.onMessage(m.message);
+                return;
+            }
+            throw new IllegalStateException("undefined message");
+        }
+    }
+    // The default message handler
+    private static final MessageHandler sDefaultHandler =
+            new MessageHandler(Looper.getMainLooper());
+
     private int mPortId = PENDING;
     private WebEventHandler mWebEventHandler;
     private AwMessagePortService mMessagePortService;
     private boolean mClosed;
     private boolean mTransferred;
     private PostMessageSender mPostMessageSender;
+    private MessageHandler mHandler;
+    private Object mLock = new Object();
 
     public MessagePort(AwMessagePortService messagePortService) {
         mMessagePortService = messagePortService;
@@ -74,8 +117,10 @@ public class MessagePort implements PostMessageSender.PostMessageSenderDelegate
         if (mTransferred) {
             throw new IllegalStateException("Port is already transferred");
         }
-        if (mClosed) return;
-        mClosed = true;
+        synchronized (mLock) {
+            if (mClosed) return;
+            mClosed = true;
+        }
         // If the port is already ready, and no messages are waiting in the
         // queue to be transferred, onPostMessageQueueEmpty() callback is not
         // received (it is received only after messages are purged). In this
@@ -97,20 +142,39 @@ public class MessagePort implements PostMessageSender.PostMessageSenderDelegate
         mTransferred = true;
     }
 
-    public void setWebEventHandler(WebEventHandler webEventHandler) {
-        mWebEventHandler = webEventHandler;
+    public void setWebEventHandler(WebEventHandler webEventHandler, Handler handler) {
+        synchronized (mLock) {
+            mWebEventHandler = webEventHandler;
+            if (handler != null) {
+                mHandler = new MessageHandler(handler.getLooper());
+            }
+        }
     }
 
-    public void onMessage(String message) {
-        if (isClosed()) {
-            Log.w(TAG, "Port [" + mPortId + "] received message in closed state");
-            return;
+    // Called on IO thread.
+    public void onReceivedMessage(String message) {
+        synchronized (mLock) {
+            PostMessageFromWeb m = new PostMessageFromWeb(this, message);
+            Handler handler = mHandler != null ? mHandler : sDefaultHandler;
+            Message msg = handler.obtainMessage(POST_MESSAGE, m);
+            handler.sendMessage(msg);
         }
-        if (mWebEventHandler == null) {
-            Log.w(TAG, "No handler set for port [" + mPortId + "], dropping message " + message);
-            return;
+    }
+
+    // This method may be called on a different thread than UI thread.
+    public void onMessage(String message) {
+        synchronized (mLock) {
+            if (isClosed()) {
+                Log.w(TAG, "Port [" + mPortId + "] received message in closed state");
+                return;
+            }
+            if (mWebEventHandler == null) {
+                Log.w(TAG, "No handler set for port [" + mPortId + "], dropping message "
+                        + message);
+                return;
+            }
+            mWebEventHandler.onMessage(message);
         }
-        mWebEventHandler.onMessage(message);
     }
 
     public void postMessage(String message, MessagePort[] msgPorts) throws IllegalStateException {

android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java

@@ -5,6 +5,7 @@
 package org.chromium.android_webview.test;
 
 import android.os.Build;
+import android.os.Handler;
 import android.test.suitebuilder.annotation.SmallTest;
 
 import static org.chromium.base.test.util.ScalableTimeout.scaleTimeout;
@@ -463,7 +464,7 @@ public class PostMessageTest extends AwTestBase {
                     public void onMessage(String message) {
                         channelContainer.setMessage(message);
                     }
-                });
+                }, null);
                 mAwContents.postMessageToFrame(null, WEBVIEW_MESSAGE, mWebServer.getBaseUrl(),
                         new MessagePort[]{channel[1]});
             }
@@ -498,7 +499,6 @@ public class PostMessageTest extends AwTestBase {
             + "   </script>"
             + "</body></html>";
 
-
     // Call on non-UI thread.
     private void waitUntilPortReady(final MessagePort port) throws Throwable {
         CriteriaHelper.pollForCriteria(new Criteria() {
@@ -545,7 +545,7 @@ public class PostMessageTest extends AwTestBase {
                     public void onMessage(String message) {
                         channelContainer.setMessage(message);
                     }
-                });
+                }, null);
                 mAwContents.postMessageToFrame(null, WEBVIEW_MESSAGE, mWebServer.getBaseUrl(),
                         new MessagePort[]{channel[1]});
                 channel[0].postMessage(HELLO, null);
@@ -561,12 +561,8 @@ public class PostMessageTest extends AwTestBase {
     // transferred to JS and full communication can happen on it.
     // Do this by sending a message to JS and let it echo'ing the message with
     // some text prepended to it.
-    // @SmallTest
-    // @Feature({"AndroidWebView", "Android-PostMessage"})
-    // TODO(sgurun) enable. This test fails occasionally because the handler that
-    // handles messages in background may not be ready. This logic will be rewritten,
-    // disabling till then.
-    @DisabledTest
+    @SmallTest
+    @Feature({"AndroidWebView", "Android-PostMessage"})
     public void testMessageChannelUsingPendingPort() throws Throwable {
         final ChannelContainer channelContainer = new ChannelContainer();
         loadPage(ECHO_PAGE);
@@ -579,7 +575,7 @@ public class PostMessageTest extends AwTestBase {
                     public void onMessage(String message) {
                         channelContainer.setMessage(message);
                     }
-                });
+                }, null);
                 mAwContents.postMessageToFrame(null, WEBVIEW_MESSAGE, mWebServer.getBaseUrl(),
                         new MessagePort[]{channel[1]});
                 channel[0].postMessage(HELLO, null);
@@ -606,7 +602,7 @@ public class PostMessageTest extends AwTestBase {
                     public void onMessage(String message) {
                         channelContainer.setMessage(message);
                     }
-                });
+                }, null);
                 channel[0].postMessage(HELLO, null);
             }
         });
@@ -676,8 +672,8 @@ public class PostMessageTest extends AwTestBase {
             return mPort.isClosed();
         }
         @Override
-        public void setWebEventHandler(WebEventHandler handler) {
-            mPort.setWebEventHandler(handler);
+        public void setWebEventHandler(WebEventHandler webEventHandler, Handler handler) {
+            mPort.setWebEventHandler(webEventHandler, handler);
         }
         @Override
         public void onMessage(String message) {

android_webview/native/aw_message_port_service_impl.cc

@@ -98,11 +98,11 @@ void AwMessagePortServiceImpl::OnConvertedWebToAppMessage(
   ScopedJavaLocalRef<jstring> jmsg = ConvertUTF16ToJavaString(env, value);
   ScopedJavaLocalRef<jintArray> jports =
       ToJavaIntArray(env, sent_message_port_ids);
-  Java_AwMessagePortService_onPostMessage(env,
-                                          jobj.obj(),
-                                          message_port_id,
-                                          jmsg.obj(),
-                                          jports.obj());
+  Java_AwMessagePortService_onReceivedMessage(env,
+                                              jobj.obj(),
+                                              message_port_id,
+                                              jmsg.obj(),
+                                              jports.obj());
 }
 
 void AwMessagePortServiceImpl::OnMessagePortMessageFilterClosing(