Revert "Reland "ProGuard: Move -checkdiscard for Log.* into @RemovableInRelease annotation""

This reverts commit 885aa020.

Reason for revert: Builders are failing again
Bug: 987226

Original change's description:
> Reland "ProGuard: Move -checkdiscard for Log.* into @RemovableInRelease annotation"
> 
> Was reverted in: 54f75614.
> 
> Reason for reland: Includes fix for downstream targets.
> 
> Change-Id: I2a5f77cdbd40934ae206dd91b2e12e7b5e6553e1
> Bug: 986693, 986693
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1715004
> Reviewed-by: Sam Maier <smaier@chromium.org>
> Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#680307}

TBR=agrieve@chromium.org,smaier@chromium.org

Change-Id: I210a6fde9d3a37b8e2ea0f7766a127fd95e8616c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 986693
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1715492Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680392}

base/android/java/src/org/chromium/base/Log.java

@@ -87,7 +87,6 @@ public class Log {
      * Note: Has no effect on whether logs are sent or not. Use a method with
      * {@link RemovableInRelease} to log something in Debug builds only.
      */
-    @RemovableInRelease
     public static boolean isLoggable(String tag, int level) {
         return android.util.Log.isLoggable(tag, level);
     }
@@ -106,7 +105,6 @@ public class Log {
      * @param args Arguments referenced by the format specifiers in the format string. If the last
      *             one is a {@link Throwable}, its trace will be printed.
      */
-    @RemovableInRelease
     private static void verbose(String tag, String messageTemplate, Object... args) {
         String message = formatLogWithStack(messageTemplate, args);
         Throwable tr = getThrowableToLog(args);
@@ -192,7 +190,6 @@ public class Log {
      * @param args Arguments referenced by the format specifiers in the format string. If the last
      *             one is a {@link Throwable}, its trace will be printed.
      */
-    @RemovableInRelease
     private static void debug(String tag, String messageTemplate, Object... args) {
         String message = formatLogWithStack(messageTemplate, args);
         Throwable tr = getThrowableToLog(args);
@@ -365,7 +362,6 @@ public class Log {
     }
 
     /** Returns a string form of the origin of the log call, to be used as secondary tag.*/
-    @RemovableInRelease
     private static String getCallOrigin() {
         StackTraceElement[] st = Thread.currentThread().getStackTrace();
 

base/android/proguard/chromium_apk.flags

@@ -48,6 +48,13 @@
   static *** v(...);
   static *** isLoggable(...);
 }
+-checkdiscard class org.chromium.base.Log {
+  public static void d(...);
+  public static void v(...);
+  private static void debug(...);
+  private static void verbose(...);
+  private static String getCallOrigin(...);
+}
 
 # The following chart was created on July 20, 2016, to decide on 3 optimization
 # passes for Chrome.

base/android/proguard/chromium_code.flags

@@ -42,15 +42,10 @@
   native <methods>;
 }
 
-# Remove methods with this annotation. Methods generally need to be void, or
-# have unused return values in order to be removed. Compiles will fail if any
-# such method was not able to be safely removed.
+# Remove methods annotated with this if their return value is unused.
 -assumenosideeffects class ** {
   @org.chromium.base.annotations.RemovableInRelease <methods>;
 }
--checkdiscard class ** {
-  @org.chromium.base.annotations.RemovableInRelease <methods>;
-}
 
 # Never inline classes or methods with this annotation, but allow shrinking and
 # obfuscation.

chrome/android/java/monochrome_public_bundle.proguard_flags.expected

@@ -137,6 +137,13 @@
   static *** v(...);
   static *** isLoggable(...);
 }
+-checkdiscard class org.chromium.base.Log {
+  public static void d(...);
+  public static void v(...);
+  private static void debug(...);
+  private static void verbose(...);
+  private static String getCallOrigin(...);
+}
 
 # The following chart was created on July 20, 2016, to decide on 3 optimization
 # passes for Chrome.
@@ -208,15 +215,10 @@
   native <methods>;
 }
 
-# Remove methods with this annotation. Methods generally need to be void, or
-# have unused return values in order to be removed. Compiles will fail if any
-# such method was not able to be safely removed.
+# Remove methods annotated with this if their return value is unused.
 -assumenosideeffects class ** {
   @org.chromium.base.annotations.RemovableInRelease <methods>;
 }
--checkdiscard class ** {
-  @org.chromium.base.annotations.RemovableInRelease <methods>;
-}
 
 # Never inline classes or methods with this annotation, but allow shrinking and
 # obfuscation.

chrome/android/java/src/org/chromium/chrome/browser/gcore/ConnectedTask.java

@@ -73,7 +73,8 @@ public abstract class ConnectedTask<T extends ChromeGoogleApiClient> implements
     protected abstract void doWhenConnected(T client);
 
     /**
-     * Returns a name of a task (for debug logging).
+     * Returns a name of a task. Implementations should not have side effects
+     * as we want to have the logging related calls removed.
      */
     @RemovableInRelease
     protected abstract String getName();
@@ -92,10 +93,6 @@ public abstract class ConnectedTask<T extends ChromeGoogleApiClient> implements
      */
     protected void connectionFailed() {}
 
-    private void debugLog(String message) {
-        Log.d(TAG, "%s:%s %s", mLogPrefix, getName(), message);
-    }
-
     @Override
     @VisibleForTesting
     // We always only pass in a string literal here.
@@ -103,30 +100,34 @@ public abstract class ConnectedTask<T extends ChromeGoogleApiClient> implements
     public final void run() {
         TraceEvent.begin("GCore:" + mLogPrefix + ":run");
         try {
-            debugLog("started");
+            Log.d(TAG, "%s:%s started", mLogPrefix, getName());
             if (mClient.connectWithTimeout(CONNECTION_TIMEOUT_MS)) {
                 try {
-                    debugLog("connected");
+                    Log.d(TAG, "%s:%s connected", mLogPrefix, getName());
                     doWhenConnected(mClient);
-                    debugLog("finished");
+                    Log.d(TAG, "%s:%s finished", mLogPrefix, getName());
                 } finally {
                     mClient.disconnect();
-                    debugLog("disconnected");
+                    Log.d(TAG, "%s:%s disconnected", mLogPrefix, getName());
                     cleanUp();
-                    debugLog("cleaned up");
+                    Log.d(TAG, "%s:%s cleaned up", mLogPrefix, getName());
                 }
             } else {
                 mRetryNumber++;
                 if (mRetryNumber < RETRY_NUMBER_LIMIT && mClient.isGooglePlayServicesAvailable()) {
-                    debugLog("calling retry");
+                    Log.d(TAG, "%s:%s calling retry", mLogPrefix, getName());
                     retry(this, CONNECTION_RETRY_TIME_MS);
                 } else {
                     connectionFailed();
-                    debugLog("number of retries exceeded");
+                    Log.d(TAG, "%s:%s number of retries exceeded", mLogPrefix, getName());
                     cleanUp();
-                    debugLog("cleaned up");
+                    Log.d(TAG, "%s:%s cleaned up", mLogPrefix, getName());
                 }
             }
+        } catch (RuntimeException e) {
+            Log.e(TAG, "%s:%s runtime exception %s: %s", mLogPrefix, getName(),
+                    e.getClass().getName(), e.getMessage());
+            throw e;
         } finally {
             TraceEvent.end("GCore:" + mLogPrefix + ":run");
         }