0

[aw] Cleanup default UMA upload QoS experiment

1. This flag has been enabled by default several
   versions ago, and this CL will delete the
   relevant code.

2. Remove my non-@chromium.org account from
   AUTHORS to avoid spam.

Bug: 40282638
Change-Id: Ifb88a3bd4d20198a51faecd7a2080c8a7fb559f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5492979
Commit-Queue: Ho Cheung <hocheung@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1294715}
This commit is contained in:
Ho Cheung
2024-05-01 01:43:52 +00:00
committed by Chromium LUCI CQ
parent 64b1fe4f59
commit d86fc10b53
15 changed files with 33 additions and 103 deletions

@ -512,7 +512,6 @@ Himanshu Joshi <h.joshi@samsung.com>
Himanshu Nayak <himanshu.nayak@amd.corp-partner.google.com>
Hiroki Oshima <hiroki.oshima@gmail.com>
Hiroyuki Matsuda <gsittyz@gmail.com>
Ho Cheung <uioptt24@gmail.com>
Hodol Han <bab6ting@gmail.com>
Holger Kraus <kraush@amazon.com>
Hong Zheng <hong.zheng@intel.com>

@ -31,7 +31,6 @@ const base::Feature* const kFeaturesExposedToJava[] = {
&features::kWebViewXRequestedWithHeaderControl,
&features::kWebViewXRequestedWithHeaderManifestAllowList,
&features::kWebViewRestrictSensitiveContent,
&features::kWebViewUmaUploadQualityOfServiceSetToDefault,
&metrics::kAndroidMetricsAsyncMetricLogging,
&features::kWebViewZoomKeyboardShortcuts,
&features::kWebViewClearFunctorInBackground,

@ -215,12 +215,6 @@ BASE_FEATURE(kWebViewInjectPlatformJsApis,
"WebViewInjectPlatformJsApis",
base::FEATURE_ENABLED_BY_DEFAULT);
// This enables uploading UMA data with a higher frequency.
// This Feature is checked and used in downstream internal code.
BASE_FEATURE(kWebViewUmaUploadQualityOfServiceSetToDefault,
"WebViewUmaUploadQualityOfServiceSetToDefault",
base::FEATURE_ENABLED_BY_DEFAULT);
// Whether to use initial network state during initialization to speed up
// startup.
BASE_FEATURE(kWebViewUseInitialNetworkStateAtStartup,

@ -51,7 +51,6 @@ BASE_DECLARE_FEATURE(kWebViewWideColorGamutSupport);
BASE_DECLARE_FEATURE(kWebViewXRequestedWithHeaderControl);
extern const base::FeatureParam<int> kWebViewXRequestedWithHeaderMode;
BASE_DECLARE_FEATURE(kWebViewXRequestedWithHeaderManifestAllowList);
BASE_DECLARE_FEATURE(kWebViewUmaUploadQualityOfServiceSetToDefault);
BASE_DECLARE_FEATURE(kWebViewUseInitialNetworkStateAtStartup);
BASE_DECLARE_FEATURE(kWebViewZoomKeyboardShortcuts);
BASE_DECLARE_FEATURE(kWebViewReduceUAAndroidVersionDeviceModel);

@ -604,10 +604,6 @@ public final class AwBrowserProcess {
/** Initialize the metrics uploader. */
public static void initializeMetricsLogUploader() {
boolean useDefaultUploadQos =
AwFeatureMap.isEnabled(
AwFeatures.WEBVIEW_UMA_UPLOAD_QUALITY_OF_SERVICE_SET_TO_DEFAULT);
boolean metricServiceEnabledOnlySdkRuntime =
ContextUtils.isSdkSandboxProcess()
&& AwFeatureMap.isEnabled(
@ -618,7 +614,7 @@ public final class AwBrowserProcess {
boolean isAsync =
AwFeatureMap.isEnabled(
AndroidMetricsFeatures.ANDROID_METRICS_ASYNC_METRIC_LOGGING);
AwMetricsLogUploader uploader = new AwMetricsLogUploader(isAsync, useDefaultUploadQos);
AwMetricsLogUploader uploader = new AwMetricsLogUploader(isAsync);
// Open a connection during startup while connecting to other services such as
// ComponentsProviderService and VariationSeedServer to try to avoid spinning the
// nonembedded ":webview_service" twice.
@ -627,7 +623,7 @@ public final class AwBrowserProcess {
} else {
AndroidMetricsLogConsumer directUploader =
data -> {
PlatformServiceBridge.getInstance().logMetrics(data, useDefaultUploadQos);
PlatformServiceBridge.getInstance().logMetrics(data);
return HttpURLConnection.HTTP_OK;
};
AndroidMetricsLogUploader.setConsumer(new MetricsFilteringDecorator(directUploader));

@ -107,16 +107,11 @@ public abstract class PlatformServiceBridge {
// Takes an uncompressed, serialized UMA proto and logs it via a platform-specific mechanism.
public void logMetrics(byte[] data) {}
// TODO(crbug.com/40282638): remove this once downstream lands
public void logMetrics(byte[] data, boolean useDefaultUploadQos) {}
/**
* Similar to {@link logMetrics}, logs a serialized UMA proto via a platform-specific mechanism
* but blocks until the operation finishes.
*
* @param data uncompressed, serialized UMA proto.
* @param useDefaultUploadQos whether to use an experimental change that increases upload
* frequency
* @return Status code of the logging operation. The status codes are:
* - Success cache (went to the devices cache): -1
* - Success: 0
@ -128,14 +123,7 @@ public abstract class PlatformServiceBridge {
*/
public int logMetricsBlocking(byte[] data) {
// TODO(crbug.com/40790308): remove this once downstream implementation lands.
logMetrics(data, true);
return 0;
}
// TODO(crbug.com/40282638): remove this once downstream lands
public int logMetricsBlocking(byte[] data, boolean useDefaultUploadQos) {
// TODO(crbug.com/40790308): remove this once downstream implementation lands.
logMetrics(data, useDefaultUploadQos);
logMetrics(data);
return 0;
}

@ -581,9 +581,6 @@ public final class ProductionSupportedFlagList {
"When enabled, reduces SubresourceResponseStarted IPC by sending"
+ "subresource notifications only if the user has allowed"
+ "HTTPS-related exceptions."),
Flag.baseFeature(
AwFeatures.WEBVIEW_UMA_UPLOAD_QUALITY_OF_SERVICE_SET_TO_DEFAULT,
"If enabled, the frequency to upload UMA is increased."),
Flag.baseFeature("CanvasColorCache"),
Flag.baseFeature(
BlinkFeatures.KEYBOARD_FOCUSABLE_SCROLLERS,

@ -12,11 +12,9 @@ interface IMetricsUploadService {
* Send the given UMA log to the clearcut service in GMS core on the device.
*
* @param serializedLog the serialized bytes of the ChromeUserMetricsExtension proto message.
* @param useDefaultUploadQos whether to use an experimental change that increases upload
* frequency.
*
* @returns an integer HTTP status code indicating the success state of sending the log to the
* platform.
*/
int uploadMetricsLog(in byte[] serializedLog, boolean useDefaultUploadQos);
int uploadMetricsLog(in byte[] serializedLog);
}

@ -43,31 +43,25 @@ public class AwMetricsLogUploader implements AndroidMetricsLogConsumer {
private final AtomicReference<MetricsLogUploaderServiceConnection> mInitialConnection;
private final boolean mIsAsync;
private final boolean mUseDefaultUploadQos;
/**
* @param isAsync Whether logging is happening on a background thread or if it is being called
* from the main thread.
* @param useDefaultUploadQos Used to experiment modifying the QOS upload rate.
* from the main thread.
*/
public AwMetricsLogUploader(boolean isAsync, boolean useDefaultUploadQos) {
public AwMetricsLogUploader(boolean isAsync) {
// A service connection that is used to establish an initial connection to the
// MetricsUploadService to keep it alive until the first metrics log is ready.
mInitialConnection = new AtomicReference();
mIsAsync = isAsync;
mUseDefaultUploadQos = useDefaultUploadQos;
}
// A service connection that sends the given serialized metrics log data to
// MetricsUploadService. It closes the connection after sending the metrics log.
private static class MetricsLogUploaderServiceConnection implements ServiceConnection {
private final boolean mUseDefaultUploadQos;
private final LinkedBlockingQueue<IMetricsUploadService> mConnectionsQueue;
public MetricsLogUploaderServiceConnection(
boolean useDefaultUploadQos,
LinkedBlockingQueue<IMetricsUploadService> connectionsQueue) {
mUseDefaultUploadQos = useDefaultUploadQos;
mConnectionsQueue = connectionsQueue;
}
@ -135,7 +129,7 @@ public class AwMetricsLogUploader implements AndroidMetricsLogConsumer {
return HttpURLConnection.HTTP_CLIENT_TIMEOUT;
}
return uploadService.uploadMetricsLog(data, mUseDefaultUploadQos);
return uploadService.uploadMetricsLog(data);
} catch (RemoteException e) {
Log.d(TAG, "Failed to send serialized metrics data to service", e);
} catch (InterruptedException e) {
@ -166,8 +160,7 @@ public class AwMetricsLogUploader implements AndroidMetricsLogConsumer {
MetricsLogUploaderServiceConnection connection = mInitialConnection.getAndSet(null);
if (connection == null) {
connection =
new MetricsLogUploaderServiceConnection(mUseDefaultUploadQos, connectionsQueue);
connection = new MetricsLogUploaderServiceConnection(connectionsQueue);
if (!connection.bind()) {
Log.w(TAG, "Failed to bind to MetricsUploadService");
@ -182,15 +175,14 @@ public class AwMetricsLogUploader implements AndroidMetricsLogConsumer {
* Initialize a connection to {@link org.chromium.android_webview.services.MetricsUploadService}
* and keep it alive until the first metrics log data is sent for upload.
*
* We do this because we already pay the startup cost of the non-embedded process due to other
* webview non-embedded services running early on.
* We can hopefully save some time on initially spinning the process since we know we
* are going to attempt to upload pretty soon after starting up WebView the first time.
* <p>We do this because we already pay the startup cost of the non-embedded process due to
* other webview non-embedded services running early on. We can hopefully save some time on
* initially spinning the process since we know we are going to attempt to upload pretty soon
* after starting up WebView the first time.
*/
public void initialize() {
MetricsLogUploaderServiceConnection connection =
new MetricsLogUploaderServiceConnection(
mUseDefaultUploadQos, new LinkedBlockingQueue(1));
new MetricsLogUploaderServiceConnection(new LinkedBlockingQueue(1));
if (connection.bind()) {
mInitialConnection.set(connection);
} else {

@ -102,7 +102,7 @@ public class AwMetricsIntegrationTest extends AwParameterizedTest {
// outside the scope of these integeration tests.
AndroidMetricsLogConsumer directUploader =
data -> {
PlatformServiceBridge.getInstance().logMetrics(data, true);
PlatformServiceBridge.getInstance().logMetrics(data);
return HttpURLConnection.HTTP_OK;
};
AndroidMetricsLogUploader.setConsumer(

@ -58,9 +58,7 @@ public class AwMetricsLogUploaderTest {
@Test
public void testSendingData_withPreBinding() throws Throwable {
AwMetricsLogUploader uploader =
new AwMetricsLogUploader(
/* waitForResults= */ true, /* useDefaultUploadQos= */ false);
AwMetricsLogUploader uploader = new AwMetricsLogUploader(/* waitForResults= */ true);
uploader.initialize();
int status = uploader.log(SAMPLE_TEST_METRICS_LOG.toByteArray());
Assert.assertEquals(HttpURLConnection.HTTP_OK, status);
@ -71,31 +69,13 @@ public class AwMetricsLogUploaderTest {
@Test
public void testSendingData_withoutPreBinding() throws Throwable {
AwMetricsLogUploader uploader =
new AwMetricsLogUploader(
/* waitForResults= */ true, /* useDefaultUploadQos= */ false);
AwMetricsLogUploader uploader = new AwMetricsLogUploader(/* waitForResults= */ true);
int status = uploader.log(SAMPLE_TEST_METRICS_LOG.toByteArray());
Assert.assertEquals(HttpURLConnection.HTTP_OK, status);
ChromeUserMetricsExtension receivedLog = mPlatformServiceBridge.waitForNextMetricsLog();
Assert.assertEquals(SAMPLE_TEST_METRICS_LOG, receivedLog);
}
@Test
public void testSendingData_setsDefaultUploadQos() throws Throwable {
testDefaultUploadQosFlag(true);
testDefaultUploadQosFlag(false);
}
private void testDefaultUploadQosFlag(boolean expectedValue) throws Throwable {
AwMetricsLogUploader uploader =
new AwMetricsLogUploader(
/* waitForResults= */ true, /* useDefaultUploadQos= */ expectedValue);
uploader.log(SAMPLE_TEST_METRICS_LOG.toByteArray());
mPlatformServiceBridge.waitForNextMetricsLog();
Assert.assertEquals(expectedValue, mPlatformServiceBridge.isLastLogUsingDefaultUploadQos());
}
@Test
public void testSendingDataException_whenInterruptedException() throws Throwable {
testSendingDataException(new InterruptedException(), HttpURLConnection.HTTP_UNAVAILABLE);
@ -109,9 +89,7 @@ public class AwMetricsLogUploaderTest {
mock(LinkedBlockingQueue.class);
when(mockedResultsQueue.poll(anyLong(), any())).thenReturn(null);
AwMetricsLogUploader uploader =
new AwMetricsLogUploader(
/* waitForResults= */ true, /* useDefaultUploadQos= */ false);
AwMetricsLogUploader uploader = new AwMetricsLogUploader(/* waitForResults= */ true);
int status = uploader.log(SAMPLE_TEST_METRICS_LOG.toByteArray(), mockedResultsQueue);
Assert.assertEquals(HttpURLConnection.HTTP_CLIENT_TIMEOUT, status);
@ -129,9 +107,7 @@ public class AwMetricsLogUploaderTest {
throw exceptionThrown;
});
AwMetricsLogUploader uploader =
new AwMetricsLogUploader(
/* waitForResults= */ true, /* useDefaultUploadQos= */ false);
AwMetricsLogUploader uploader = new AwMetricsLogUploader(/* waitForResults= */ true);
int status = uploader.log(SAMPLE_TEST_METRICS_LOG.toByteArray(), mockedResultsQueue);
Assert.assertEquals(expectedStatus, status);
@ -144,18 +120,14 @@ public class AwMetricsLogUploaderTest {
// We should still get a success code since this code is not waiting for this.
mPlatformServiceBridge.setLogMetricsBlockingStatus(1);
AwMetricsLogUploader uploader =
new AwMetricsLogUploader(
/* waitForResults= */ false, /* useDefaultUploadQos= */ false);
AwMetricsLogUploader uploader = new AwMetricsLogUploader(/* waitForResults= */ false);
int status = uploader.log(SAMPLE_TEST_METRICS_LOG.toByteArray());
Assert.assertEquals(HttpURLConnection.HTTP_OK, status);
}
@Test
public void testSendingMultipleLogs() throws Throwable {
AwMetricsLogUploader uploader =
new AwMetricsLogUploader(
/* waitForResults= */ true, /* useDefaultUploadQos= */ false);
AwMetricsLogUploader uploader = new AwMetricsLogUploader(/* waitForResults= */ true);
uploader.initialize();
final int numberOfLogs = 5;

@ -51,7 +51,7 @@ public class MetricsFilteringDecoratorTest extends AwParameterizedTest {
AndroidMetricsLogConsumer directUploader =
data -> {
PlatformServiceBridge.getInstance().logMetrics(data, true);
PlatformServiceBridge.getInstance().logMetrics(data);
return HttpURLConnection.HTTP_OK;
};
mUploader = new MetricsFilteringDecorator(directUploader);

@ -14,11 +14,12 @@ import org.chromium.components.metrics.ChromeUserMetricsExtensionProtos.ChromeUs
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue;
/** An implementation of PlatformServiceBridge that can be used to wait for metrics logs in tests. */
/**
* An implementation of PlatformServiceBridge that can be used to wait for metrics logs in tests.
*/
public class MetricsTestPlatformServiceBridge extends PlatformServiceBridge {
private final BlockingQueue<byte[]> mQueue;
private int mStatus;
private boolean mUseDefaultUploadQos;
public MetricsTestPlatformServiceBridge() {
mQueue = new LinkedBlockingQueue<>();
@ -38,23 +39,19 @@ public class MetricsTestPlatformServiceBridge extends PlatformServiceBridge {
}
@Override
public void logMetrics(byte[] data, boolean useDefaultUploadQos) {
public void logMetrics(byte[] data) {
mQueue.add(data);
mUseDefaultUploadQos = useDefaultUploadQos;
}
@Override
public int logMetricsBlocking(byte[] data, boolean useDefaultUploadQos) {
logMetrics(data, useDefaultUploadQos);
mUseDefaultUploadQos = useDefaultUploadQos;
public int logMetricsBlocking(byte[] data) {
logMetrics(data);
return mStatus;
}
public boolean isLastLogUsingDefaultUploadQos() {
return mUseDefaultUploadQos;
}
/** This method can be used to set the status code to return from the {@link logMetricsBlocking}. */
/**
* This method can be used to set the status code to return from the {@link logMetricsBlocking}.
*/
public void setLogMetricsBlockingStatus(int status) {
mStatus = status;
}

@ -98,7 +98,7 @@ public class MetricsUploadServiceTest {
new ServiceConnectionHelper(intent, Context.BIND_AUTO_CREATE)) {
IMetricsUploadService service =
IMetricsUploadService.Stub.asInterface(helper.getBinder());
int returnedStatus = service.uploadMetricsLog(mMetricsLog.toByteArray(), false);
int returnedStatus = service.uploadMetricsLog(mMetricsLog.toByteArray());
Assert.assertEquals(expectedHttpStatusCode, returnedStatus);
}

@ -23,10 +23,9 @@ public class MetricsUploadService extends Service {
private final IMetricsUploadService.Stub mBinder =
new IMetricsUploadService.Stub() {
@Override
public int uploadMetricsLog(byte[] serializedLog, boolean useDefaultUploadQos) {
public int uploadMetricsLog(byte[] serializedLog) {
int status =
PlatformServiceBridge.getInstance()
.logMetricsBlocking(serializedLog, useDefaultUploadQos);
PlatformServiceBridge.getInstance().logMetricsBlocking(serializedLog);
// We map the platform reported statuses out to HTTP status codes so that
// Chromium metrics can work with a single standard.