[Android log] Promote using hardcoded string tags

This is to avoid static initializers that would make the code slower.
See the linked bug for discussion.

This patch deprecates Log#makeTag(String) and adds presubmit checks
to enforce at submit time what makeTag was trying to do: length and
naming rules

BUG=485772

Review URL: https://codereview.chromium.org/1131903007

Cr-Commit-Position: refs/heads/master@{#333710}

4 files changed, 162 insertions, 20 deletions

diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 4d5c853..515da1d 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -1304,6 +1304,77 @@ def _CheckJavaStyle(input_api, output_api):
       black_list=_EXCLUDED_PATHS + input_api.DEFAULT_BLACK_LIST)
 
 
+def _CheckAndroidCrLogUsage(input_api, output_api):
+  """Checks that new logs using org.chromium.base.Log:
+    - Are using 'TAG' as variable name for the tags (warn)
+    - Are using the suggested name format for the tags: "cr.<PackageTag>" (warn)
+    - Are using a tag that is shorter than 23 characters (error)
+  """
+  cr_log_import_pattern = input_api.re.compile(
+      r'^import org\.chromium\.base\.Log;$', input_api.re.MULTILINE);
+  # Extract the tag from lines like `Log.d(TAG, "*");` or `Log.d("TAG", "*");`
+  cr_log_pattern = input_api.re.compile(r'^\s*Log\.\w\((?P<tag>\"?\w+\"?)\,')
+  log_decl_pattern = input_api.re.compile(
+      r'^\s*private static final String TAG = "(?P<name>(.*)")',
+      input_api.re.MULTILINE)
+  log_name_pattern = input_api.re.compile(r'^cr[.\w]*')
+
+  REF_MSG = ('See base/android/java/src/org/chromium/base/README_logging.md '
+            'or contact dgn@chromium.org for more info.')
+  sources = lambda x: input_api.FilterSourceFile(x, white_list=(r'.*\.java$',))
+  tag_errors = []
+  tag_decl_errors = []
+  tag_length_errors = []
+
+  for f in input_api.AffectedSourceFiles(sources):
+    file_content = input_api.ReadFile(f)
+    has_modified_logs = False
+
+    # Per line checks
+    if cr_log_import_pattern.search(file_content):
+      for line_num, line in f.ChangedContents():
+
+        # Check if the new line is doing some logging
+        match = cr_log_pattern.search(line)
+        if match:
+          has_modified_logs = True
+
+          # Make sure it uses "TAG"
+          if not match.group('tag') == 'TAG':
+            tag_errors.append("%s:%d" % (f.LocalPath(), line_num))
+
+    # Per file checks
+    if has_modified_logs:
+      # Make sure the tag is using the "cr" prefix and is not too long
+      match = log_decl_pattern.search(file_content)
+      tag_name = match.group('name') if match else ''
+      if not log_name_pattern.search(tag_name ):
+        tag_decl_errors.append(f.LocalPath())
+      if len(tag_name) > 23:
+        tag_length_errors.append(f.LocalPath())
+
+  results = []
+  if tag_decl_errors:
+    results.append(output_api.PresubmitPromptWarning(
+        'Please define your tags using the suggested format: .\n'
+        '"private static final String TAG = "cr.<package tag>".\n' + REF_MSG,
+        tag_decl_errors))
+
+  if tag_length_errors:
+    results.append(output_api.PresubmitError(
+        'The tag length is restricted by the system to be at most '
+        '23 characters.\n' + REF_MSG,
+        tag_length_errors))
+
+  if tag_errors:
+    results.append(output_api.PresubmitPromptWarning(
+        'Please use a variable named "TAG" for your log tags.\n' + REF_MSG,
+        tag_errors))
+
+  return results
+
+
+# TODO(dgn): refactor with _CheckAndroidCrLogUsage
 def _CheckNoNewUtilLogUsage(input_api, output_api):
   """Checks that new logs are using org.chromium.base.Log."""
 
@@ -1456,6 +1527,14 @@ def _CheckNoDeprecatedJS(input_api, output_api):
   return results
 
 
+def _AndroidSpecificOnUploadChecks(input_api, output_api):
+  """Groups checks that target android code."""
+  results = []
+  results.extend(_CheckNoNewUtilLogUsage(input_api, output_api))
+  results.extend(_CheckAndroidCrLogUsage(input_api, output_api))
+  return results
+
+
 def _CommonChecks(input_api, output_api):
   """Checks common to both upload and commit."""
   results = []
@@ -1728,7 +1807,7 @@ def CheckChangeOnUpload(input_api, output_api):
   results.extend(
       input_api.canned_checks.CheckGNFormatted(input_api, output_api))
   results.extend(_CheckUmaHistogramChanges(input_api, output_api))
-  results.extend(_CheckNoNewUtilLogUsage(input_api, output_api))
+  results.extend(_AndroidSpecificOnUploadChecks(input_api, output_api))
   return results
 
 
diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py
index 195fe4f..613fd41 100755
--- a/PRESUBMIT_test.py
+++ b/PRESUBMIT_test.py
@@ -859,6 +859,77 @@ class LogUsageTest(unittest.TestCase):
     self.assertTrue('HasAndroidLog.java' in warnings[0].items[0])
     self.assertTrue('HasExplicitLog.java' in warnings[0].items[1])
 
+  def testCheckAndroidCrLogUsage(self):
+    mock_input_api = MockInputApi()
+    mock_output_api = MockOutputApi()
+
+    mock_input_api.files = [
+      MockAffectedFile('RandomStuff.java', [
+        'random stuff'
+      ]),
+      MockAffectedFile('HasCorrectTag.java', [
+        'import org.chromium.base.Log;',
+        'some random stuff',
+        'private static final String TAG = "cr.Foo";',
+        'Log.d(TAG, "foo");',
+      ]),
+      MockAffectedFile('HasShortCorrectTag.java', [
+        'import org.chromium.base.Log;',
+        'some random stuff',
+        'private static final String TAG = "cr";',
+        'Log.d(TAG, "foo");',
+      ]),
+      MockAffectedFile('HasNoTagDecl.java', [
+        'import org.chromium.base.Log;',
+        'some random stuff',
+        'Log.d(TAG, "foo");',
+      ]),
+      MockAffectedFile('HasIncorrectTagDecl.java', [
+        'import org.chromium.base.Log;',
+        'private static final String TAHG = "cr.Foo";',
+        'some random stuff',
+        'Log.d(TAG, "foo");',
+      ]),
+      MockAffectedFile('HasInlineTag.java', [
+        'import org.chromium.base.Log;',
+        'some random stuff',
+        'private static final String TAG = "cr.Foo";',
+        'Log.d("TAG", "foo");',
+      ]),
+      MockAffectedFile('HasIncorrectTag.java', [
+        'import org.chromium.base.Log;',
+        'some random stuff',
+        'private static final String TAG = "rubbish";',
+        'Log.d(TAG, "foo");',
+      ]),
+      MockAffectedFile('HasTooLongTag.java', [
+        'import org.chromium.base.Log;',
+        'some random stuff',
+        'private static final String TAG = "cr.24_charachers_long___";',
+        'Log.d(TAG, "foo");',
+      ]),
+    ]
+
+    msgs = PRESUBMIT._CheckAndroidCrLogUsage(
+        mock_input_api, mock_output_api)
+
+    self.assertEqual(3, len(msgs))
+
+    # Declaration format
+    self.assertEqual(3, len(msgs[0].items))
+    self.assertTrue('HasNoTagDecl.java' in msgs[0].items)
+    self.assertTrue('HasIncorrectTagDecl.java' in msgs[0].items)
+    self.assertTrue('HasIncorrectTag.java' in msgs[0].items)
+
+    # Tag length
+    self.assertEqual(1, len(msgs[1].items))
+    self.assertTrue('HasTooLongTag.java' in msgs[1].items)
+
+    # Tag must be a variable named TAG
+    self.assertEqual(1, len(msgs[2].items))
+    self.assertTrue('HasInlineTag.java:4' in msgs[2].items)
+
+
 
 if __name__ == '__main__':
   unittest.main()
diff --git a/base/android/java/src/org/chromium/base/Log.java b/base/android/java/src/org/chromium/base/Log.java
index b7fd774..c83cfe7 100644
--- a/base/android/java/src/org/chromium/base/Log.java
+++ b/base/android/java/src/org/chromium/base/Log.java
@@ -23,8 +23,6 @@ import java.util.Locale;
  * </p>
  */
 public class Log {
-    private static final String BASE_TAG = "cr";
-
     /** Convenience property, same as {@link android.util.Log#ASSERT}. */
     public static final int ASSERT = android.util.Log.ASSERT;
 
@@ -72,10 +70,12 @@ public class Log {
      *
      * @see android.util.Log#isLoggable(String, int)
      * @throws IllegalArgumentException if the tag is too long.
+     * @deprecated Directly use a string (e.g. "cr.Tag") in your class. See http://crbug.com/485772
      */
+    @Deprecated
     public static String makeTag(String groupTag) {
-        if (TextUtils.isEmpty(groupTag)) return BASE_TAG;
-        String tag = BASE_TAG + "." + groupTag;
+        if (TextUtils.isEmpty(groupTag)) return "cr";
+        String tag = "cr." + groupTag;
         if (tag.length() > 23) {
             throw new IllegalArgumentException(
                     "The full tag (" + tag + ") is longer than 23 characters.");
@@ -95,9 +95,7 @@ public class Log {
      * than 7 parameters, consider building your log message using a function annotated with
      * {@link NoSideEffects}.
      *
-     * @param tag Used to identify the source of a log message. Should be created using
-     *            {@link #makeTag(String)}.
-     *
+     * @param tag Used to identify the source of a log message.
      * @param messageTemplate The message you would like logged. It is to be specified as a format
      *                        string.
      * @param args Arguments referenced by the format specifiers in the format string. If the last
@@ -167,9 +165,7 @@ public class Log {
      * than 7 parameters, consider building your log message using a function annotated with
      * {@link NoSideEffects}.
      *
-     * @param tag Used to identify the source of a log message. Should be created using
-     *            {@link #makeTag(String)}.
-     *
+     * @param tag Used to identify the source of a log message.
      * @param messageTemplate The message you would like logged. It is to be specified as a format
      *                        string.
      * @param args Arguments referenced by the format specifiers in the format string. If the last
@@ -233,8 +229,7 @@ public class Log {
     /**
      * Sends an {@link android.util.Log#INFO} log message.
      *
-     * @param tag Used to identify the source of a log message. Should be created using
-     *            {@link #makeTag(String)}.
+     * @param tag Used to identify the source of a log message.
      * @param messageTemplate The message you would like logged. It is to be specified as a format
      *                        string.
      * @param args Arguments referenced by the format specifiers in the format string. If the last
@@ -255,8 +250,7 @@ public class Log {
     /**
      * Sends a {@link android.util.Log#WARN} log message.
      *
-     * @param tag Used to identify the source of a log message. Should be created using
-     *            {@link #makeTag(String)}.
+     * @param tag Used to identify the source of a log message.
      * @param messageTemplate The message you would like logged. It is to be specified as a format
      *                        string.
      * @param args Arguments referenced by the format specifiers in the format string. If the last
@@ -277,8 +271,7 @@ public class Log {
     /**
      * Sends an {@link android.util.Log#ERROR} log message.
      *
-     * @param tag Used to identify the source of a log message. Should be created using
-     *            {@link #makeTag(String)}.
+     * @param tag Used to identify the source of a log message.
      * @param messageTemplate The message you would like logged. It is to be specified as a format
      *                        string.
      * @param args Arguments referenced by the format specifiers in the format string. If the last
@@ -303,8 +296,7 @@ public class Log {
      *
      * @see android.util.Log#wtf(String, String, Throwable)
      *
-     * @param tag Used to identify the source of a log message. Should be created using
-     *            {@link #makeTag(String)}.
+     * @param tag Used to identify the source of a log message.
      * @param messageTemplate The message you would like logged. It is to be specified as a format
      *                        string.
      * @param args Arguments referenced by the format specifiers in the format string. If the last
diff --git a/base/android/java/src/org/chromium/base/README_logging.md b/base/android/java/src/org/chromium/base/README_logging.md
index d7fbcb4..a795b6b 100644
--- a/base/android/java/src/org/chromium/base/README_logging.md
+++ b/base/android/java/src/org/chromium/base/README_logging.md
@@ -9,7 +9,7 @@ or off for individual groups.
 
 Usage:
 
-    private static final String TAG = Log.makeTag("YourModuleTag");
+    private static final String TAG = "cr.YourModuleTag";
     ...
     Log.i(TAG, "Logged INFO message.");
     Log.d(TAG, "Some DEBUG info: %s", data);