Fix errorprone MissingFail warnings

Fix errorprone MissingFail warnings and make
sure it is treated as error after this CL.

http://errorprone.info/bugpattern/MissingFail

Bug: 803589

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I26bb2b58844a9a517e27cc25e334e529eaa6f06a
Reviewed-on: https://chromium-review.googlesource.com/960422Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552830}

base/android/junit/src/org/chromium/base/PromiseTest.java

@@ -7,6 +7,7 @@ package org.chromium.base;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -198,6 +199,7 @@ public class PromiseTest {
         try {
             promise.reject(new NegativeArraySizeException(message));
             ShadowLooper.runUiThreadTasksIncludingDelayedTasks();
+            fail();
         } catch (UnhandledRejectionException e) {
             assertTrue(e.getCause() instanceof NegativeArraySizeException);
             assertEquals(e.getCause().getMessage(), message);
@@ -311,4 +313,4 @@ public class PromiseTest {
             }
         };
     }
-}
\ No newline at end of file
+}

build/android/gyp/javac.py

@@ -38,8 +38,6 @@ ERRORPRONE_WARNINGS_TO_TURN_OFF = [
   'CatchFail',
   # TODO(crbug.com/803485): Follow steps in bug.
   'JUnitAmbiguousTestClass',
-  # TODO(crbug.com/803589): Follow steps in bug.
-  'MissingFail',
   # Android platform default is always UTF-8.
   # https://developer.android.com/reference/java/nio/charset/Charset.html#defaultCharset()
   'DefaultCharset',
@@ -97,6 +95,7 @@ ERRORPRONE_WARNINGS_TO_ERROR = [
   'AssertionFailureIgnored',
   'FloatingPointLiteralPrecision',
   'JavaLangClash',
+  'MissingFail',
   'MissingOverride',
   'NarrowingCompoundAssignment',
   'ParameterName',

chrome/android/javatests/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridgeTest.java

@@ -85,11 +85,13 @@ public class NotificationPlatformBridgeTest {
                 mNotificationTestRule.getTestServer().getURL(NOTIFICATION_TEST_PAGE));
     }
 
+    @SuppressWarnings("MissingFail")
     private void waitForTitle(String expectedTitle) throws InterruptedException {
         Tab tab = mNotificationTestRule.getActivity().getActivityTab();
         TabTitleObserver titleObserver = new TabTitleObserver(tab, expectedTitle);
         try {
             titleObserver.waitForTitleUpdate(TITLE_UPDATE_TIMEOUT_SECONDS);
+
         } catch (TimeoutException e) {
             // The title is not as expected, this assertion neatly logs what the difference is.
             Assert.assertEquals(expectedTitle, tab.getTitle());

chrome/android/javatests/src/org/chromium/chrome/browser/push_messaging/PushMessagingTest.java

@@ -335,6 +335,7 @@ public class PushMessagingTest implements PushMessagingServiceObserver.Listener
         mMessageHandledHelper.waitForCallback(mMessageHandledHelper.getCallCount());
     }
 
+    @SuppressWarnings("MissingFail")
     private void waitForTitle(Tab tab, String expectedTitle) throws InterruptedException {
         TabTitleObserver titleObserver = new TabTitleObserver(tab, expectedTitle);
         try {

components/cronet/android/BUILD.gn

@@ -696,6 +696,7 @@ instrumentation_test_apk("cronet_test_instrumentation_apk") {
     "//net/android:net_java",
     "//net/android:net_java_test_support",
     "//third_party/android_support_test_runner:runner_java",
+    "//third_party/hamcrest:hamcrest_core_java",
   ]
   additional_apks = [ "//net/android:net_test_support_apk" ]
 

components/cronet/android/test/javatests/src/org/chromium/net/urlconnection/CronetFixedModeOutputStreamTest.java

@@ -4,10 +4,15 @@
 
 package org.chromium.net.urlconnection;
 
+import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import org.hamcrest.Description;
+import org.hamcrest.TypeSafeMatcher;
+
 import static org.chromium.net.CronetTestRule.getContext;
 
 import android.support.test.filters.SmallTest;
@@ -16,6 +21,7 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 
 import org.chromium.base.test.BaseJUnit4ClassRunner;
@@ -26,6 +32,7 @@ import org.chromium.net.CronetTestRule.CompareDefaultWithCronet;
 import org.chromium.net.CronetTestRule.OnlyRunCronetHttpURLConnection;
 import org.chromium.net.NativeTestServer;
 import org.chromium.net.NetworkException;
+import org.chromium.net.impl.CallbackExceptionImpl;
 
 import java.io.IOException;
 import java.io.OutputStream;
@@ -47,6 +54,9 @@ public class CronetFixedModeOutputStreamTest {
     @Rule
     public final CronetTestRule mTestRule = new CronetTestRule();
 
+    @Rule
+    public ExpectedException thrown = ExpectedException.none();
+
     @Before
     public void setUp() throws Exception {
         mTestRule.setStreamHandlerFactory(new CronetEngine.Builder(getContext()).build());
@@ -428,11 +438,37 @@ public class CronetFixedModeOutputStreamTest {
         connection.disconnect();
     }
 
+    private static class CauseMatcher extends TypeSafeMatcher<Throwable> {
+        private final Class<? extends Throwable> mType;
+        private final String mExpectedMessage;
+        private final Class<? extends Throwable> mInnerCauseType;
+        private final String mInnerCauseExpectedMessage;
+
+        public CauseMatcher(Class<? extends Throwable> type, String expectedMessage,
+                Class<? extends Throwable> innerCauseType, String innerCauseExpectedMessage) {
+            this.mType = type;
+            this.mExpectedMessage = expectedMessage;
+            this.mInnerCauseType = innerCauseType;
+            this.mInnerCauseExpectedMessage = innerCauseExpectedMessage;
+        }
+
+        @Override
+        protected boolean matchesSafely(Throwable item) {
+            return item.getClass().isAssignableFrom(mType)
+                    && item.getMessage().equals(mExpectedMessage)
+                    && item.getCause().getClass().isAssignableFrom(mInnerCauseType)
+                    && item.getCause().getMessage().equals(mInnerCauseExpectedMessage);
+        }
+        @Override
+        public void describeTo(Description description) {}
+    }
+
     @Test
     @SmallTest
     @Feature({"Cronet"})
-    @CompareDefaultWithCronet
-    public void testRewind() throws Exception {
+    @OnlyRunCronetHttpURLConnection
+    public void testRewindWithCronet() throws Exception {
+        assertFalse(mTestRule.testingSystemHttpURLConnection());
         // Post preserving redirect should fail.
         URL url = new URL(NativeTestServer.getRedirectToEchoBody());
         HttpURLConnection connection =
@@ -440,12 +476,14 @@ public class CronetFixedModeOutputStreamTest {
         connection.setDoOutput(true);
         connection.setRequestMethod("POST");
         connection.setFixedLengthStreamingMode(TestUtil.UPLOAD_DATA.length);
-        try {
-            OutputStream out = connection.getOutputStream();
-            out.write(TestUtil.UPLOAD_DATA);
-        } catch (HttpRetryException e) {
-            assertEquals("Cannot retry streamed Http body", e.getMessage());
-        }
+        thrown.expectMessage("Cronet Test failed.");
+        thrown.expectCause(instanceOf(CallbackExceptionImpl.class));
+        thrown.expectCause(new CauseMatcher(CallbackExceptionImpl.class,
+                "Exception received from UploadDataProvider", HttpRetryException.class,
+                "Cannot retry streamed Http body"));
+        OutputStream out = connection.getOutputStream();
+        out.write(TestUtil.UPLOAD_DATA);
+        connection.getResponseCode();
         connection.disconnect();
     }
 }

components/cronet/android/test/javatests/src/org/chromium/net/urlconnection/CronetURLStreamHandlerFactoryTest.java

@@ -5,6 +5,7 @@
 package org.chromium.net.urlconnection;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
 import android.support.test.filters.SmallTest;
 
@@ -32,6 +33,7 @@ public class CronetURLStreamHandlerFactoryTest {
         mTestRule.startCronetTestFramework();
         try {
             new CronetURLStreamHandlerFactory(null);
+            fail();
         } catch (NullPointerException e) {
             assertEquals("CronetEngine is null.", e.getMessage());
         }