From 8418bef70abf31d3cbfcfa500b2e694988735557 Mon Sep 17 00:00:00 2001 From: Ioana Stefan Date: Tue, 27 Oct 2020 11:30:38 +0000 Subject: [PATCH] Optimized workflow for IME tracing on InputMethodService side Optimized the tracing logic for the IMS information. The InputMethodService triggers a tracing dump through the new method triggerServiceDump, exposed by the meTracing interface. This change was done to be able to support custom dumps from clients and custom dumps from InputMethodService. This change only covers the IMS information. The IMMS information will be dumped in next changes. Bug: 154348613 Test: start IME tracing by calling "adb shell ime tracing start" end IME tracing by calling "adb shell ime tracing stop" pull trace using "adb pull /data/misc/wmtrace/ime_trace_service.pb ime_trace_service.pb" Change-Id: Icda0f82d76fb7db5b2bd8d021069b1ff15a4e15b --- .../AbstractInputMethodService.java | 9 ++-- .../IInputMethodWrapper.java | 17 +------ .../InputMethodService.java | 21 ++++++-- .../android/util/imetracing/ImeTracing.java | 4 +- .../util/imetracing/ImeTracingClientImpl.java | 25 ++++++++-- .../util/imetracing/ImeTracingServerImpl.java | 50 +++++++++++++------ .../InputMethodManagerService.java | 10 +++- 7 files changed, 92 insertions(+), 44 deletions(-) diff --git a/core/java/android/inputmethodservice/AbstractInputMethodService.java b/core/java/android/inputmethodservice/AbstractInputMethodService.java index cd436374b489b..7cf0b10031acc 100644 --- a/core/java/android/inputmethodservice/AbstractInputMethodService.java +++ b/core/java/android/inputmethodservice/AbstractInputMethodService.java @@ -21,6 +21,7 @@ import android.annotation.NonNull; import android.app.Service; import android.content.Intent; import android.os.IBinder; +import android.util.proto.ProtoOutputStream; import android.view.KeyEvent; import android.view.MotionEvent; import android.view.inputmethod.InputConnection; @@ -195,15 +196,13 @@ public abstract class AbstractInputMethodService extends Service public abstract AbstractInputMethodSessionImpl onCreateInputMethodSessionInterface(); /** - * Dumps the internal state of IME to a protocol buffer output stream initialized using the - * given {@link FileDescriptor}. + * Dumps the internal state of IME to a protocol buffer output stream. * - * @param fd The file descriptor to which proto dump should be written. - * @param args The arguments passed to the dump method. + * @param proto ProtoOutputStream to dump data to. * @hide */ @SuppressWarnings("HiddenAbstractMethod") - abstract void dumpProtoInternal(FileDescriptor fd, String[] args); + public abstract void dumpProtoInternal(ProtoOutputStream proto); /** * Implement this to handle {@link android.os.Binder#dump Binder.dump()} diff --git a/core/java/android/inputmethodservice/IInputMethodWrapper.java b/core/java/android/inputmethodservice/IInputMethodWrapper.java index 3d1755e9b0176..5cfcd667632b0 100644 --- a/core/java/android/inputmethodservice/IInputMethodWrapper.java +++ b/core/java/android/inputmethodservice/IInputMethodWrapper.java @@ -16,8 +16,6 @@ package android.inputmethodservice; -import static android.util.imetracing.ImeTracing.PROTO_ARG; - import android.annotation.BinderThread; import android.annotation.MainThread; import android.annotation.Nullable; @@ -157,20 +155,9 @@ class IInputMethodWrapper extends IInputMethod.Stub return; } SomeArgs args = (SomeArgs)msg.obj; - String[] dumpArgs = (String[]) args.arg3; - boolean protoDumpRequested = false; - for (String arg : dumpArgs) { - if (arg.equals(PROTO_ARG)) { - protoDumpRequested = true; - break; - } - } try { - if (protoDumpRequested) { - target.dumpProtoInternal((FileDescriptor) args.arg1, dumpArgs); - } else { - target.dump((FileDescriptor) args.arg1, (PrintWriter) args.arg2, dumpArgs); - } + target.dump((FileDescriptor) args.arg1, + (PrintWriter) args.arg2, (String[]) args.arg3); } catch (RuntimeException e) { ((PrintWriter)args.arg2).println("Exception: " + e); } diff --git a/core/java/android/inputmethodservice/InputMethodService.java b/core/java/android/inputmethodservice/InputMethodService.java index 44640c44332ee..fe8f3d7edae0b 100644 --- a/core/java/android/inputmethodservice/InputMethodService.java +++ b/core/java/android/inputmethodservice/InputMethodService.java @@ -90,6 +90,7 @@ import android.text.method.MovementMethod; import android.util.Log; import android.util.PrintWriterPrinter; import android.util.Printer; +import android.util.imetracing.ImeTracing; import android.util.proto.ProtoOutputStream; import android.view.Gravity; import android.view.KeyCharacterMap; @@ -116,6 +117,7 @@ import android.view.inputmethod.InputBinding; import android.view.inputmethod.InputConnection; import android.view.inputmethod.InputContentInfo; import android.view.inputmethod.InputMethod; +import android.view.inputmethod.InputMethodEditorTraceProto.InputMethodServiceTraceProto; import android.view.inputmethod.InputMethodManager; import android.view.inputmethod.InputMethodSubtype; import android.widget.FrameLayout; @@ -708,6 +710,8 @@ public class InputMethodService extends AbstractInputMethodService { + " Use requestHideSelf(int) itself"); return; } + ImeTracing.getInstance().triggerServiceDump( + "InputMethodService.InputMethodImpl#hideSoftInput", InputMethodService.this); final boolean wasVisible = isInputViewShown(); Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "IMS.hideSoftInput"); @@ -762,6 +766,8 @@ public class InputMethodService extends AbstractInputMethodService { Binder.disableTracing(); } Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "IMS.showSoftInput"); + ImeTracing.getInstance().triggerServiceDump( + "InputMethodService.InputMethodImpl#showSoftInput", InputMethodService.this); final boolean wasVisible = isInputViewShown(); if (dispatchOnShowInputRequested(flags, false)) { @@ -2138,6 +2144,8 @@ public class InputMethodService extends AbstractInputMethodService { return; } + ImeTracing.getInstance().triggerServiceDump("InputMethodService#showWindow", this); + mDecorViewWasVisible = mDecorViewVisible; mInShowWindow = true; final int previousImeWindowStatus = @@ -2212,6 +2220,8 @@ public class InputMethodService extends AbstractInputMethodService { * @param setVisible {@code true} to make it visible, false to hide it. */ private void applyVisibilityInInsetsConsumerIfNecessary(boolean setVisible) { + ImeTracing.getInstance().triggerServiceDump( + "InputMethodService#applyVisibilityInInsetsConsumerIfNecessary", this); mPrivOps.applyImeVisibility(setVisible ? mCurShowInputToken : mCurHideInputToken, setVisible); } @@ -2236,6 +2246,7 @@ public class InputMethodService extends AbstractInputMethodService { public void hideWindow() { if (DEBUG) Log.v(TAG, "CALL: hideWindow"); + ImeTracing.getInstance().triggerServiceDump("InputMethodService#hideWindow", this); mWindowVisible = false; finishViews(false /* finishingInput */); if (mDecorViewVisible) { @@ -2306,6 +2317,7 @@ public class InputMethodService extends AbstractInputMethodService { void doFinishInput() { if (DEBUG) Log.v(TAG, "CALL: doFinishInput"); + ImeTracing.getInstance().triggerServiceDump("InputMethodService#doFinishInput", this); finishViews(true /* finishingInput */); if (mInputStarted) { mInlineSuggestionSessionController.notifyOnFinishInput(); @@ -2321,6 +2333,7 @@ public class InputMethodService extends AbstractInputMethodService { if (!restarting) { doFinishInput(); } + ImeTracing.getInstance().triggerServiceDump("InputMethodService#doStartInput", this); mInputStarted = true; mStartedInputConnection = ic; mInputEditorInfo = attribute; @@ -2479,6 +2492,7 @@ public class InputMethodService extends AbstractInputMethodService { * @param flags Provides additional operating flags. */ public void requestHideSelf(int flags) { + ImeTracing.getInstance().triggerServiceDump("InputMethodService#requestHideSelf", this); mPrivOps.hideMySoftInput(flags); } @@ -2491,6 +2505,7 @@ public class InputMethodService extends AbstractInputMethodService { * @param flags Provides additional operating flags. */ public final void requestShowSelf(int flags) { + ImeTracing.getInstance().triggerServiceDump("InputMethodService#requestShowSelf", this); mPrivOps.showMySoftInput(flags); } @@ -3310,8 +3325,8 @@ public class InputMethodService extends AbstractInputMethodService { * @hide */ @Override - final void dumpProtoInternal(FileDescriptor fd, String[] args) { - final ProtoOutputStream proto = new ProtoOutputStream(fd); + public final void dumpProtoInternal(ProtoOutputStream proto) { + final long token = proto.start(InputMethodServiceTraceProto.INPUT_METHOD_SERVICE); mWindow.dumpDebug(proto, SOFT_INPUT_WINDOW); proto.write(VIEWS_CREATED, mViewsCreated); proto.write(DECOR_VIEW_VISIBLE, mDecorViewVisible); @@ -3339,6 +3354,6 @@ public class InputMethodService extends AbstractInputMethodService { proto.write(STATUS_ICON, mStatusIcon); mTmpInsets.dumpDebug(proto, LAST_COMPUTED_INSETS); proto.write(SETTINGS_OBSERVER, Objects.toString(mSettingsObserver)); - proto.flush(); + proto.end(token); } } diff --git a/core/java/android/util/imetracing/ImeTracing.java b/core/java/android/util/imetracing/ImeTracing.java index 5c155e20d0878..a43c87203e032 100644 --- a/core/java/android/util/imetracing/ImeTracing.java +++ b/core/java/android/util/imetracing/ImeTracing.java @@ -18,7 +18,7 @@ package android.util.imetracing; import android.app.ActivityThread; import android.content.Context; -import android.inputmethodservice.InputMethodService; +import android.inputmethodservice.AbstractInputMethodService; import android.os.RemoteException; import android.os.ServiceManager; import android.os.ServiceManager.ServiceNotFoundException; @@ -113,7 +113,7 @@ public abstract class ImeTracing { * * @param where Place where the trace was triggered. */ - public abstract void triggerServiceDump(String where, InputMethodService service); + public abstract void triggerServiceDump(String where, AbstractInputMethodService service); /** * Starts a proto dump of the InputMethodManagerService information. diff --git a/core/java/android/util/imetracing/ImeTracingClientImpl.java b/core/java/android/util/imetracing/ImeTracingClientImpl.java index 8330572e60ff8..9c21f4dd39dfd 100644 --- a/core/java/android/util/imetracing/ImeTracingClientImpl.java +++ b/core/java/android/util/imetracing/ImeTracingClientImpl.java @@ -16,7 +16,7 @@ package android.util.imetracing; -import android.inputmethodservice.InputMethodService; +import android.inputmethodservice.AbstractInputMethodService; import android.os.RemoteException; import android.os.ServiceManager.ServiceNotFoundException; import android.os.ShellCommand; @@ -66,8 +66,27 @@ class ImeTracingClientImpl extends ImeTracing { } @Override - public void triggerServiceDump(String where, InputMethodService service) { - // TODO (b/154348613) + public void triggerServiceDump(String where, AbstractInputMethodService service) { + if (!isEnabled() || !isAvailable()) { + return; + } + + synchronized (mDumpInProgressLock) { + if (mDumpInProgress) { + return; + } + mDumpInProgress = true; + } + + try { + ProtoOutputStream proto = new ProtoOutputStream(); + service.dumpProtoInternal(proto); + sendToService(proto.getBytes(), IME_TRACING_FROM_IMS, where); + } catch (RemoteException e) { + Log.e(TAG, "Exception while sending ime-related service dump to server", e); + } finally { + mDumpInProgress = false; + } } @Override diff --git a/core/java/android/util/imetracing/ImeTracingServerImpl.java b/core/java/android/util/imetracing/ImeTracingServerImpl.java index a8c0f66f15f0b..d2147f8e89f31 100644 --- a/core/java/android/util/imetracing/ImeTracingServerImpl.java +++ b/core/java/android/util/imetracing/ImeTracingServerImpl.java @@ -17,15 +17,14 @@ package android.util.imetracing; import static android.os.Build.IS_USER; -import static android.view.inputmethod.InputMethodEditorTraceProto.InputMethodClientsTraceFileProto.MAGIC_NUMBER; -import static android.view.inputmethod.InputMethodEditorTraceProto.InputMethodClientsTraceFileProto.MAGIC_NUMBER_H; -import static android.view.inputmethod.InputMethodEditorTraceProto.InputMethodClientsTraceFileProto.MAGIC_NUMBER_L; -import android.inputmethodservice.InputMethodService; +import android.inputmethodservice.AbstractInputMethodService; import android.os.ServiceManager.ServiceNotFoundException; import android.os.ShellCommand; import android.util.Log; import android.util.proto.ProtoOutputStream; +import android.view.inputmethod.InputMethodEditorTraceProto.InputMethodClientsTraceFileProto; +import android.view.inputmethod.InputMethodEditorTraceProto.InputMethodServiceTraceFileProto; import com.android.internal.annotations.GuardedBy; import com.android.internal.util.TraceBuffer; @@ -40,25 +39,37 @@ import java.io.PrintWriter; class ImeTracingServerImpl extends ImeTracing { private static final String TRACE_DIRNAME = "/data/misc/wmtrace/"; private static final String TRACE_FILENAME_CLIENTS = "ime_trace_clients.pb"; + private static final String TRACE_FILENAME_IMS = "ime_trace_service.pb"; private static final int BUFFER_CAPACITY = 4096 * 1024; // Needed for winscope to auto-detect the dump type. Explained further in - // core.proto.android.view.inputmethod.inputmethodeditortrace.proto. This magic number - // corresponds to InputMethodClientsTraceFileProto. + // core.proto.android.view.inputmethod.inputmethodeditortrace.proto. + // This magic number corresponds to InputMethodClientsTraceFileProto. private static final long MAGIC_NUMBER_CLIENTS_VALUE = - ((long) MAGIC_NUMBER_H << 32) | MAGIC_NUMBER_L; + ((long) InputMethodClientsTraceFileProto.MAGIC_NUMBER_H << 32) + | InputMethodClientsTraceFileProto.MAGIC_NUMBER_L; + // This magic number corresponds to InputMethodServiceTraceFileProto. + private static final long MAGIC_NUMBER_IMS_VALUE = + ((long) InputMethodServiceTraceFileProto.MAGIC_NUMBER_H << 32) + | InputMethodServiceTraceFileProto.MAGIC_NUMBER_L; private final TraceBuffer mBufferClients; private final File mTraceFileClients; + private final TraceBuffer mBufferIms; + private final File mTraceFileIms; + private final Object mEnabledLock = new Object(); ImeTracingServerImpl() throws ServiceNotFoundException { mBufferClients = new TraceBuffer<>(BUFFER_CAPACITY); mTraceFileClients = new File(TRACE_DIRNAME + TRACE_FILENAME_CLIENTS); + mBufferIms = new TraceBuffer<>(BUFFER_CAPACITY); + mTraceFileIms = new File(TRACE_DIRNAME + TRACE_FILENAME_IMS); } /** - * The provided dump is added to the current dump buffer {@link ImeTracingServerImpl#mBuffer}. + * The provided dump is added to the corresponding dump buffer: + * {@link ImeTracingServerImpl#mBufferClients} or {@link ImeTracingServerImpl#mBufferIms}. * * @param proto dump to be added to the buffer */ @@ -70,7 +81,8 @@ class ImeTracingServerImpl extends ImeTracing { mBufferClients.add(proto); return; case IME_TRACING_FROM_IMS: - // TODO (b/154348613) + mBufferIms.add(proto); + return; case IME_TRACING_FROM_IMMS: // TODO (b/154348613) default: @@ -111,7 +123,7 @@ class ImeTracingServerImpl extends ImeTracing { } @Override - public void triggerServiceDump(String where, InputMethodService service) { + public void triggerServiceDump(String where, AbstractInputMethodService service) { // Intentionally left empty, this is implemented in ImeTracingClientImpl } @@ -122,9 +134,14 @@ class ImeTracingServerImpl extends ImeTracing { private void writeTracesToFilesLocked() { try { - ProtoOutputStream proto = new ProtoOutputStream(); - proto.write(MAGIC_NUMBER, MAGIC_NUMBER_CLIENTS_VALUE); - mBufferClients.writeTraceToFile(mTraceFileClients, proto); + ProtoOutputStream clientsProto = new ProtoOutputStream(); + clientsProto.write(InputMethodClientsTraceFileProto.MAGIC_NUMBER, + MAGIC_NUMBER_CLIENTS_VALUE); + mBufferClients.writeTraceToFile(mTraceFileClients, clientsProto); + + ProtoOutputStream imsProto = new ProtoOutputStream(); + imsProto.write(InputMethodServiceTraceFileProto.MAGIC_NUMBER, MAGIC_NUMBER_IMS_VALUE); + mBufferIms.writeTraceToFile(mTraceFileIms, imsProto); } catch (IOException e) { Log.e(TAG, "Unable to write buffer to file", e); } @@ -143,9 +160,11 @@ class ImeTracingServerImpl extends ImeTracing { return; } - pw.println("Starting tracing in " + TRACE_DIRNAME + ": " + TRACE_FILENAME_CLIENTS); + pw.println("Starting tracing in " + TRACE_DIRNAME + ": " + TRACE_FILENAME_CLIENTS + + ", " + TRACE_FILENAME_IMS); sEnabled = true; mBufferClients.resetBuffer(); + mBufferIms.resetBuffer(); } } @@ -163,10 +182,11 @@ class ImeTracingServerImpl extends ImeTracing { } pw.println("Stopping tracing and writing traces in " + TRACE_DIRNAME + ": " - + TRACE_FILENAME_CLIENTS); + + TRACE_FILENAME_CLIENTS + ", " + TRACE_FILENAME_IMS); sEnabled = false; writeTracesToFilesLocked(); mBufferClients.resetBuffer(); + mBufferIms.resetBuffer(); } } } diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java index ba6e673711878..c08e6a9cd8abb 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java @@ -141,6 +141,8 @@ import android.view.inputmethod.InputConnectionInspector.MissingMethodFlags; import android.view.inputmethod.InputMethod; import android.view.inputmethod.InputMethodEditorTraceProto.InputMethodClientsTraceFileProto; import android.view.inputmethod.InputMethodEditorTraceProto.InputMethodClientsTraceProto; +import android.view.inputmethod.InputMethodEditorTraceProto.InputMethodServiceTraceFileProto; +import android.view.inputmethod.InputMethodEditorTraceProto.InputMethodServiceTraceProto; import android.view.inputmethod.InputMethodInfo; import android.view.inputmethod.InputMethodManager; import android.view.inputmethod.InputMethodSubtype; @@ -4031,7 +4033,13 @@ public class InputMethodManagerService extends IInputMethodManager.Stub proto.end(client_token); break; case ImeTracing.IME_TRACING_FROM_IMS: - // TODO (b/154348613) + final long service_token = proto.start(InputMethodServiceTraceFileProto.ENTRY); + proto.write(InputMethodServiceTraceProto.ELAPSED_REALTIME_NANOS, + SystemClock.elapsedRealtimeNanos()); + proto.write(InputMethodServiceTraceProto.WHERE, where); + proto.write(InputMethodServiceTraceProto.INPUT_METHOD_SERVICE, protoDump); + proto.end(service_token); + break; default: // Dump triggered by a source not recognised. return;