[Chromecast] Added filter for log spam

Bug: internal b/70164086
Test: LogcatElisionUnitTest and submit feedback
Change-Id: I09bef97eed079c347d729416f6772937dcf3b5f8
Reviewed-on: https://chromium-review.googlesource.com/1034287Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Commit-Queue: Sandeep Vijayasekar <sandv@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555248}

chromecast/browser/android/BUILD.gn

@@ -175,6 +175,7 @@ junit_binary("cast_shell_junit_tests") {
   deps = [
     ":cast_intents_java",
     ":cast_shell_java",
+    "//base:base_java_test_support",
     "//base:base_junit_test_support",
     "//content/public/android:content_java",
     "//third_party/android_tools:android_support_core_utils_java",

chromecast/browser/android/apk/src/org/chromium/chromecast/shell/AndroidAppLogcatProvider.java

@@ -4,14 +4,15 @@
 
 package org.chromium.chromecast.shell;
 
+import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
-import org.chromium.chromecast.base.CircularBuffer;
 
 import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
 import java.io.IOException;
-import java.io.InputStreamReader;
-import java.nio.charset.Charset;
-import java.nio.charset.UnsupportedCharsetException;
+import java.io.Reader;
+import java.io.StringReader;
 
 /**
  * Extracts logcat out of Android devices and elide PII sensitive info from it.
@@ -25,39 +26,32 @@ class AndroidAppLogcatProvider extends ElidedLogcatProvider {
 
     @Override
     protected void getRawLogcat(RawLogcatCallback callback) {
-        CircularBuffer<String> rawLogcat = new CircularBuffer<>(BuildConfig.LOGCAT_SIZE);
-        String logLn = null;
         Integer exitValue = null;
 
+        Reader reader = new StringReader("");
         try {
-            Process p = Runtime.getRuntime().exec("logcat -d");
-            try (BufferedReader bReader = new BufferedReader(
-                         new InputStreamReader(p.getInputStream(), Charset.forName("UTF-8")))) {
-                while (exitValue == null) {
-                    while ((logLn = bReader.readLine()) != null) {
-                        // Add each new string to the end of the buffer.
-                        rawLogcat.add(logLn);
-                    }
-                    try {
-                        exitValue = p.exitValue();
-                    } catch (IllegalThreadStateException itse) {
-                        Thread.sleep(HALF_SECOND);
-                    }
-                }
-                if (exitValue != 0) {
-                    String msg = "Logcat process exit value: " + exitValue;
-                    Log.w(TAG, msg);
+            File outputDir = ContextUtils.getApplicationContext().getCacheDir();
+            File outputFile = File.createTempFile("temp_logcat", ".txt", outputDir);
+
+            Process p = Runtime.getRuntime().exec("logcat -d -f " + outputFile.getAbsolutePath());
+
+            while (exitValue == null) {
+                try {
+                    exitValue = p.exitValue();
+                } catch (IllegalThreadStateException itse) {
+                    Thread.sleep(HALF_SECOND);
                 }
-            } catch (UnsupportedCharsetException e) {
-                // Should never happen; all Java implementations are required to support UTF-8.
-                Log.wtf(TAG, "UTF-8 not supported", e);
-            } catch (InterruptedException e) {
-                Log.e(TAG, "Logcat subprocess interrupted ", e);
             }
-        } catch (IOException e) {
-            Log.e(TAG, "Error occurred trying to upload crash dump", e);
+            if (exitValue != 0) {
+                String msg = "Logcat process exit value: " + exitValue;
+                Log.w(TAG, msg);
+            }
+            reader = new FileReader(outputFile);
+
+        } catch (IOException | InterruptedException e) {
+            Log.e(TAG, "Error writing logcat", e);
         } finally {
-            callback.onLogsDone(rawLogcat);
+            callback.onLogsDone(new BufferedReader(reader));
         }
     }
 }

chromecast/browser/android/apk/src/org/chromium/chromecast/shell/ElidedLogcatProvider.java

@@ -4,8 +4,12 @@
 
 package org.chromium.chromecast.shell;
 
+import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
 
+import java.io.BufferedReader;
+import java.io.IOException;
+
 /**
  * Extracts logcat out of Android devices and elide PII sensitive info from it.
  *
@@ -13,22 +17,30 @@ import org.chromium.base.VisibleForTesting;
  * Javascript console messages.
  */
 abstract class ElidedLogcatProvider {
+    private static final String TAG = "cr_ElidedLogcatProvider";
+
     protected abstract void getRawLogcat(RawLogcatCallback rawLogcatCallback);
 
-    protected interface RawLogcatCallback { public void onLogsDone(Iterable<String> logs); }
+    protected interface RawLogcatCallback { public void onLogsDone(BufferedReader logsFileReader); }
     public interface LogcatCallback { public void onLogsDone(String logs); }
 
     public void getElidedLogcat(LogcatCallback callback) {
-        getRawLogcat((Iterable<String> rawLogs) -> callback.onLogsDone(elideLogcat(rawLogs)));
+        getRawLogcat((BufferedReader logsFileReader)
+                             -> callback.onLogsDone(elideLogcat(logsFileReader)));
     }
 
     @VisibleForTesting
-    protected static String elideLogcat(Iterable<String> rawLogcat) {
+    protected static String elideLogcat(BufferedReader logsFileReader) {
         StringBuilder builder = new StringBuilder();
-        for (String line : rawLogcat) {
-            builder.append(LogcatElision.elide(line));
-            builder.append("\n");
+        try (BufferedReader autoClosableBufferedReader = logsFileReader) {
+            String logLn;
+            while ((logLn = autoClosableBufferedReader.readLine()) != null) {
+                builder.append(LogcatElision.elide(logLn + "\n"));
+            }
+        } catch (IOException e) {
+            Log.e(TAG, "Can't read logs", e);
+        } finally {
+            return builder.toString();
         }
-        return builder.toString();
     }
-}
+}
\ No newline at end of file

chromecast/browser/android/apk/src/org/chromium/chromecast/shell/ExternalServiceDeviceLogcatProvider.java

@@ -13,11 +13,11 @@ import android.os.RemoteException;
 
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
-import org.chromium.chromecast.base.CircularBuffer;
 
 import java.io.BufferedReader;
+import java.io.FileNotFoundException;
 import java.io.FileReader;
-import java.io.IOException;
+import java.io.StringReader;
 
 /**
  * Gets file with system logs for the Assistant Devices and elide PII sensitive info from it.
@@ -44,28 +44,18 @@ class ExternalServiceDeviceLogcatProvider extends ElidedLogcatProvider {
                 IDeviceLogsProvider provider = IDeviceLogsProvider.Stub.asInterface(service);
 
                 ServiceConnection conn = this;
+
                 new AsyncTaskRunner().doAsync(() -> {
-                    CircularBuffer<String> rawLogcat =
-                            new CircularBuffer<>(BuildConfig.LOGCAT_SIZE);
+                    String logsFileName = "";
                     try {
                         // getLogs() currently gives us the filename of the location of the logs
-                        String logsFileName = provider.getLogs();
-                        Log.i(TAG, "Log Location: " + logsFileName);
-
-                        try (BufferedReader bReader =
-                                        new BufferedReader(new FileReader(logsFileName))) {
-                            String logLn;
-                            while ((logLn = bReader.readLine()) != null) {
-                                rawLogcat.add(logLn);
-                            }
-                        } catch (IOException e) {
-                            Log.e(TAG, "Can't read logs", e);
-                        }
-                    } catch (RemoteException e) {
+                        logsFileName = provider.getLogs();
+                        return new BufferedReader(new FileReader(logsFileName));
+                    } catch (FileNotFoundException | RemoteException e) {
                         Log.e(TAG, "Can't get logs", e);
+                        return new BufferedReader(new StringReader(""));
                     } finally {
                         ContextUtils.getApplicationContext().unbindService(conn);
-                        return rawLogcat;
                     }
                 }, callback::onLogsDone);
             }

chromecast/browser/android/apk/src/org/chromium/chromecast/shell/LogcatElision.java

@@ -71,6 +71,8 @@ class LogcatElision {
 
     private static final Pattern JAVA_FILE = Pattern.compile(".java:[0-9]+$");
 
+    private static final String[] LOG_SPAM = new String[] {"persist.mtk.mlog2logcat", "MLOG_KERN"};
+
     /**
      * Elides any emails in the specified {@link String} with {@link
      * #EMAIL_ELISION}.
@@ -134,6 +136,15 @@ class LogcatElision {
         return false;
     }
 
+    private static boolean likelyToBeLogSpam(String logline) {
+        for (String spam : LOG_SPAM) {
+            if (logline.contains(spam)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     public static boolean likelyToBeJavaFile(String url) {
         return JAVA_FILE.matcher(url).find();
     }
@@ -169,6 +180,7 @@ class LogcatElision {
     }
 
     public static String elide(String ln) {
+        if (likelyToBeLogSpam(ln)) return "";
         ln = elideEmail(ln);
         ln = elideIp(ln);
         ln = elideUrl(ln);

chromecast/browser/android/junit/src/org/chromium/chromecast/shell/ElidedLogcatProviderUnitTest.java

@@ -11,9 +11,13 @@ import static org.junit.Assert.assertThat;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.junit.runners.BlockJUnit4ClassRunner;
+import org.robolectric.annotation.Config;
 
-import java.util.ArrayList;
+import org.chromium.base.test.BaseRobolectricTestRunner;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.StringReader;
 import java.util.Arrays;
 import java.util.List;
 
@@ -23,32 +27,38 @@ import java.util.List;
  *
  * Full testing of elision of PII is done in LogcatElisionUnitTest.
  */
-@RunWith(BlockJUnit4ClassRunner.class)
+@RunWith(BaseRobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
 public class ElidedLogcatProviderUnitTest {
-    @Test
-    public void testEmptyLogcat() {
-        List<String> lines = new ArrayList<String>();
-        assertEquals("", ElidedLogcatProvider.elideLogcat(lines));
+    private static final String LOG_FILE_NAME = "log.txt";
+
+    private BufferedReader concatLines(List<String> contents) {
+        StringBuilder sb = new StringBuilder();
+        for (String s : contents) {
+            sb.append(s);
+            sb.append("\n");
+        }
+        return new BufferedReader(new StringReader(sb.toString()));
     }
 
     @Test
-    public void testLogcatOfEmptyString() {
+    public void testLogcatOfEmptyString() throws IOException {
         List<String> lines = Arrays.asList("");
-        assertEquals("\n", ElidedLogcatProvider.elideLogcat(lines));
+        assertEquals("\n", ElidedLogcatProvider.elideLogcat(concatLines(lines)));
     }
 
     @Test
-    public void testJoinsLines() {
-        List<String> lines = Arrays.asList("a", "b", "c");
-        assertEquals("a\nb\nc\n", ElidedLogcatProvider.elideLogcat(lines));
+    public void testPreservesLines() throws IOException {
+        List<String> lines = Arrays.asList("A", "B", "C");
+        assertEquals("A\nB\nC\n", ElidedLogcatProvider.elideLogcat(concatLines(lines)));
     }
 
     @Test
-    public void testElidesPii() {
+    public void testElidesPii() throws IOException {
         List<String> lines = Arrays.asList("email me at someguy@mailservice.com",
                 "file bugs at crbug.com", "at android.content.Intent", "at java.util.ArrayList",
                 "mac address: AB-AB-AB-AB-AB-AB");
-        String elided = ElidedLogcatProvider.elideLogcat(lines);
+        String elided = ElidedLogcatProvider.elideLogcat(concatLines(lines));
         // PII like email addresses, web addresses, and MAC addresses are elided.
         assertThat(elided, not(containsString("someguy@mailservice.com")));
         assertThat(elided, not(containsString("crbug.com")));
@@ -57,4 +67,26 @@ public class ElidedLogcatProviderUnitTest {
         assertThat(elided, containsString("android.content.Intent"));
         assertThat(elided, containsString("java.util.ArrayList"));
     }
+
+    @Test
+    public void testSpamRemoved() throws IOException {
+        List<String> lines = Arrays.asList(
+                "04-30 16:30:11.030 15721 15721 E libc    : Access denied finding property \"persist.mtk.mlog2logcat\"",
+                "04-30 16:30:51.590 15721 15721 W MLOG_KERN: type=1400 audit(0.0:137421): avc: denied",
+                "05-01 15:34:55.523   651   651 I chromium: [651:651:INFO:ccs_manager_impl.cc(891)] Waiting for all transports to be enabled");
+
+        String elided = ElidedLogcatProvider.elideLogcat(concatLines(lines));
+        assertThat(elided, not(containsString(lines.get(0))));
+        assertThat(elided, not(containsString(lines.get(1))));
+        assertThat(elided, containsString(lines.get(2)));
+    }
+
+    @Test
+    public void testSpamBecomesEmptyString() throws IOException {
+        List<String> lines = Arrays.asList(
+                "04-30 16:30:11.030 15721 15721 E libc    : Access denied finding property \"persist.mtk.mlog2logcat\"");
+
+        String elided = ElidedLogcatProvider.elideLogcat(concatLines(lines));
+        assertEquals(elided, "");
+    }
 }