Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 57355699 authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Add "cmd input_method tracing save-for-bugreport" 

This CL reworks our previous CL [1], which enabled "adb bugreport" to
include ime-tracing files into bugreport with some unpleasant
side-effect described in Bug 177462676.

Basically what this CL aims to do is to mirror my CL [2] for
InputMethodManagerService.  Instead of relying on heuristics, this CL
introduces a clear and dedicated command

  adb shell cmd input_method tracing save-for-bugreport

so that "adb bugreport" can explicitly tell IMMS to dump the tracing
files.

See the corresponding CL for frameworks/native [3] about how it's
triggered from "adb bugreport".

 [1]: Ie87eb8423e2bb70f28c330983d45b95e2e07062d
      ac24994a
 [2]: I887ae6941b4844a606675b447f67ecee88d7f192
      96a51f99
 [3]: I62105334e4efbb4514390ffa8be5416482ca3b29
      02b1d2c70b2f4703a9dd51ba69d78604d66916e1

Bug: 177462676
Test: Manually done as follows.
  1. adb shell cmd input_method tracing start
  2. adb logcat -d -s imeTracing:*
       Make sure ime tracing is running
  3. adb shell dumpsys input_method \
         --dump-priority CRITICAL --proto > /dev/null
  4. adb logcat -d -s imeTracing:*
       Make sure ime tracing has not been stopped.
  5. adb shell cmd input_method tracing save-for-bugreport
  6. adb logcat -d -s imeTracing:*
       Make sure ime-tracing was dumped and is still running.
  7. adb root && adb shell ls /data/misc/wmtrace/ -al
       Make sure ime tracing files are saved.
Test: Manually done as follows.
  1. adb shell cmd input_method tracing start
  2. adb logcat -d -s imeTracing:*
       Make sure ime tracing is running
  3. adb bugreport bugreport.zip
  4. adb logcat -d -s imeTracing:*
       Make sure ime-tracing was dumped and is still running.
  5. unzip -v bugreport.zip | grep ime_trace_
       Make sure ime_trace_{clients,service,managerservice}.pb are
       included.
Change-Id: Ib106e077a10eb6a4f37b38aa9172e7c99a81f898
parent 597dbb07

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

+10 −13
Original line number Diff line number Diff line
@@ -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

+0 −8
Original line number Diff line number Diff line
@@ -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

+28 −16
Original line number Diff line number Diff line
@@ -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,10 +199,36 @@ class ImeTracingServerImpl extends ImeTracing { 
                    + TRACE_FILENAME_CLIENTS + ", " + TRACE_FILENAME_IMS + ", "

                    + TRACE_FILENAME_IMMS);

            sEnabled = false;

            if (writeToFile) {

            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;

        }

    }



    private void resetBuffers() {

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

+7 −14
Original line number Diff line number Diff line
@@ -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;