Skip to content

Commit

Permalink
[WebView] Refactor AppWebMessagePort.java to a native wrapper
Browse files Browse the repository at this point in the history
This CL move the send/receive message over MessagePort logic from Java
to native code, and remove useless MessagePortDescriptor in Java.
The public API of AppWebMessagePort is not changed.

https://chromium-review.googlesource.com/c/chromium/src/+/3585178
is related, this CL will make encode/decode data in a consistent way.

Change-Id: Ie53c6b79386429ae49264927fd9e955c3d433a53
Bug: 1023334
Bug: 1324599
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3646943
Commit-Queue: Linyue He <linyhe@microsoft.com>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1022416}
  • Loading branch information
richard1122 authored and Chromium LUCI CQ committed Jul 9, 2022
1 parent a7dbf1d commit 57e4ed8
Show file tree
Hide file tree
Showing 20 changed files with 759 additions and 872 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include "components/js_injection/browser/web_message.h"
#include "components/js_injection/browser/web_message_host.h"
#include "components/js_injection/common/origin_matcher.h"
#include "content/public/browser/android/app_web_message_port.h"
#include "content/public/browser/android/message_port_helper.h"

namespace android_webview {
namespace {
Expand All @@ -37,9 +37,8 @@ class AwWebMessageHost : public js_injection::WebMessageHost {
void OnPostMessage(
std::unique_ptr<js_injection::WebMessage> message) override {
JNIEnv* env = base::android::AttachCurrentThread();
base::android::ScopedJavaGlobalRef<jobjectArray> jports =
content::AppWebMessagePort::WrapJavaArray(env,
std::move(message->ports));
base::android::ScopedJavaLocalRef<jobjectArray> jports =
content::android::CreateJavaMessagePort(std::move(message->ports));
Java_WebMessageListenerHolder_onPostMessage(
env, listener_,
base::android::ConvertUTF16ToJavaString(env, message->message),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,30 @@

import org.chromium.android_webview.AwContents;
import org.chromium.android_webview.test.util.CommonResources;
import org.chromium.base.ThreadUtils;
import org.chromium.base.task.PostTask;
import org.chromium.base.test.util.Batch;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.Criteria;
import org.chromium.base.test.util.CriteriaHelper;
import org.chromium.base.test.util.Feature;
import org.chromium.content_public.browser.MessagePayload;
import org.chromium.content_public.browser.MessagePort;
import org.chromium.content_public.browser.UiThreadTaskTraits;
import org.chromium.content_public.browser.test.util.TestCallbackHelperContainer.OnPageFinishedHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.net.test.util.TestWebServer;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicReference;

/**
* The tests for content postMessage API.
*/
@Batch(Batch.PER_CLASS)
@RunWith(AwJUnit4ClassRunner.class)
public class PostMessageTest {
@Rule
Expand Down Expand Up @@ -969,4 +977,172 @@ public void testPostMessageBeforePageLoadWontBlockNavigation() throws Throwable
Assert.assertEquals(WEBVIEW_MESSAGE, data.mMessage);
Assert.assertEquals(SOURCE_ORIGIN, data.mOrigin);
}

@Test
@SmallTest
@Feature({"AndroidWebView", "Android-PostMessage"})
public void testMessagePortLifecycle() throws Throwable {
final String baseUrl = mWebServer.getBaseUrl();
loadPage(TEST_PAGE);
InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
final MessagePort[] ports = mAwContents.createMessageChannel();
Assert.assertFalse(ports[0].isTransferred());
Assert.assertFalse(ports[0].isClosed());
Assert.assertFalse(ports[0].isStarted());
Assert.assertFalse(ports[1].isTransferred());
Assert.assertFalse(ports[1].isClosed());
Assert.assertFalse(ports[1].isStarted());

// Post port1 to main frame.
mAwContents.postMessageToMainFrame(
new MessagePayload("1"), baseUrl, new MessagePort[] {ports[1]});
Assert.assertTrue(ports[1].isTransferred());
Assert.assertFalse(ports[1].isClosed());
Assert.assertFalse(ports[1].isStarted());

// Close one port.
ports[0].close();
Assert.assertFalse(ports[0].isTransferred());
Assert.assertTrue(ports[0].isClosed());
Assert.assertFalse(ports[0].isStarted());
});
}

private static final String COUNT_PORT_FROM_MESSAGE = "<!DOCTYPE html><html><body>"
+ " <script>"
+ " var counter = 0;"
+ " var received = '';"
+ " onmessage = function (e) {"
+ " e.ports[0].onmessage = function(e) {"
+ " received += e.data;"
+ " counter += e.ports.length;"
+ " document.title = received + counter;"
+ " e.ports[0].postMessage(received + counter);"
+ " };"
+ " };"
+ " </script>"
+ "</body></html>";

@Test
@SmallTest
@Feature({"AndroidWebView", "Android-PostMessage"})
// Previously postMessage can be called on any thread, but no tests or CTS tests checked.
public void testTransferPortOnAnotherThread() throws Throwable {
loadPage(COUNT_PORT_FROM_MESSAGE);
final ChannelContainer container = new ChannelContainer();
final MessagePort[] ports =
TestThreadUtils.runOnUiThreadBlocking(() -> mAwContents.createMessageChannel());

TestThreadUtils.runOnUiThreadBlocking(() -> {
mAwContents.postMessageToMainFrame(
new MessagePayload(""), "*", new MessagePort[] {ports[1]});
});
final MessagePort[] ports2 =
TestThreadUtils.runOnUiThreadBlocking(() -> mAwContents.createMessageChannel());
ports2[0].setMessageCallback((messagePayload, sentPorts) -> {
ThreadUtils.checkUiThread();
container.notifyCalled(messagePayload);
}, null);
ports[0].postMessage(new MessagePayload(HELLO), new MessagePort[] {ports2[1]});
expectTitle(HELLO + "1");
Assert.assertEquals(HELLO + "1", container.waitForMessageCallback().getStringValue());
ports[0].close();
ports2[0].close();
}

@Test
@SmallTest
@Feature({"AndroidWebView", "Android-PostMessage"})
public void testTransferPortImmediateAfterPostMessageOnAnotherThread() throws Throwable {
loadPage(COUNT_PORT_FROM_MESSAGE);
final MessagePort[] ports =
TestThreadUtils.runOnUiThreadBlocking(() -> mAwContents.createMessageChannel());

TestThreadUtils.runOnUiThreadBlocking(() -> {
mAwContents.postMessageToMainFrame(
new MessagePayload(""), "*", new MessagePort[] {ports[1]});
});
final CallbackHelper callbackHelper = new CallbackHelper();
final AtomicReference<IllegalStateException> exceptionRef = new AtomicReference<>();
PostTask.postTask(UiThreadTaskTraits.DEFAULT, () -> {
try {
callbackHelper.waitForCallback(0);
mAwContents.postMessageToMainFrame(
new MessagePayload(HELLO), "*", new MessagePort[] {ports[0]});
} catch (TimeoutException ignored) {
} catch (IllegalStateException e) {
exceptionRef.set(e);
callbackHelper.notifyCalled();
}
});
ports[0].postMessage(new MessagePayload(HELLO), null);
callbackHelper.notifyCalled();

callbackHelper.waitForCallback(1);
Assert.assertEquals("Port is already started", exceptionRef.get().getMessage());
}

@Test
@SmallTest
@Feature({"AndroidWebView", "Android-PostMessage"})
public void testCloseMessagePortOnAnotherThread() throws Throwable {
final MessagePort[] messagePorts = new MessagePort[1];
InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
final MessagePort[] ports = mAwContents.createMessageChannel();
messagePorts[0] = ports[0];
// Move message port into |receiving| state.
messagePorts[0].setMessageCallback((messagePayload, sentPorts) -> {}, null);
});
// Close message channel on another thread, simulate the case where the "finalize" is called
// on finalizer thread.
messagePorts[0].close();
}

@Test
@SmallTest
@Feature({"AndroidWebView", "Android-PostMessage"})
public void testTransferPortInAnotherThreadRaceCondition() throws Throwable {
loadPage(COUNT_PORT_FROM_MESSAGE);
final MessagePort[] ports =
TestThreadUtils.runOnUiThreadBlocking(() -> mAwContents.createMessageChannel());
TestThreadUtils.runOnUiThreadBlocking(() -> {
mAwContents.postMessageToMainFrame(
new MessagePayload(""), "*", new MessagePort[] {ports[1]});
});
final MessagePort[] portsToTransfer =
TestThreadUtils.runOnUiThreadBlocking(() -> mAwContents.createMessageChannel());
// Transfer the port in another thread.
ports[0].postMessage(new MessagePayload("test"), new MessagePort[] {portsToTransfer[0]});
// Check port2[0] is transferred right now.
Assert.assertTrue(portsToTransfer[0].isTransferred());
// Set callback on the just transferred port right now. It should fail.
try {
portsToTransfer[0].setMessageCallback((messagePayload, sentPorts) -> {}, null);
Assert.fail("Port transferred, should not able to listen on");
} catch (IllegalStateException e) {
// Ignored.
}
}

@Test
@SmallTest
@Feature({"AndroidWebView", "Android-PostMessage"})
public void testSetReceiverAfterMessageReceived() throws Throwable {
loadPage(COUNT_PORT_FROM_MESSAGE);
final ChannelContainer container = new ChannelContainer();
final HandlerThread thread = new HandlerThread("test-thread");
thread.start();
final Handler handler = new Handler(thread.getLooper());
final MessagePort[] ports =
TestThreadUtils.runOnUiThreadBlocking(() -> mAwContents.createMessageChannel());

InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
// Post message before set callback
ports[0].postMessage(new MessagePayload("msg1"), null);
});
ports[1].setMessageCallback((messagePayload, sentPorts) -> {
container.notifyCalled(messagePayload);
}, handler);
Assert.assertEquals("msg1", container.waitForMessageCallback().getStringValue());
}
}
4 changes: 3 additions & 1 deletion content/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2286,6 +2286,7 @@ source_set("browser") {
"android/android_overlay_provider_impl.cc",
"android/android_overlay_provider_impl.h",
"android/app_web_message_port.cc",
"android/app_web_message_port.h",
"android/background_sync_network_observer_android.cc",
"android/background_sync_network_observer_android.h",
"android/battery_metrics.cc",
Expand All @@ -2306,7 +2307,6 @@ source_set("browser") {
"android/java_interfaces_impl.h",
"android/launcher_thread.cc",
"android/launcher_thread.h",
"android/message_port_descriptor.cc",
"android/scoped_surface_request_manager.cc",
"android/scoped_surface_request_manager.h",
"android/text_suggestion_host_android.cc",
Expand Down Expand Up @@ -2883,6 +2883,8 @@ source_set("browser") {
"android/javascript_injector.cc",
"android/javascript_injector.h",
"android/load_url_params.cc",
"android/message_payload.cc",
"android/message_payload.h",
"android/navigation_handle_proxy.cc",
"android/navigation_handle_proxy.h",
"android/nfc_host.cc",
Expand Down
Loading

0 comments on commit 57e4ed8

Please sign in to comment.