Skip to content

Commit

Permalink
Add a synthetic field trial for the WebView APK type
Browse files Browse the repository at this point in the history
I needed to fix the IsolatedSplitsSynthetic trial to only log for
monochrome, and thought it might be useful to have this as well. This
will let us filter UMA data by the WebView APK type, and can help us
answer the age old question of how many users have standalone WebView on
N+.

Bug: 1150162
Change-Id: I4cc97632f514b0c54b07e205c33dd1c1d3703071
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2618625
Reviewed-by: Richard Coles <[email protected]>
Commit-Queue: Clark DuVall <[email protected]>
Cr-Commit-Position: refs/heads/master@{#842113}
  • Loading branch information
clarkduvall authored and Chromium LUCI CQ committed Jan 11, 2021
1 parent d0f8811 commit 7b0a540
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 10 deletions.
2 changes: 2 additions & 0 deletions android_webview/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import("//components/spellcheck/spellcheck_build_features.gni")

java_cpp_enum("browser_enums") {
sources = [
"aw_apk_type.h",
"aw_renderer_priority.h",
"aw_settings.h",
"permission/aw_permission_request.h",
Expand All @@ -19,6 +20,7 @@ source_set("browser") {
sources = [
"android_protocol_handler.cc",
"android_protocol_handler.h",
"aw_apk_type.h",
"aw_autofill_client.cc",
"aw_autofill_client.h",
"aw_browser_context.cc",
Expand Down
15 changes: 15 additions & 0 deletions android_webview/browser/aw_apk_type.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef ANDROID_WEBVIEW_BROWSER_AW_APK_TYPE_H_
#define ANDROID_WEBVIEW_BROWSER_AW_APK_TYPE_H_

namespace android_webview {

// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.android_webview
enum class ApkType { STANDALONE = 0, MONOCHROME = 1, TRICHROME = 2 };

} // namespace android_webview

#endif // ANDROID_WEBVIEW_BROWSER_AW_APK_TYPE_H_
22 changes: 20 additions & 2 deletions android_webview/browser/aw_browser_main_parts.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,29 @@ void AwBrowserMainParts::RegisterSyntheticTrials() {
metrics->synthetic_trial_registry()->AddSyntheticTrialObserver(
variations::SyntheticTrialsActiveGroupIdProvider::GetInstance());

static constexpr char kWebViewApkTypeTrial[] = "WebViewApkType";
ApkType apk_type = AwBrowserProcess::GetApkType();
std::string apk_type_string;
switch (apk_type) {
case ApkType::TRICHROME:
apk_type_string = "Trichrome";
break;
case ApkType::MONOCHROME:
apk_type_string = "Monochrome";
break;
case ApkType::STANDALONE:
apk_type_string = "Standalone";
break;
}
AwMetricsServiceAccessor::RegisterSyntheticFieldTrial(
metrics, kWebViewApkTypeTrial, apk_type_string);

// If isolated splits are enabled at build time, Monochrome and Trichrome will
// have a different bundle layout, so measure N+ even though isolated splits
// are only supported by Android in O+.
if (base::android::BuildInfo::GetInstance()->sdk_int() >=
base::android::SDK_VERSION_NOUGAT) {
if (apk_type == ApkType::MONOCHROME &&
base::android::BuildInfo::GetInstance()->sdk_int() >=
base::android::SDK_VERSION_NOUGAT) {
static constexpr char kIsolatedSplitsTrial[] = "IsolatedSplitsSynthetic";
AwMetricsServiceAccessor::RegisterSyntheticFieldTrial(
metrics, kIsolatedSplitsTrial,
Expand Down
6 changes: 6 additions & 0 deletions android_webview/browser/aw_browser_process.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,12 @@ void AwBrowserProcess::TriggerMinidumpUploading() {
base::android::AttachCurrentThread());
}

// static
ApkType AwBrowserProcess::GetApkType() {
return static_cast<ApkType>(
Java_AwBrowserProcess_getApkType(base::android::AttachCurrentThread()));
}

static void JNI_AwBrowserProcess_SetProcessNameCrashKey(
JNIEnv* env,
const base::android::JavaParamRef<jstring>& processName) {
Expand Down
2 changes: 2 additions & 0 deletions android_webview/browser/aw_browser_process.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#ifndef ANDROID_WEBVIEW_BROWSER_AW_BROWSER_PROCESS_H_
#define ANDROID_WEBVIEW_BROWSER_AW_BROWSER_PROCESS_H_

#include "android_webview/browser/aw_apk_type.h"
#include "android_webview/browser/aw_browser_context.h"
#include "android_webview/browser/aw_feature_list_creator.h"
#include "android_webview/browser/lifecycle/aw_contents_lifecycle_notifier.h"
Expand Down Expand Up @@ -73,6 +74,7 @@ class AwBrowserProcess {
void PreMainMessageLoopRun();

static void TriggerMinidumpUploading();
static ApkType GetApkType();

private:
void CreateSafeBrowsingUIManager();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

import com.android.webview.chromium.WebViewDelegateFactory.WebViewDelegate;

import org.chromium.android_webview.ApkType;
import org.chromium.android_webview.AwBrowserContext;
import org.chromium.android_webview.AwBrowserProcess;
import org.chromium.android_webview.AwContentsStatics;
Expand Down Expand Up @@ -258,6 +259,7 @@ private void initialize(WebViewDelegate webViewDelegate) {
packageInfo = WebViewFactory.getLoadedPackageInfo();
}
AwBrowserProcess.setWebViewPackageName(packageInfo.packageName);
AwBrowserProcess.initializeApkType(packageInfo.applicationInfo);

mAwInit = createAwInit();
mWebViewDelegate = webViewDelegate;
Expand Down Expand Up @@ -288,7 +290,7 @@ private void initialize(WebViewDelegate webViewDelegate) {
}
int packageId = webViewDelegate.getPackageId(ctx.getResources(), resourcePackage);
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q
&& !isTrichrome(packageInfo.applicationInfo)
&& AwBrowserProcess.getApkType() != ApkType.TRICHROME
&& packageId > SHARED_LIBRARY_MAX_ID) {
throw new RuntimeException("Package ID too high for WebView: " + packageId);
}
Expand Down Expand Up @@ -413,13 +415,6 @@ private void initialize(WebViewDelegate webViewDelegate) {
}
}

/**
* Determines whether this is Trichrome WebView by checking if any shared library files exist.
*/
private static boolean isTrichrome(ApplicationInfo info) {
return info.sharedLibraryFiles != null && info.sharedLibraryFiles.length > 0;
}

/**
* Both versionCodes should be from a WebView provider package implemented by Chromium.
* VersionCodes from other kinds of packages won't make any sense in this method.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import android.content.Context;
import android.content.Intent;
import android.content.ServiceConnection;
import android.content.pm.ApplicationInfo;
import android.os.IBinder;
import android.os.ParcelFileDescriptor;
import android.os.RemoteException;
Expand Down Expand Up @@ -56,6 +57,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.TimeUnit;

Expand All @@ -77,6 +79,7 @@ public final class AwBrowserProcess {
PostTask.createSequencedTaskRunner(TaskTraits.BEST_EFFORT_MAY_BLOCK);

private static String sWebViewPackageName;
private static @ApkType int sApkType;

/**
* Loads the native library, and performs basic static construction of objects needed
Expand Down Expand Up @@ -171,6 +174,27 @@ public static String getWebViewPackageName() {
return sWebViewPackageName;
}

public static void initializeApkType(ApplicationInfo info) {
if (info.sharedLibraryFiles != null && info.sharedLibraryFiles.length > 0) {
// Only Trichrome uses shared library files.
sApkType = ApkType.TRICHROME;
} else if (info.className.toLowerCase(Locale.ROOT).contains("monochrome")) {
// Only Monochrome has "monochrome" in the application class name.
sApkType = ApkType.MONOCHROME;
} else {
// Everything else must be standalone.
sApkType = ApkType.STANDALONE;
}
}

/**
* Returns the WebView APK type.
*/
@CalledByNative
public static @ApkType int getApkType() {
return sApkType;
}

/**
* Trigger minidump copying, which in turn triggers minidump uploading.
*/
Expand Down

0 comments on commit 7b0a540

Please sign in to comment.