Skip to content

Commit

Permalink
AW SafeMode: log histograms
Browse files Browse the repository at this point in the history 
This adds histogram logging to the production code for querying and
executing SafeMode. These metrics are patterned after some of the
initial Android.WebView.DevUi.* histograms since SafeMode follows a
similar implementation.

This moves the try-catch into the if-condition. This should not
significantly impact stability, as the main concern was about unexpected
crashes when exercising the seldom-enabled SafeMode code path, whereas
the code outside of the if-condition is covered in the common case.

Bug: 1216239
Test: Validity check - start up WebView and verify no unexpected crashes
Change-Id: Ib2f4ac525940a4e4c4b639d1780d0e2306529cf6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2957259
Reviewed-by: Richard Coles <[email protected]>
Reviewed-by: Jesse Doherty <[email protected]>
Commit-Queue: Jesse Doherty <[email protected]>
Auto-Submit: Nate Fischer <[email protected]>
Cr-Commit-Position: refs/heads/master@{#892576}
  • Loading branch information
@ntfschr-chromium
ntfschr-chromium authored and Chromium LUCI CQ committed Jun 15, 2021
1 parent b56b8aa commit 88e212b
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 9 deletions.
48 changes: 39 additions & 9 deletions ...id_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import android.webkit.WebViewFactoryProvider;
import android.webkit.WebViewProvider;

import androidx.annotation.IntDef;

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

import org.chromium.android_webview.ApkType;
Expand Down Expand Up @@ -399,20 +401,34 @@ private void initialize(WebViewDelegate webViewDelegate) {
AwContentsStatics.logFlagOverridesWithNative(flagOverrides);
}

try {
SafeModeController controller = SafeModeController.getInstance();

controller.registerActions(BrowserSafeModeActionList.sList);
boolean isSafeModeEnabled = controller.isSafeModeEnabled(webViewPackageName);
if (isSafeModeEnabled) {
SafeModeController controller = SafeModeController.getInstance();
controller.registerActions(BrowserSafeModeActionList.sList);
long safeModeStart = SystemClock.elapsedRealtime();
boolean isSafeModeEnabled = controller.isSafeModeEnabled(webViewPackageName);
long safeModeEnd = SystemClock.elapsedRealtime();
RecordHistogram.recordTimesHistogram(
"Android.WebView.SafeMode.CheckStateBlockingTime", safeModeEnd - safeModeStart);
RecordHistogram.recordBooleanHistogram(
"Android.WebView.SafeMode.SafeModeEnabled", isSafeModeEnabled);
if (isSafeModeEnabled) {
try {
long safeModeQueryExecuteStart = SystemClock.elapsedRealtime();
Set<String> actions = controller.queryActions(webViewPackageName);
Log.w(TAG, "WebViewSafeMode is enabled: received %d SafeModeActions",
actions.size());
RecordHistogram.recordCount100Histogram(
"Android.WebView.SafeMode.ActionsCount", actions.size());
controller.executeActions(actions);
long safeModeQueryExecuteEnd = SystemClock.elapsedRealtime();
RecordHistogram.recordTimesHistogram(
"Android.WebView.SafeMode.QueryAndExecuteBlockingTime",
safeModeQueryExecuteEnd - safeModeQueryExecuteStart);
logSafeModeExecutionResult(SafeModeExecutionResult.SUCCESS);
} catch (Throwable t) {
// Don't let SafeMode crash WebView. Instead just log the error.
Log.e(TAG, "WebViewSafeMode threw exception: ", t);
logSafeModeExecutionResult(SafeModeExecutionResult.UNKNOWN_ERROR);
}
} catch (Throwable t) {
// Don't let SafeMode crash WebView. Instead just log the error.
Log.e(TAG, "WebViewSafeMode threw exception: ", t);
}

mAwInit.startVariationsInit();
Expand Down Expand Up @@ -442,6 +458,20 @@ private void initialize(WebViewDelegate webViewDelegate) {
*/
}

// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
@IntDef({SafeModeExecutionResult.SUCCESS, SafeModeExecutionResult.UNKNOWN_ERROR})
private @interface SafeModeExecutionResult {
int SUCCESS = 0;
int UNKNOWN_ERROR = 1;
int COUNT = 2;
}

private static void logSafeModeExecutionResult(@SafeModeExecutionResult int result) {
RecordHistogram.recordEnumeratedHistogram(
"Android.WebView.SafeMode.ExecutionResult", result, SafeModeExecutionResult.COUNT);
}

/* package */ static void checkStorageIsNotDeviceProtected(Context context) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N
&& GlueApiHelperForN.isDeviceProtectedStorage(context)) {
Expand Down
5 changes: 5 additions & 0 deletions tools/metrics/histograms/enums.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2448,6 +2448,11 @@ Unknown properties are collapsed to zero. -->
<int value="2" label="DIRECT"/>
</enum>

<enum name="AndroidWebViewSafeModeResult">
<int value="0" label="Success"/>
<int value="1" label="Unknown error"/>
</enum>

<enum name="AndroidWebViewSingleOrMultiProcess">
<int value="0" label="SINGLE_PROCESS"/>
<int value="1" label="MULTI_PROCESS"/>
Expand Down
61 changes: 61 additions & 0 deletions tools/metrics/histograms/histograms_xml/android/histograms.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3361,6 +3361,67 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
</summary>
</histogram>

<histogram name="Android.WebView.SafeMode.ActionsCount"
units="SafeMode actions" expires_after="2022-06-08">
<owner>[email protected]</owner>
<owner>src/android_webview/OWNERS</owner>
<summary>
Records the number of SafeMode actions WebView received from the
SafeModeService. This is only recorded once during startup if SafeMode is
enabled (see &quot;Android.WebView.SafeMode.Enabled&quot;), otherwise this
is not recorded at all (because there were no SafeMode actions to execute).
This is recorded before executing any SafeMode actions.
</summary>
</histogram>

<histogram name="Android.WebView.SafeMode.CheckStateBlockingTime" units="ms"
expires_after="2022-06-08">
<owner>[email protected]</owner>
<owner>src/android_webview/OWNERS</owner>
<summary>
Records the time spent blocking WebView startup to check for SafeMode (see
&quot;Android.WebView.SafeMode.Enabled&quot;). This is recorded once during
WebView startup, regardless of whether SafeMode is enabled.
</summary>
</histogram>

<histogram name="Android.WebView.SafeMode.Enabled" enum="BooleanEnabled"
expires_after="2022-06-08">
<owner>[email protected]</owner>
<owner>src/android_webview/OWNERS</owner>
<summary>
Records whether or not WebView is starting up in SafeMode. This is a mode
where WebView takes extra steps during startup to reduce the risk of
starting in a bad state. This is recorded once during WebView startup,
regardless of whether SafeMode is enabled.
</summary>
</histogram>

<histogram name="Android.WebView.SafeMode.ExecutionResult"
enum="AndroidWebViewSafeModeResult" expires_after="2022-06-08">
<owner>[email protected]</owner>
<owner>src/android_webview/OWNERS</owner>
<summary>
Records whether SafeMode was able to execute all the specified SafeMode
actions or if it encountered an error. This is recorded once during WebView
startup and only if SafeMode is enabled (see
&quot;Android.WebView.SafeMode.Enabled&quot;).
</summary>
</histogram>

<histogram name="Android.WebView.SafeMode.QueryAndExecuteBlockingTime"
units="ms" expires_after="2022-06-08">
<owner>[email protected]</owner>
<owner>src/android_webview/OWNERS</owner>
<summary>
Records the time spent blocking WebView startup to both retrieve the list of
SafeMode actions from the SafeModeService and to execute those actions. This
is recorded once during startup only if SafeMode is enabled (see
&quot;Android.WebView.SafeMode.Enabled&quot;). This can be interpreted as
the additional startup cost caused by enabling SafeMode.
</summary>
</histogram>

<histogram name="Android.WebView.SecureCookieAction" enum="SecureCookieAction"
expires_after="2022-05-10">
<owner>[email protected]</owner>
Expand Down

0 comments on commit 88e212b

Please sign in to comment.