Merge "[Rlog27] Refactor LogBuffer"

java/src/com/android/inputmethod/latin/LatinIME.java

@@ -2078,12 +2078,11 @@ public final class LatinIME extends InputMethodService implements KeyboardAction
             if (ProductionFlag.IS_INTERNAL) {
                 Stats.onAutoCorrection(typedWord, autoCorrection, separatorString, mWordComposer);
             }
-            mExpectingUpdateSelection = true;
             if (ProductionFlag.IS_EXPERIMENTAL) {
                 ResearchLogger.latinIme_commitCurrentAutoCorrection(typedWord, autoCorrection,
                         separatorString);
             }
-
+            mExpectingUpdateSelection = true;
             commitChosenWord(autoCorrection, LastComposedWord.COMMIT_TYPE_DECIDED_WORD,
                     separatorString);
             if (!typedWord.equals(autoCorrection)) {

java/src/com/android/inputmethod/research/FixedLogBuffer.java

@@ -0,0 +1,123 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.android.inputmethod.research;
+
+import java.util.LinkedList;
+
+/**
+ * A buffer that holds a fixed number of LogUnits.
+ *
+ * LogUnits are added in and shifted out in temporal order.  Only a subset of the LogUnits are
+ * actual words; the other LogUnits do not count toward the word limit.  Once the buffer reaches
+ * capacity, adding another LogUnit that is a word evicts the oldest LogUnits out one at a time to
+ * stay under the capacity limit.
+ *
+ * This variant of a LogBuffer has a limited memory footprint because of its limited size.  This
+ * makes it useful, for example, for recording a window of the user's most recent actions in case
+ * they want to report an observed error that they do not know how to reproduce.
+ */
+public class FixedLogBuffer extends LogBuffer {
+    /* package for test */ int mWordCapacity;
+    // The number of members of mLogUnits that are actual words.
+    private int mNumActualWords;
+
+    /**
+     * Create a new LogBuffer that can hold a fixed number of LogUnits that are words (and
+     * unlimited number of non-word LogUnits), and that outputs its result to a researchLog.
+     *
+     * @param wordCapacity maximum number of words
+     */
+    public FixedLogBuffer(final int wordCapacity) {
+        super();
+        if (wordCapacity <= 0) {
+            throw new IllegalArgumentException("wordCapacity must be 1 or greater.");
+        }
+        mWordCapacity = wordCapacity;
+        mNumActualWords = 0;
+    }
+
+    protected int getNumActualWords() {
+        return mNumActualWords;
+    }
+
+    /**
+     * Adds a new LogUnit to the front of the LIFO queue, evicting existing LogUnit's
+     * (oldest first) if word capacity is reached.
+     */
+    @Override
+    public void shiftIn(final LogUnit newLogUnit) {
+        if (newLogUnit.getWord() == null) {
+            // This LogUnit isn't a word, so it doesn't count toward the word-limit.
+            super.shiftIn(newLogUnit);
+            return;
+        }
+        if (mNumActualWords == mWordCapacity) {
+            shiftOutThroughFirstWord();
+        }
+        super.shiftIn(newLogUnit);
+        mNumActualWords++; // Must be a word, or we wouldn't be here.
+    }
+
+    private void shiftOutThroughFirstWord() {
+        final LinkedList<LogUnit> logUnits = getLogUnits();
+        while (!logUnits.isEmpty()) {
+            final LogUnit logUnit = logUnits.removeFirst();
+            onShiftOut(logUnit);
+            if (logUnit.hasWord()) {
+                // Successfully shifted out a word-containing LogUnit and made space for the new
+                // LogUnit.
+                mNumActualWords--;
+                break;
+            }
+        }
+    }
+
+    /**
+     * Removes all LogUnits from the buffer without calling onShiftOut().
+     */
+    @Override
+    public void clear() {
+        super.clear();
+        mNumActualWords = 0;
+    }
+
+    /**
+     * Called when a LogUnit is removed from the LogBuffer as a result of a shiftIn.  LogUnits are
+     * removed in the order entered.  This method is not called when shiftOut is called directly.
+     *
+     * Base class does nothing; subclasses may override if they want to record non-privacy sensitive
+     * events that fall off the end.
+     */
+    protected void onShiftOut(final LogUnit logUnit) {
+    }
+
+    /**
+     * Called to deliberately remove the oldest LogUnit.  Usually called when draining the
+     * LogBuffer.
+     */
+    @Override
+    public LogUnit shiftOut() {
+        if (isEmpty()) {
+            return null;
+        }
+        final LogUnit logUnit = super.shiftOut();
+        if (logUnit.hasWord()) {
+            mNumActualWords--;
+        }
+        return logUnit;
+    }
+}

java/src/com/android/inputmethod/research/LogBuffer.java

@@ -16,102 +16,44 @@
 
 package com.android.inputmethod.research;
 
-import com.android.inputmethod.latin.CollectionUtils;
-
 import java.util.LinkedList;
 
 /**
- * A buffer that holds a fixed number of LogUnits.
+ * Maintain a FIFO queue of LogUnits.
  *
- * LogUnits are added in and shifted out in temporal order.  Only a subset of the LogUnits are
- * actual words; the other LogUnits do not count toward the word limit.  Once the buffer reaches
- * capacity, adding another LogUnit that is a word evicts the oldest LogUnits out one at a time to
- * stay under the capacity limit.
+ * This class provides an unbounded queue.  This is useful when the user is aware that their actions
+ * are being recorded, such as when they are trying to reproduce a bug.  In this case, there should
+ * not be artificial restrictions on how many events that can be saved.
  */
 public class LogBuffer {
-    protected final LinkedList<LogUnit> mLogUnits;
-    /* package for test */ int mWordCapacity;
-    // The number of members of mLogUnits that are actual words.
-    protected int mNumActualWords;
+    // TODO: Gracefully handle situations in which this LogBuffer is consuming too much memory.
+    // This may happen, for example, if the user has forgotten that data is being logged.
+    private final LinkedList<LogUnit> mLogUnits;
 
-    /**
-     * Create a new LogBuffer that can hold a fixed number of LogUnits that are words (and
-     * unlimited number of non-word LogUnits), and that outputs its result to a researchLog.
-     *
-     * @param wordCapacity maximum number of words
-     */
-    LogBuffer(final int wordCapacity) {
-        if (wordCapacity <= 0) {
-            throw new IllegalArgumentException("wordCapacity must be 1 or greater.");
-        }
-        mLogUnits = CollectionUtils.newLinkedList();
-        mWordCapacity = wordCapacity;
-        mNumActualWords = 0;
+    public LogBuffer() {
+        mLogUnits = new LinkedList<LogUnit>();
     }
 
-    /**
-     * Adds a new LogUnit to the front of the LIFO queue, evicting existing LogUnit's
-     * (oldest first) if word capacity is reached.
-     */
-    public void shiftIn(LogUnit newLogUnit) {
-        if (newLogUnit.getWord() == null) {
-            // This LogUnit isn't a word, so it doesn't count toward the word-limit.
-            mLogUnits.add(newLogUnit);
-            return;
-        }
-        if (mNumActualWords == mWordCapacity) {
-            shiftOutThroughFirstWord();
-        }
-        mLogUnits.add(newLogUnit);
-        mNumActualWords++; // Must be a word, or we wouldn't be here.
+    protected LinkedList<LogUnit> getLogUnits() {
+        return mLogUnits;
     }
 
-    private void shiftOutThroughFirstWord() {
-        while (!mLogUnits.isEmpty()) {
-            final LogUnit logUnit = mLogUnits.removeFirst();
-            onShiftOut(logUnit);
-            if (logUnit.hasWord()) {
-                // Successfully shifted out a word-containing LogUnit and made space for the new
-                // LogUnit.
-                mNumActualWords--;
-                break;
-            }
-        }
-    }
-
-    /**
-     * Removes all LogUnits from the buffer without calling onShiftOut().
-     */
     public void clear() {
         mLogUnits.clear();
-        mNumActualWords = 0;
     }
 
-    /**
-     * Called when a LogUnit is removed from the LogBuffer as a result of a shiftIn.  LogUnits are
-     * removed in the order entered.  This method is not called when shiftOut is called directly.
-     *
-     * Base class does nothing; subclasses may override.
-     */
-    protected void onShiftOut(LogUnit logUnit) {
-    }
-
-    /**
-     * Called to deliberately remove the oldest LogUnit.  Usually called when draining the
-     * LogBuffer.
-     */
-    public LogUnit shiftOut() {
-        if (mLogUnits.isEmpty()) {
-            return null;
-        }
-        final LogUnit logUnit = mLogUnits.removeFirst();
-        if (logUnit.hasWord()) {
-            mNumActualWords--;
-        }
-        return logUnit;
+    public void shiftIn(final LogUnit logUnit) {
+        mLogUnits.add(logUnit);
     }
 
     public boolean isEmpty() {
         return mLogUnits.isEmpty();
     }
+
+    public LogUnit shiftOut() {
+        if (isEmpty()) {
+            return null;
+        }
+        return mLogUnits.removeFirst();
+    }
 }

java/src/com/android/inputmethod/research/MainLogBuffer.java

@@ -22,15 +22,24 @@ import com.android.inputmethod.latin.Dictionary;
 import com.android.inputmethod.latin.Suggest;
 import com.android.inputmethod.latin.define.ProductionFlag;
 
+import java.util.LinkedList;
 import java.util.Random;
 
-public class MainLogBuffer extends LogBuffer {
+/**
+ * Provide a log buffer of fixed length that enforces privacy restrictions.
+ *
+ * The privacy restrictions include making sure that no numbers are logged, that all logged words
+ * are in the dictionary, and that words are recorded infrequently enough that the user's meaning
+ * cannot be easily determined.
+ */
+public class MainLogBuffer extends FixedLogBuffer {
     private static final String TAG = MainLogBuffer.class.getSimpleName();
     private static final boolean DEBUG = false && ProductionFlag.IS_EXPERIMENTAL_DEBUG;
 
     // The size of the n-grams logged.  E.g. N_GRAM_SIZE = 2 means to sample bigrams.
     private static final int N_GRAM_SIZE = 2;
-    // The number of words between n-grams to omit from the log.
+    // The number of words between n-grams to omit from the log.  If debugging, record 50% of all
+    // words.  Otherwise, only record 10%.
     private static final int DEFAULT_NUMBER_OF_WORDS_BETWEEN_SAMPLES =
             ProductionFlag.IS_EXPERIMENTAL_DEBUG ? 2 : 18;
 
@@ -56,7 +65,7 @@ public class MainLogBuffer extends LogBuffer {
         mWordsUntilSafeToSample = random.nextInt(mMinWordPeriod);
     }
 
-    public void setSuggest(Suggest suggest) {
+    public void setSuggest(final Suggest suggest) {
         mSuggest = suggest;
     }
 
@@ -108,9 +117,10 @@ public class MainLogBuffer extends LogBuffer {
         }
         // Check each word in the buffer.  If any word poses a privacy threat, we cannot upload the
         // complete buffer contents in detail.
-        final int length = mLogUnits.size();
+        final LinkedList<LogUnit> logUnits = getLogUnits();
+        final int length = logUnits.size();
         for (int i = 0; i < length; i++) {
-            final LogUnit logUnit = mLogUnits.get(i);
+            final LogUnit logUnit = logUnits.get(i);
             final String word = logUnit.getWord();
             if (word == null) {
                 // Digits outside words are a privacy threat.
@@ -133,7 +143,7 @@ public class MainLogBuffer extends LogBuffer {
     }
 
     @Override
-    protected void onShiftOut(LogUnit logUnit) {
+    protected void onShiftOut(final LogUnit logUnit) {
         if (mResearchLog != null) {
             mResearchLog.publish(logUnit, false /* isIncludingPrivateData */);
         }

java/src/com/android/inputmethod/research/ResearchLogger.java

@@ -377,7 +377,7 @@ public class ResearchLogger implements SharedPreferences.OnSharedPreferenceChang
             mFeedbackLog = new ResearchLog(createLogFile(mFilesDir));
             // LogBuffer is one more than FEEDBACK_WORD_BUFFER_SIZE, because it must also hold
             // the feedback LogUnit itself.
-            mFeedbackLogBuffer = new LogBuffer(FEEDBACK_WORD_BUFFER_SIZE + 1);
+            mFeedbackLogBuffer = new FixedLogBuffer(FEEDBACK_WORD_BUFFER_SIZE + 1);
         }
     }