[Android] Read the *last* N lines of a logcat to associate with a crash.

chrome/android/java/src/org/chromium/chrome/browser/crash/LogcatExtractionCallable.java

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chrome.browser.crash;
 
 import android.content.Context;
 import android.content.Intent;
 import android.util.Patterns;
 
@@ skipping 212 matching lines @@
     }
 
     private static void move(File from, File to) {
         if (!from.renameTo(to)) {
             Log.w(TAG, "Fail to rename logcat file");
         }
     }
 
     @VisibleForTesting
     protected List<String> getLogcat() throws IOException, InterruptedException {
-        return getLogcatInternal();
-    }
+        List<String> rawLogcat = new LinkedList<>();
+        boolean inMicrodump = false;
 
-    private static List<String> getLogcatInternal() throws IOException, InterruptedException {
-        List<String> rawLogcat = null;
         Integer exitValue = null;
-        // In the absence of the android.permission.READ_LOGS permission the
-        // the logcat call will just hang.
+        // In the absence of the android.permission.READ_LOGS permission, the logcat call will just
+        // hang.

[Ilya Sherman, 2016/09/01 02:00:03]

FWIW, I don't understand the relevance of this comment.  I see no code that
handles a hang, nor tests the permission.  Alan, do you know why this is here?

[acleung1, 2016/09/01 17:54:42]

This comment is some what irrelevant. The issue is that before a certain version
of Android, it hangs, there might been a version check around these lines
before.

Someone might have done a code clean up and removed that version check after
Chrome no longer support those versions.

I say we remove this.

[Ilya Sherman, 2016/09/01 21:46:58]

Okay, removed -- thanks.

         Process p = Runtime.getRuntime().exec("logcat -d");
         BufferedReader reader = null;
         try {
             reader = new BufferedReader(new InputStreamReader(p.getInputStream()));
             while (exitValue == null) {
-                rawLogcat = extractLogcatFromReader(reader, LOGCAT_SIZE);
+                inMicrodump =
+                        extractLogcatChunkFromReader(reader, LOGCAT_SIZE, inMicrodump, rawLogcat);

[Ilya Sherman, 2016/09/01 02:00:03]

It feels kind of hacky to read the logcat in chunks.  Would it perhaps make more
sense to first allow the command to finish executing; and then once it's fully
executed, to read the output?  (The only reason I didn't immediately implement
it that way is that I assumed there was probably a good reason to follow the
structure that the code currently follows -- e.g. maybe not wanting to buffer
too much in memory at a time?)

[acleung1, 2016/09/01 17:54:42]

Not that I know of.

[Ilya Sherman, 2016/09/01 21:46:58]

Maria, WDYT?

                 try {
                     exitValue = p.exitValue();
                 } catch (IllegalThreadStateException itse) {
+                    // The logcat dump has not yet completed; give it some more time.
                     Thread.sleep(HALF_SECOND);
                 }
             }
             if (exitValue != 0) {
                 String msg = "Logcat failed: " + exitValue;
                 Log.w(TAG, msg);
                 throw new IOException(msg);
             }
             return rawLogcat;
         } finally {
             if (reader != null) {
                 reader.close();
             }
         }
     }
 
     /**
      * Extract microdump-free logcat for more informative crash reports
      *
      * @param reader A buffered reader from which lines of initial logcat is read.
-     * @param maxLines The maximum number of lines logcat extracts from minidump.
+     * @param maxLines The maximum number of lines logcat extracts from logcat.
+     * @param inMicrodump Whether the upcoming lines in the logcat are known to be part of a
+     *         microdump.
+     * @param rawLogcat The tail of the logcat that has been read so far, as a list of strings.
      *
-     * @return Logcat up to specified length as a list of strings.
+     * @return Whether the next lines to be read from the logcat are known to be part of a
+     *         microdump.
      * @throws IOException if the buffered reader encounters an I/O error.
      */
     @VisibleForTesting
-    protected static List<String> extractLogcatFromReader(
-            BufferedReader reader, int maxLines) throws IOException {
-        return extractLogcatFromReaderInternal(reader, maxLines);
-    }
-
-    private static List<String> extractLogcatFromReaderInternal(
-            BufferedReader reader, int maxLines) throws IOException {
-        boolean inMicrodump = false;
-        List<String> rawLogcat = new LinkedList<>();
+    protected static boolean extractLogcatChunkFromReader(BufferedReader reader, int maxLines,
+            boolean inMicrodump, List<String> rawLogcat) throws IOException {
         String logLn;
-        while ((logLn = reader.readLine()) != null && rawLogcat.size() < maxLines) {
+        while ((logLn = reader.readLine()) != null) {
             if (logLn.contains(BEGIN_MICRODUMP)) {
                 // If the log contains two begin markers without an end marker
                 // in between, we ignore the second begin marker.
                 inMicrodump = true;
             } else if (logLn.contains(END_MICRODUMP)) {
                 if (!inMicrodump) {
                     // If we have been extracting microdump the whole time,
                     // start over with a clean logcat.
                     rawLogcat.clear();
                 } else {
                     inMicrodump = false;
                 }
             } else {
                 if (!inMicrodump) {
                     rawLogcat.add(logLn);
+                    if (rawLogcat.size() > maxLines) {
+                        rawLogcat.remove(0);
+                    }
                 }
             }
         }
-        return rawLogcat;
+        return inMicrodump;
     }
 
     private File writeLogcat(List<String> elidedLogcat) throws IOException {
         CrashFileManager fileManager = new CrashFileManager(mContext.getCacheDir());
         File logcatFile = fileManager.createNewTempFile("logcat.txt");
         PrintWriter pWriter = null;
         try {
             pWriter = new PrintWriter(new FileWriter(logcatFile));
             for (String ln : elidedLogcat) {
                 pWriter.println(ln);
@@ skipping 105 matching lines @@
      * {@link #CONSOLE_ELISION}.
      *
      * @param original String potentially containing console messages.
      * @return String with elided console messages.
      */
     @VisibleForTesting
     protected static String elideConsole(String original) {
         return CONSOLE_MSG.matcher(original).replaceAll(CONSOLE_ELISION);
     }
 }