[aw] Throw exception from AwProxyController

Throw IllegalArgumentException from AwProxyController instead of
throwing it in the support library glue. Update runBlockingFuture in
WebViewChromiumRunQueue to handle ExecutionException.

Bug: 1016285
Change-Id: Ic6a7903b2cfc6ca4f29e7a93ea1613434f5899da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1872592
Commit-Queue: Laís Minchillo <laisminchillo@chromium.org>
Auto-Submit: Laís Minchillo <laisminchillo@chromium.org>
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712582}

android_webview/java/src/org/chromium/android_webview/AwProxyController.java

@@ -46,7 +46,7 @@ public class AwProxyController {
 
     public AwProxyController() {}
 
-    public String setProxyOverride(
+    public void setProxyOverride(
             String[][] proxyRules, String[] bypassRules, Runnable listener, Executor executor) {
         int length = (proxyRules == null ? 0 : proxyRules.length);
         String[] urlSchemes = new String[length];
@@ -63,7 +63,7 @@ public class AwProxyController {
             // proxy URLs
             proxyUrls[i] = proxyRules[i][1];
             if (proxyUrls[i] == null) {
-                return "Proxy rule " + i + " has a null url";
+                throw new IllegalArgumentException("Proxy rule " + i + " has a null url");
             }
             // Check schemes for UMA
             if (proxyRules[i][0].equals("http")) {
@@ -86,40 +86,41 @@ public class AwProxyController {
         length = (bypassRules == null ? 0 : bypassRules.length);
         for (int i = 0; i < length; i++) {
             if (bypassRules[i] == null) {
-                return "Bypass rule " + i + " is null";
+                throw new IllegalArgumentException("Bypass rule " + i + " is null");
             }
         }
         if (executor == null) {
-            return "Executor must not be null";
+            throw new IllegalArgumentException("Executor must not be null");
         }
 
         String result = AwProxyControllerJni.get().setProxyOverride(
                 AwProxyController.this, urlSchemes, proxyUrls, bypassRules, listener, executor);
-        if (result.equals("")) {
-            // In case operation is successful, log UMA data on SetProxyOverride
-            // Proxy scheme filter
-            if (schemeHttp && schemeHttps) {
-                recordProxySchemeType(ProxySchemeType.ALL);
-            } else if (schemeHttp) {
-                recordProxySchemeType(ProxySchemeType.HTTP);
-            } else if (schemeHttps) {
-                recordProxySchemeType(ProxySchemeType.HTTPS);
-            }
-            // Proxy url type
-            if (urlHttp) {
-                recordProxyUrlType(ProxyUrlType.HTTP);
-            }
-            if (urlHttps) {
-                recordProxyUrlType(ProxyUrlType.HTTPS);
-            }
-            if (urlDirect) {
-                recordProxyUrlType(ProxyUrlType.DIRECT);
-            }
-            // Bypass rules
-            RecordHistogram.recordBooleanHistogram("Android.WebView.SetProxyOverride.BypassRules",
-                    bypassRules == null || bypassRules.length == 0 ? false : true);
+        if (!result.isEmpty()) {
+            throw new IllegalArgumentException(result);
+        }
+
+        // In case operation is successful, log UMA data on SetProxyOverride
+        // Proxy scheme filter
+        if (schemeHttp && schemeHttps) {
+            recordProxySchemeType(ProxySchemeType.ALL);
+        } else if (schemeHttp) {
+            recordProxySchemeType(ProxySchemeType.HTTP);
+        } else if (schemeHttps) {
+            recordProxySchemeType(ProxySchemeType.HTTPS);
+        }
+        // Proxy url type
+        if (urlHttp) {
+            recordProxyUrlType(ProxyUrlType.HTTP);
+        }
+        if (urlHttps) {
+            recordProxyUrlType(ProxyUrlType.HTTPS);
+        }
+        if (urlDirect) {
+            recordProxyUrlType(ProxyUrlType.DIRECT);
         }
-        return result;
+        // Bypass rules
+        RecordHistogram.recordBooleanHistogram("Android.WebView.SetProxyOverride.BypassRules",
+                bypassRules == null || bypassRules.length == 0 ? false : true);
     }
 
     private static void recordProxySchemeType(@ProxySchemeType int proxySchemeType) {
@@ -133,15 +134,14 @@ public class AwProxyController {
                 proxyUrlType, ProxyUrlType.NUM_ENTRIES);
     }
 
-    public String clearProxyOverride(Runnable listener, Executor executor) {
+    public void clearProxyOverride(Runnable listener, Executor executor) {
         if (executor == null) {
-            return "Executor must not be null";
+            throw new IllegalArgumentException("Executor must not be null");
         }
 
         AwProxyControllerJni.get().clearProxyOverride(AwProxyController.this, listener, executor);
         // Log UMA data on ClearProxyOverride
         RecordHistogram.recordBooleanHistogram("Android.WebView.ClearProxyOverride", true);
-        return "";
     }
 
     @CalledByNativeUnchecked

android_webview/javatests/src/org/chromium/android_webview/test/AwProxyControllerTest.java

@@ -21,6 +21,7 @@ import org.chromium.base.test.util.Feature;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.net.test.util.TestWebServer;
 
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
 
 /**
@@ -212,37 +213,42 @@ public class AwProxyControllerTest {
             throws Exception {
         CallbackHelper ch = new CallbackHelper();
         int callCount = ch.getCallCount();
-        String result = TestThreadUtils.runOnUiThreadBlocking(() -> {
-            return mAwProxyController.setProxyOverride(proxyRules, bypassRules, new Runnable() {
+        runOnUiThreadBlocking(() -> {
+            mAwProxyController.setProxyOverride(proxyRules, bypassRules, new Runnable() {
                 @Override
                 public void run() {
                     ch.notifyCalled();
                 }
             }, new SynchronousExecutor());
         });
-        if (!result.isEmpty()) {
-            throw new IllegalArgumentException(result);
-        }
         ch.waitForCallback(callCount);
     }
 
     private void clearProxyOverrideSync() throws Exception {
         CallbackHelper ch = new CallbackHelper();
         int callCount = ch.getCallCount();
-        String result = TestThreadUtils.runOnUiThreadBlocking(() -> {
-            return mAwProxyController.clearProxyOverride(new Runnable() {
+        runOnUiThreadBlocking(() -> {
+            mAwProxyController.clearProxyOverride(new Runnable() {
                 @Override
                 public void run() {
                     ch.notifyCalled();
                 }
             }, new SynchronousExecutor());
         });
-        if (!result.isEmpty()) {
-            throw new IllegalArgumentException(result);
-        }
         ch.waitForCallback(callCount);
     }
 
+    private void runOnUiThreadBlocking(Runnable r) throws Exception {
+        try {
+            TestThreadUtils.runOnUiThreadBlocking(r);
+        } catch (RuntimeException e) {
+            Throwable cause = e.getCause();
+            if (cause instanceof ExecutionException) cause = cause.getCause();
+            if (cause instanceof IllegalArgumentException) throw (IllegalArgumentException) cause;
+            throw e;
+        }
+    }
+
     static class SynchronousExecutor implements Executor {
         @Override
         public void execute(Runnable r) {

android_webview/support_library/java/src/org/chromium/support_lib_glue/SupportLibProxyControllerAdapter.java

@@ -9,6 +9,7 @@ import org.chromium.android_webview.WebViewChromiumRunQueue;
 import org.chromium.base.ThreadUtils;
 import org.chromium.support_lib_boundary.ProxyControllerBoundaryInterface;
 
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
 
 /**
@@ -27,31 +28,44 @@ public class SupportLibProxyControllerAdapter implements ProxyControllerBoundary
     @Override
     public void setProxyOverride(
             String[][] proxyRules, String[] bypassRules, Runnable listener, Executor executor) {
-        String result;
         if (checkNeedsPost()) {
-            result = mRunQueue.runOnUiThreadBlocking(() -> {
-                return mProxyController.setProxyOverride(
-                        proxyRules, bypassRules, listener, executor);
+            RuntimeException exception = mRunQueue.runOnUiThreadBlocking(() -> {
+                try {
+                    mProxyController.setProxyOverride(proxyRules, bypassRules, listener, executor);
+                } catch (RuntimeException e) {
+                    return e;
+                }
+                return null;
             });
+            maybeThrowUnwrappedException(exception);
         } else {
-            result = mProxyController.setProxyOverride(proxyRules, bypassRules, listener, executor);
-        }
-        if (!result.isEmpty()) {
-            throw new IllegalArgumentException(result);
+            mProxyController.setProxyOverride(proxyRules, bypassRules, listener, executor);
         }
     }
 
     @Override
     public void clearProxyOverride(Runnable listener, Executor executor) {
-        String result;
         if (checkNeedsPost()) {
-            result = mRunQueue.runOnUiThreadBlocking(
-                    () -> { return mProxyController.clearProxyOverride(listener, executor); });
+            RuntimeException exception = mRunQueue.runOnUiThreadBlocking(() -> {
+                try {
+                    mProxyController.clearProxyOverride(listener, executor);
+                } catch (RuntimeException e) {
+                    return e;
+                }
+                return null;
+            });
+            maybeThrowUnwrappedException(exception);
         } else {
-            result = mProxyController.clearProxyOverride(listener, executor);
+            mProxyController.clearProxyOverride(listener, executor);
         }
-        if (!result.isEmpty()) {
-            throw new IllegalArgumentException(result);
+    }
+
+    private void maybeThrowUnwrappedException(RuntimeException exception) {
+        if (exception != null) {
+            Throwable cause = exception.getCause();
+            if (cause instanceof ExecutionException) cause = cause.getCause();
+            if (cause instanceof RuntimeException) throw (RuntimeException) cause;
+            throw exception;
         }
     }