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/+/1715004Reviewed-by: Sam Maier <smaier@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680307}

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

@@ -87,6 +87,7 @@ 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);
     }
@@ -105,6 +106,7 @@ 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);
@@ -190,6 +192,7 @@ 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);
@@ -362,6 +365,7 @@ 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,13 +48,6 @@
   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,10 +42,15 @@
   native <methods>;
 }
 
-# Remove methods annotated with this if their return value is unused.
+# 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.
 -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,13 +137,6 @@
   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.
@@ -215,10 +208,15 @@
   native <methods>;
 }
 
-# Remove methods annotated with this if their return value is unused.
+# 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.
 -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,8 +73,7 @@ public abstract class ConnectedTask<T extends ChromeGoogleApiClient> implements
     protected abstract void doWhenConnected(T client);
 
     /**
-     * Returns a name of a task. Implementations should not have side effects
-     * as we want to have the logging related calls removed.
+     * Returns a name of a task (for debug logging).
      */
     @RemovableInRelease
     protected abstract String getName();
@@ -93,6 +92,10 @@ 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.
@@ -100,34 +103,30 @@ public abstract class ConnectedTask<T extends ChromeGoogleApiClient> implements
     public final void run() {
         TraceEvent.begin("GCore:" + mLogPrefix + ":run");
         try {
-            Log.d(TAG, "%s:%s started", mLogPrefix, getName());
+            debugLog("started");
             if (mClient.connectWithTimeout(CONNECTION_TIMEOUT_MS)) {
                 try {
-                    Log.d(TAG, "%s:%s connected", mLogPrefix, getName());
+                    debugLog("connected");
                     doWhenConnected(mClient);
-                    Log.d(TAG, "%s:%s finished", mLogPrefix, getName());
+                    debugLog("finished");
                 } finally {
                     mClient.disconnect();
-                    Log.d(TAG, "%s:%s disconnected", mLogPrefix, getName());
+                    debugLog("disconnected");
                     cleanUp();
-                    Log.d(TAG, "%s:%s cleaned up", mLogPrefix, getName());
+                    debugLog("cleaned up");
                 }
             } else {
                 mRetryNumber++;
                 if (mRetryNumber < RETRY_NUMBER_LIMIT && mClient.isGooglePlayServicesAvailable()) {
-                    Log.d(TAG, "%s:%s calling retry", mLogPrefix, getName());
+                    debugLog("calling retry");
                     retry(this, CONNECTION_RETRY_TIME_MS);
                 } else {
                     connectionFailed();
-                    Log.d(TAG, "%s:%s number of retries exceeded", mLogPrefix, getName());
+                    debugLog("number of retries exceeded");
                     cleanUp();
-                    Log.d(TAG, "%s:%s cleaned up", mLogPrefix, getName());
+                    debugLog("cleaned up");
                 }
             }
-        } 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");
         }