Skip to content

Commit

Permalink
Revert "[WebView] Refactor AppWebMessagePort.java to a native wrapper"
Browse files Browse the repository at this point in the history 
This reverts commit 57e4ed8.

Reason for revert: https://issuetracker.google.com/245837736 WebView Beta WebMessageChannel stops working after 100 - 1000 messages

Original change's description:
> [WebView] Refactor AppWebMessagePort.java to a native wrapper
>
> 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}

Bug: 1023334
Bug: 1324599
Change-Id: I026e80d4c7bf81dd004146c2037bd90f5126b934
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3894143
Owners-Override: Krishna Govind <govind@chromium.org>
Reviewed-by: Krishna Govind <govind@chromium.org>
Commit-Queue: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/5195_124@{#3}
Cr-Branched-From: bd283fb-refs/branch-heads/5195@{#1106}
Cr-Branched-From: 7aa3f07-refs/heads/main@{#1027018}
  • Loading branch information
Abhijith Nair authored and Krishna Govind committed Sep 14, 2022
1 parent 376c907 commit 1807ecb
Show file tree
Hide file tree
Showing 20 changed files with 872 additions and 759 deletions.
7 changes: 4 additions & 3 deletions android_webview/browser/js_java_interaction/aw_web_message_host_factory.cc
View file
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/message_port_helper.h"
#include "content/public/browser/android/app_web_message_port.h"

namespace android_webview {
namespace {
Expand All @@ -37,8 +37,9 @@ class AwWebMessageHost : public js_injection::WebMessageHost {
void OnPostMessage(
std::unique_ptr<js_injection::WebMessage> message) override {
JNIEnv* env = base::android::AttachCurrentThread();
base::android::ScopedJavaLocalRef<jobjectArray> jports =
content::android::CreateJavaMessagePort(std::move(message->ports));
base::android::ScopedJavaGlobalRef<jobjectArray> jports =
content::AppWebMessagePort::WrapJavaArray(env,
std::move(message->ports));
Java_WebMessageListenerHolder_onPostMessage(
env, listener_,
base::android::ConvertUTF16ToJavaString(env, message->message),
Expand Down
176 changes: 0 additions & 176 deletions android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,30 +24,22 @@

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 @@ -977,172 +969,4 @@ 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: 1 addition & 3 deletions content/browser/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2293,7 +2293,6 @@ 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 @@ -2314,6 +2313,7 @@ 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 @@ -2882,8 +2882,6 @@ 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

0 comments on commit 1807ecb

Please sign in to comment.