Skip to content

Commit

Permalink
Change JS Sandbox client library in line with AndroidX lint rules
Browse files Browse the repository at this point in the history
AndroidX lint rules are more stricter than the chromium lint rules, so
make changes here to align with the AndroidX rules. Also, add other minor improvements to the client code.

Bug: 1297672
Change-Id: Id3cd7d88518361eae76a7bf53ac9714a1b06a058
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3714103
Reviewed-by: Richard Coles <torne@chromium.org>
Commit-Queue: Abhijith Nair <abhijithnair@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1019539}
  • Loading branch information
Abhijith Nair authored and Chromium LUCI CQ committed Jun 30, 2022
1 parent dafab3b commit bf9afa9
Show file tree
Hide file tree
Showing 12 changed files with 228 additions and 107 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ public void testSimpleJsEvaluation() throws Throwable {
Context context = ContextUtils.getApplicationContext();

ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate = jsSandbox.createIsolate();
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascript(code);
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascriptAsync(code);
String result = resultFuture.get(5, TimeUnit.SECONDS);
jsIsolate.close();
jsSandbox.close();
Expand All @@ -56,12 +56,12 @@ public void testClosingOneIsolate() throws Throwable {
Context context = ContextUtils.getApplicationContext();

ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate1 = jsSandbox.createIsolate();
JsIsolate jsIsolate2 = jsSandbox.createIsolate();
jsIsolate1.close();
ListenableFuture<String> resultFuture = jsIsolate2.evaluateJavascript(code);
ListenableFuture<String> resultFuture = jsIsolate2.evaluateJavascriptAsync(code);
String result = resultFuture.get(5, TimeUnit.SECONDS);
jsIsolate2.close();
jsSandbox.close();
Expand All @@ -80,13 +80,13 @@ public void testEvaluationInTwoIsolates() throws Throwable {
Context context = ContextUtils.getApplicationContext();

ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate1 = jsSandbox.createIsolate();
ListenableFuture<String> resultFuture1 = jsIsolate1.evaluateJavascript(code1);
ListenableFuture<String> resultFuture1 = jsIsolate1.evaluateJavascriptAsync(code1);
String result1 = resultFuture1.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate2 = jsSandbox.createIsolate();
ListenableFuture<String> resultFuture2 = jsIsolate2.evaluateJavascript(code2);
ListenableFuture<String> resultFuture2 = jsIsolate2.evaluateJavascriptAsync(code2);
String result2 = resultFuture2.get(5, TimeUnit.SECONDS);
jsIsolate1.close();
jsIsolate2.close();
Expand All @@ -106,13 +106,13 @@ public void testTwoIsolatesDoNotShareEnvironment() throws Throwable {
Context context = ContextUtils.getApplicationContext();

ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate1 = jsSandbox.createIsolate();
ListenableFuture<String> resultFuture1 = jsIsolate1.evaluateJavascript(code1);
ListenableFuture<String> resultFuture1 = jsIsolate1.evaluateJavascriptAsync(code1);
String result1 = resultFuture1.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate2 = jsSandbox.createIsolate();
ListenableFuture<String> resultFuture2 = jsIsolate2.evaluateJavascript(code2);
ListenableFuture<String> resultFuture2 = jsIsolate2.evaluateJavascriptAsync(code2);
String result2 = resultFuture2.get(5, TimeUnit.SECONDS);
jsIsolate1.close();
jsIsolate2.close();
Expand All @@ -132,12 +132,12 @@ public void testTwoExecutionsShareEnvironment() throws Throwable {
Context context = ContextUtils.getApplicationContext();

ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate1 = jsSandbox.createIsolate();
ListenableFuture<String> resultFuture1 = jsIsolate1.evaluateJavascript(code1);
ListenableFuture<String> resultFuture1 = jsIsolate1.evaluateJavascriptAsync(code1);
String result1 = resultFuture1.get(5, TimeUnit.SECONDS);
ListenableFuture<String> resultFuture2 = jsIsolate1.evaluateJavascript(code2);
ListenableFuture<String> resultFuture2 = jsIsolate1.evaluateJavascriptAsync(code2);
String result2 = resultFuture2.get(5, TimeUnit.SECONDS);
jsIsolate1.close();
jsSandbox.close();
Expand All @@ -154,10 +154,10 @@ public void testJsEvaluationError() throws Throwable {
Context context = ContextUtils.getApplicationContext();

ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate = jsSandbox.createIsolate();
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascript(code);
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascriptAsync(code);
boolean isOfCorrectType = false;
String error = "";
try {
Expand All @@ -180,10 +180,10 @@ public void testInfiniteLoop() throws Throwable {
Context context = ContextUtils.getApplicationContext();

ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate = jsSandbox.createIsolate();
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascript(code);
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascriptAsync(code);
boolean isOfCorrectType = false;
try {
jsIsolate.close();
Expand All @@ -204,12 +204,12 @@ public void testMultipleInfiniteLoops() throws Throwable {
Context context = ContextUtils.getApplicationContext();

ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate = jsSandbox.createIsolate();
Vector<ListenableFuture<String>> resultFutures = new Vector<ListenableFuture<String>>();
for (int i = 0; i < num_of_evaluations; i++) {
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascript(code);
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascriptAsync(code);
resultFutures.add(resultFuture);
}
jsIsolate.close();
Expand Down Expand Up @@ -240,12 +240,12 @@ public void testSimpleArrayBuffer() throws Throwable {
+ "});";
Context context = ContextUtils.getApplicationContext();
ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
try (JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate = jsSandbox.createIsolate()) {
boolean provideNamedDataReturn = jsIsolate.provideNamedData("id-1", bytes);
Assert.assertTrue(provideNamedDataReturn);
ListenableFuture<String> resultFuture1 = jsIsolate.evaluateJavascript(code);
ListenableFuture<String> resultFuture1 = jsIsolate.evaluateJavascriptAsync(code);
String result = resultFuture1.get(5, TimeUnit.SECONDS);

Assert.assertEquals(provideString, result);
Expand All @@ -266,12 +266,12 @@ public void testArrayBufferWasmCompilation() throws Throwable {
+ "});";
Context context = ContextUtils.getApplicationContext();
ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
try (JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate = jsSandbox.createIsolate()) {
boolean provideNamedDataReturn = jsIsolate.provideNamedData("id-1", bytes);
Assert.assertTrue(provideNamedDataReturn);
ListenableFuture<String> resultFuture1 = jsIsolate.evaluateJavascript(code);
ListenableFuture<String> resultFuture1 = jsIsolate.evaluateJavascriptAsync(code);
String result = resultFuture1.get(5, TimeUnit.SECONDS);

Assert.assertEquals(success, result);
Expand All @@ -285,10 +285,10 @@ public void testPromiseReturn() throws Throwable {
final String expected = "PASS";
Context context = ContextUtils.getApplicationContext();
ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
try (JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate = jsSandbox.createIsolate()) {
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascript(code);
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascriptAsync(code);
String result = resultFuture.get(5, TimeUnit.SECONDS);

Assert.assertEquals(expected, result);
Expand All @@ -308,11 +308,11 @@ public void testPromiseReturnLaterResolve() throws Throwable {
Context context = ContextUtils.getApplicationContext();

ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
try (JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate = jsSandbox.createIsolate()) {
ListenableFuture<String> resultFuture1 = jsIsolate.evaluateJavascript(code1);
ListenableFuture<String> resultFuture2 = jsIsolate.evaluateJavascript(code2);
ListenableFuture<String> resultFuture1 = jsIsolate.evaluateJavascriptAsync(code1);
ListenableFuture<String> resultFuture2 = jsIsolate.evaluateJavascriptAsync(code2);
String result = resultFuture1.get(5, TimeUnit.SECONDS);

Assert.assertEquals(expected, result);
Expand Down Expand Up @@ -341,7 +341,7 @@ public void testNestedConsumeNamedDataAsArrayBuffer() throws Throwable {
+ "});";
Context context = ContextUtils.getApplicationContext();
ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
try (JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate = jsSandbox.createIsolate()) {
jsIsolate.provideNamedData("id-1", bytes);
Expand All @@ -350,7 +350,7 @@ public void testNestedConsumeNamedDataAsArrayBuffer() throws Throwable {
jsIsolate.provideNamedData("id-4", bytes);
jsIsolate.provideNamedData("id-5", bytes);
Thread.sleep(1000);
ListenableFuture<String> resultFuture1 = jsIsolate.evaluateJavascript(code);
ListenableFuture<String> resultFuture1 = jsIsolate.evaluateJavascriptAsync(code);
String result = resultFuture1.get(5, TimeUnit.SECONDS);

Assert.assertEquals(success, result);
Expand All @@ -370,10 +370,10 @@ public void testPromiseEvaluationThrow() throws Throwable {
Context context = ContextUtils.getApplicationContext();

ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
try (JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate = jsSandbox.createIsolate()) {
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascript(code);
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascriptAsync(code);
try {
String result = resultFuture.get(5, TimeUnit.SECONDS);
Assert.fail("Should have thrown.");
Expand All @@ -393,10 +393,10 @@ public void testEvaluationThrowsWhenSandboxDead() throws Throwable {
Context context = ContextUtils.getApplicationContext();

ListenableFuture<JsSandbox> JsSandboxFuture =
JsSandbox.newConnectedInstanceForTesting(context);
JsSandbox.newConnectedInstanceForTestingAsync(context);
JsSandbox jsSandbox = JsSandboxFuture.get(5, TimeUnit.SECONDS);
JsIsolate jsIsolate = jsSandbox.createIsolate();
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascript(code);
ListenableFuture<String> resultFuture = jsIsolate.evaluateJavascriptAsync(code);
try {
jsSandbox.close();
resultFuture.get(5, TimeUnit.SECONDS);
Expand Down
1 change: 1 addition & 0 deletions android_webview/js_sandbox/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ android_library("js_sandbox_service_java") {

android_library("js_sandbox_java") {
sources = [
"java/src/org/chromium/android_webview/js_sandbox/client/CloseGuardHelper.java",
"java/src/org/chromium/android_webview/js_sandbox/client/EvaluationFailedException.java",
"java/src/org/chromium/android_webview/js_sandbox/client/IsolateTerminatedException.java",
"java/src/org/chromium/android_webview/js_sandbox/client/JsException.java",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// Copyright 2022 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.

package org.chromium.android_webview.js_sandbox.client;

import android.os.Build;
import android.util.CloseGuard;

import androidx.annotation.NonNull;
import androidx.annotation.RequiresApi;
import androidx.core.util.Preconditions;

/**
* Helper for accessing CloseGuard on API levels that support it.
*
* <p>All operations will be no-ops on non-supported API levels.
*/
@RequiresApi(21)
public final class CloseGuardHelper {
private final CloseGuardImpl mImpl;

private CloseGuardHelper(CloseGuardImpl impl) {
mImpl = impl;
}

/**
* Returns a {@link CloseGuardHelper} which defers to the platform close guard if it is
* available.
*/
@NonNull
public static CloseGuardHelper create() {
if (Build.VERSION.SDK_INT >= 30) {
return new CloseGuardHelper(new CloseGuardApi30Impl());
}

return new CloseGuardHelper(new CloseGuardNoOpImpl());
}

/**
* Initializes the instance with a warning that the caller should have explicitly called the
* {@code closeMethodName} method instead of relying on finalization.
*
* @param closeMethodName non-null name of explicit termination method. Printed by
* warnIfOpen.
* @throws NullPointerException if closeMethodName is null.
*/
public void open(@NonNull String closeMethodName) {
mImpl.open(closeMethodName);
}

/** Marks this CloseGuard instance as closed to avoid warnings on finalization. */
public void close() {
mImpl.close();
}

/**
* Logs a warning if the caller did not properly cleanup by calling an explicit close method
* before finalization.
*/
public void warnIfOpen() {
mImpl.warnIfOpen();
}

private interface CloseGuardImpl {
void open(@NonNull String closeMethodName);
void close();
void warnIfOpen();
}

@RequiresApi(30)
static final class CloseGuardApi30Impl implements CloseGuardImpl {
private final CloseGuard mPlatformImpl = new CloseGuard();

@Override
public void open(@NonNull String closeMethodName) {
mPlatformImpl.open(closeMethodName);
}

@Override
public void close() {
mPlatformImpl.close();
}

@Override
public void warnIfOpen() {
mPlatformImpl.warnIfOpen();
}
}

static final class CloseGuardNoOpImpl implements CloseGuardImpl {
@Override
public void open(@NonNull String closeMethodName) {
Preconditions.checkNotNull(closeMethodName, "CloseMethodName must not be null.");
}

@Override
public void close() {}

@Override
public void warnIfOpen() {}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@

package org.chromium.android_webview.js_sandbox.client;

import androidx.annotation.NonNull;

/** Wrapper for the exception thrown by the JS evaluation engine. */
public class EvaluationFailedException extends JsException {
public EvaluationFailedException(String error) {
public EvaluationFailedException(@NonNull String error) {
super(error);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@

package org.chromium.android_webview.js_sandbox.client;

import androidx.annotation.NonNull;

/** Super class for all exceptions thrown during evaluation. */
public class JsException extends Exception {
public JsException(String error) {
public JsException(@NonNull String error) {
super(error);
}

Expand Down
Loading

0 comments on commit bf9afa9

Please sign in to comment.