Merge "Add "cmd input_method tracing save-for-bugreport"" into sc-dev

core/java/android/util/imetracing/ImeTracing.java

@@ -129,6 +129,16 @@ public abstract class ImeTracing {
      */
     public abstract void triggerManagerServiceDump(String where);
 
+    /**
+     * Being called while taking a bugreport so that tracing files can be included in the bugreport
+     * when the IME tracing is running.  Does nothing otherwise.
+     *
+     * @param pw Print writer
+     */
+    public void saveForBugreport(@Nullable PrintWriter pw) {
+        // does nothing by default.
+    }
+
     /**
      * Sets whether ime tracing is enabled.
      *
@@ -152,11 +162,6 @@ public abstract class ImeTracing {
         return mService != null;
     }
 
-    /**
-     * Writes the current tracing data to the specific output proto file.
-     */
-    public abstract void writeTracesToFiles();
-
     /**
      * Starts a new IME trace if one is not already started.
      *
@@ -171,14 +176,6 @@ public abstract class ImeTracing {
      */
     public abstract void stopTrace(@Nullable PrintWriter pw);
 
-    /**
-     * Stops the IME trace if one was previously started.
-     *
-     * @param pw Print writer
-     * @param writeToFile If the current buffer should be written to disk or not
-     */
-    public abstract void stopTrace(@Nullable PrintWriter pw, boolean writeToFile);
-
     private static boolean isSystemProcess() {
         return ActivityThread.isSystem();
     }

core/java/android/util/imetracing/ImeTracingClientImpl.java

@@ -98,10 +98,6 @@ class ImeTracingClientImpl extends ImeTracing {
         // Intentionally left empty, this is implemented in ImeTracingServerImpl
     }
 
-    @Override
-    public void writeTracesToFiles() {
-    }
-
     @Override
     public void startTrace(PrintWriter pw) {
     }
@@ -109,8 +105,4 @@ class ImeTracingClientImpl extends ImeTracing {
     @Override
     public void stopTrace(PrintWriter pw) {
     }
-
-    @Override
-    public void stopTrace(PrintWriter pw, boolean writeToFile) {
-    }
 }

core/java/android/util/imetracing/ImeTracingServerImpl.java

@@ -139,14 +139,6 @@ class ImeTracingServerImpl extends ImeTracing {
         }
     }
 
-    @GuardedBy("mEnabledLock")
-    @Override
-    public void writeTracesToFiles() {
-        synchronized (mEnabledLock) {
-            writeTracesToFilesLocked();
-        }
-    }
-
     private void writeTracesToFilesLocked() {
         try {
             ProtoOutputStream clientsProto = new ProtoOutputStream();
@@ -192,12 +184,6 @@ class ImeTracingServerImpl extends ImeTracing {
 
     @Override
     public void stopTrace(@Nullable PrintWriter pw) {
-        stopTrace(pw, true /* writeToFile */);
-    }
-
-    @GuardedBy("mEnabledLock")
-    @Override
-    public void stopTrace(@Nullable PrintWriter pw, boolean writeToFile) {
         if (IS_USER) {
             Log.w(TAG, "Warn: Tracing is not supported on user builds.");
             return;
@@ -213,9 +199,35 @@ class ImeTracingServerImpl extends ImeTracing {
                     + TRACE_FILENAME_CLIENTS + ", " + TRACE_FILENAME_IMS + ", "
                     + TRACE_FILENAME_IMMS);
             sEnabled = false;
-            if (writeToFile) {
-                writeTracesToFilesLocked();
+            writeTracesToFilesLocked();
+        }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public void saveForBugreport(@Nullable PrintWriter pw) {
+        if (IS_USER) {
+            return;
+        }
+        synchronized (mEnabledLock) {
+            if (!isAvailable() || !isEnabled()) {
+                return;
             }
+            // Temporarily stop accepting logs from trace event providers.  There is a small chance
+            // that we may drop some trace events while writing the file, but we currently need to
+            // live with that.  Note that addToBuffer() also has a bug that it doesn't do
+            // read-acquire so flipping sEnabled here doesn't even guarantee that addToBuffer() will
+            // temporarily stop accepting incoming events...
+            // TODO(b/175761228): Implement atomic snapshot to avoid downtime.
+            // TODO(b/175761228): Fix synchronization around sEnabled.
+            sEnabled = false;
+            logAndPrintln(pw, "Writing traces in " + TRACE_DIRNAME + ": "
+                    + TRACE_FILENAME_CLIENTS + ", " + TRACE_FILENAME_IMS + ", "
+                    + TRACE_FILENAME_IMMS);
+            writeTracesToFilesLocked();
+            sEnabled = true;
         }
     }
 

services/core/java/com/android/server/inputmethod/InputMethodManagerService.java

@@ -175,7 +175,6 @@ import com.android.internal.inputmethod.StartInputReason;
 import com.android.internal.inputmethod.UnbindReason;
 import com.android.internal.messages.nano.SystemMessageProto.SystemMessage;
 import com.android.internal.notification.SystemNotificationChannels;
-import com.android.internal.os.BackgroundThread;
 import com.android.internal.os.HandlerCaller;
 import com.android.internal.os.SomeArgs;
 import com.android.internal.os.TransferPipe;
@@ -5230,15 +5229,6 @@ public class InputMethodManagerService extends IInputMethodManager.Stub
         if (!DumpUtils.checkDumpPermission(mContext, TAG, pw)) return;
 
         if (ArrayUtils.contains(args, PROTO_ARG)) {
-            final ImeTracing imeTracing = ImeTracing.getInstance();
-            if (imeTracing.isEnabled()) {
-                imeTracing.stopTrace(null, false /* writeToFile */);
-                BackgroundThread.getHandler().post(() -> {
-                    imeTracing.writeTracesToFiles();
-                    imeTracing.startTrace(null);
-                });
-            }
-
             final ProtoOutputStream proto = new ProtoOutputStream(fd);
             dumpDebug(proto, InputMethodManagerServiceTraceProto.INPUT_METHOD_MANAGER_SERVICE);
             proto.flush();
@@ -5816,7 +5806,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub
     }
 
     /**
-     * Handles {@code adb shell ime tracing start/stop}.
+     * Handles {@code adb shell cmd input_method tracing start/stop/save-for-bugreport}.
      * @param shellCommand {@link ShellCommand} object that is handling this command.
      * @return Exit code of the command.
      */
@@ -5828,16 +5818,19 @@ public class InputMethodManagerService extends IInputMethodManager.Stub
         switch (cmd) {
             case "start":
                 ImeTracing.getInstance().getInstance().startTrace(pw);
-                break;
+                break;  // proceed to the next step to update the IME client processes.
             case "stop":
                 ImeTracing.getInstance().stopTrace(pw);
-                break;
+                break;  // proceed to the next step to update the IME client processes.
+            case "save-for-bugreport":
+                ImeTracing.getInstance().saveForBugreport(pw);
+                return ShellCommandResult.SUCCESS;  // no need to update the IME client processes.
             default:
                 pw.println("Unknown command: " + cmd);
                 pw.println("Input method trace options:");
                 pw.println("  start: Start tracing");
                 pw.println("  stop: Stop tracing");
-                return ShellCommandResult.FAILURE;
+                return ShellCommandResult.FAILURE;  // no need to update the IME client processes.
         }
         boolean isImeTraceEnabled = ImeTracing.getInstance().isEnabled();
         ArrayMap<IBinder, ClientState> clients;