0

Reland "WebView: Implement request monitor in EmbeddedTestServerAndroid"

This reverts commit 369c5093d8.

Reason for revert (reland):
The original CL was reverted due to compile failures on the Cronet bot.
This is because the code generated by AIDL used a language feature that
requires SDK version 24 for Map<String, String>, but the Cronet requires
SDK version 23 as the minimum version (https://crrev.com/c/5913439). To
avoid the restriction, this CL encodes the headers into String[] that
should be available even in SDK version 23 instead of Map.

Original change's description:
> Revert "WebView: Implement request monitor in EmbeddedTestServerAndroid"
>
> This reverts commit 3d9eb93fb1.
>
> Reason for revert:
> This CL breaks the android-cronet-arm-dbg build:
> https://ci.chromium.org/b/8729401193435602897
>
> Original change's description:
> > WebView: Implement request monitor in EmbeddedTestServerAndroid
> >
> > This CL implements the request monitor to capture request headers in
> > EmbeddedTestServerAndroid. The headers are exposed to Java, so that
> > WebView tests can check if requests have appropriate headers.
> >
> > As an example usage, this CL uses the mechanism in a prerendering test.
> > The test checks if prerendering navigation has the Sec-Purpose header.
> >
> > Change-Id: Iec9843917fc4ac3cafa15b71304ba92352502635
> > Bug: 41490450
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6072982
> > Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
> > Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
> > Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1392138}
>
> Bug: 41490450
> Change-Id: Id50f9bc047209bd593a015373b93e7bbceaf3b0c
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6072588
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Auto-Submit: Hiroki Nakagawa <nhiroki@chromium.org>
> Owners-Override: Yifan Luo <lyf@chromium.org>
> Commit-Queue: Yifan Luo <lyf@chromium.org>
> Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1392197}

Bug: 41490450
Change-Id: Ib189894a9df2e5923564fe81fc95129db0ce2b94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6072608
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1392770}
This commit is contained in:
Hiroki Nakagawa
2024-12-06 09:38:46 +00:00
committed by Chromium LUCI CQ
parent d547e68c2b
commit 074e1ef34f
6 changed files with 128 additions and 8 deletions
android_webview/javatests/src/org/chromium/android_webview/test
net/test

@@ -402,6 +402,10 @@ public class AwPrerenderTest extends AwParameterizedTest {
Assert.assertEquals(onPageStartedHelper.getCallCount(), 1);
Assert.assertEquals(onPageStartedHelper.getUrl(), mPageUrl);
// Make sure that prerendering navigation has the Sec-Purpose header.
HashMap<String, String> headers = mTestServer.getRequestHeadersForUrl(PRERENDER_URL);
Assert.assertEquals("prefetch;prerender", headers.get("Sec-Purpose"));
activatePage(mPrerenderingUrl, ActivationBy.LOAD_URL);
// Wait until the navigation activates the prerendered page.

@@ -24,11 +24,13 @@ import org.chromium.net.X509Util;
import org.chromium.net.test.util.CertTestUtil;
import java.io.File;
import java.util.HashMap;
/**
* A simple file server for java tests.
*
* An example use:
*
* <pre>
* EmbeddedTestServer s = EmbeddedTestServer.createAndStartServer(context);
*
@@ -470,12 +472,12 @@ public class EmbeddedTestServer {
}
}
/** Get the full URLs for the given relative URLs.
/**
* Get the full URLs for the given relative URLs.
*
* @see #getURL(String)
*
* @param relativeUrls The relative URLs for which full URLs will be obtained.
* @return The URLs as a String array.
* @see #getURL(String)
* @param relativeUrls The relative URLs for which full URLs will be obtained.
* @return The URLs as a String array.
*/
public String[] getURLs(String... relativeUrls) {
String[] absoluteUrls = new String[relativeUrls.length];
@@ -485,10 +487,37 @@ public class EmbeddedTestServer {
return absoluteUrls;
}
/**
* Get the request headers observed on the server for the given relative URL.
*
* @param relativeUrl The relative URL for which request headers should be returned.
* @return The map of the header key and value pairs.
*/
public HashMap<String, String> getRequestHeadersForUrl(final String relativeUrl) {
try {
synchronized (mImplMonitor) {
checkServiceLocked();
String[] headers_array = mImpl.getRequestHeadersForUrl(relativeUrl);
// The length of the vector should be even, as the vector alternates between header
// names (even indices) and their corresponding values (odd indices).
Assert.assertEquals(0, headers_array.length % 2);
HashMap<String, String> headers = new HashMap<String, String>();
for (int i = 0; i < headers_array.length; i += 2) {
headers.put(headers_array[i], headers_array[i + 1]);
}
return headers;
}
} catch (RemoteException e) {
throw new EmbeddedTestServerFailure(
"Failed to get request headers for " + relativeUrl, e);
}
}
/**
* Stop and destroy the server.
*
* This handles stopping the server and destroying the native object.
* This handles stopping the server and destroying the native object.
*/
public void stopAndDestroyServer() {
synchronized (mImplMonitor) {

@@ -12,6 +12,7 @@ import android.os.RemoteException;
import org.jni_zero.CalledByNative;
import org.jni_zero.JNINamespace;
import org.jni_zero.JniType;
import org.jni_zero.NativeMethods;
import org.chromium.base.ContextUtils;
@@ -257,9 +258,29 @@ public class EmbeddedTestServerImpl extends IEmbeddedTestServerImpl.Stub {
});
}
/** Shut down the server.
/**
* Get the request headers observed on the server for the given relative URL.
*
* @return Whether the server was successfully shut down.
* @param relativeUrl The relative URL for which request headers should be returned.
* @return The vector alternates between header names (even indices) and their corresponding
* values (odd indices).
*/
@Override
public String[] getRequestHeadersForUrl(final String relativeUrl) {
return runOnHandlerThread(
new Callable<String[]>() {
@Override
public String[] call() {
return EmbeddedTestServerImplJni.get()
.getRequestHeadersForUrl(mNativeEmbeddedTestServer, relativeUrl);
}
});
}
/**
* Shut down the server.
*
* @return Whether the server was successfully shut down.
*/
@Override
public boolean shutdownAndWaitUntilComplete() {
@@ -362,5 +383,8 @@ public class EmbeddedTestServerImpl extends IEmbeddedTestServerImpl.Stub {
long nativeEmbeddedTestServerAndroid, String hostName, String relativeUrl);
void serveFilesFromDirectory(long nativeEmbeddedTestServerAndroid, String directoryPath);
@JniType("std::vector<std::string>")
String[] getRequestHeadersForUrl(long nativeEmbeddedTestServerAndroid, String relativeUrl);
}
}

@@ -60,6 +60,21 @@ interface IEmbeddedTestServerImpl {
*/
String getURLWithHostName(String hostName, String relativeUrl);
/** Get the request headers observed on the server for the given relative URL.
*
* @param relativeUrl The relative URL for which request headers should be returned.
* @return The vector alternates between header names (even indices) and their corresponding
* values (odd indices).
*
* Ideally the returned type should be map but it is not available due to the required SDK
* version in Cronet. The generated code for the map requires SDK 24, but
* the Cronet requires SDK 23 as the minimum version (https://crrev.com/c/5913439).
*
* Note that the HTTP spec allows to have multiple headers for the same name, but the returned
* vector only contains one of them. See https://crbug.com/382316472 for details.
*/
String[] getRequestHeadersForUrl(String relativeUrl);
/** Shut down the server.
*
* @return Whether the server was successfully shut down.

@@ -54,6 +54,11 @@ EmbeddedTestServerAndroid::EmbeddedTestServerAndroid(
test_server_.SetConnectionListener(&connection_listener_);
Java_EmbeddedTestServerImpl_setNativePtr(env, jobj,
reinterpret_cast<intptr_t>(this));
// Register the request monitor to capture request headers.
test_server_.RegisterRequestMonitor(
base::BindRepeating(&EmbeddedTestServerAndroid::MonitorResourceRequest,
base::Unretained(this)));
}
EmbeddedTestServerAndroid::~EmbeddedTestServerAndroid() {
@@ -93,6 +98,26 @@ ScopedJavaLocalRef<jstring> EmbeddedTestServerAndroid::GetURLWithHostName(
return base::android::ConvertUTF8ToJavaString(env, gurl.spec());
}
std::vector<std::string> EmbeddedTestServerAndroid::GetRequestHeadersForUrl(
JNIEnv* env,
const base::android::JavaParamRef<jstring>& jrelative_url) {
base::AutoLock auto_lock(lock_);
std::string path = base::android::ConvertJavaStringToUTF8(env, jrelative_url);
CHECK(request_headers_by_path_.contains(path)) << path;
// Copy headers from HttpRequest::HeaderMap to std::vector for passing them to
// Java. For the required SDK version in Cronet, std::map is not available
// (see the comment in IEmbeddedTestServerImpl.aidl for details). The vector
// alternates between header names (even indices) and their corresponding
// values (odd indices).
std::vector<std::string> headers;
for (auto [key, value] : request_headers_by_path_[path]) {
headers.push_back(key);
headers.push_back(value);
}
return headers;
}
void EmbeddedTestServerAndroid::AddDefaultHandlers(
JNIEnv* env,
const JavaParamRef<jstring>& jdirectory_path) {
@@ -156,4 +181,11 @@ static void JNI_EmbeddedTestServerImpl_Init(
new EmbeddedTestServerAndroid(env, jobj, jhttps);
}
void EmbeddedTestServerAndroid::MonitorResourceRequest(
const net::test_server::HttpRequest& request) {
base::AutoLock auto_lock(lock_);
request_headers_by_path_.emplace(request.GetURL().PathForRequest(),
request.headers);
}
} // namespace net::test_server

@@ -10,6 +10,7 @@
#include "base/android/jni_weak_ref.h"
#include "base/android/scoped_java_ref.h"
#include "base/memory/raw_ptr.h"
#include "base/synchronization/lock.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/embedded_test_server_connection_listener.h"
#include "net/test/embedded_test_server/http_request.h"
@@ -48,6 +49,10 @@ class EmbeddedTestServerAndroid {
const base::android::JavaParamRef<jstring>& jhostname,
const base::android::JavaParamRef<jstring>& jrelative_url) const;
std::vector<std::string> GetRequestHeadersForUrl(
JNIEnv* env,
const base::android::JavaParamRef<jstring>& jrelative_url);
void AddDefaultHandlers(
JNIEnv* jenv,
const base::android::JavaParamRef<jstring>& jdirectory_path);
@@ -81,10 +86,21 @@ class EmbeddedTestServerAndroid {
void AcceptedSocket(const void* socket_id);
void ReadFromSocket(const void* socket_id);
void MonitorResourceRequest(const net::test_server::HttpRequest& request);
JavaObjectWeakGlobalRef weak_java_server_;
EmbeddedTestServer test_server_;
ConnectionListener connection_listener_;
// Headers of requests sent to the server. Keyed by path (not by full URL)
// because the host part of the requests is translated ("a.test" to
// "127.0.0.1") before the server handles them.
// This is accessed from the UI thread and `EmbeddedTestServer::io_thread_`,
// so it's guarded by the lock.
std::map<std::string, net::test_server::HttpRequest::HeaderMap>
request_headers_by_path_ GUARDED_BY(lock_);
base::Lock lock_;
};
} // namespace net::test_server