Bug 747629 - Part 1: Post (most) Gecko's IME callbacks from Gecko thread to UI thread. r=blassey
authorChris Peterson <cpeterson@mozilla.com>
Wed, 19 Sep 2012 11:08:39 -0700
changeset 108214 4d33341a7ec39e99734a5e111132363f9c2fa69b
parent 108213 19da37cf65ae77ac5254276d83030dfdd091ebee
child 108215 153fc8564818712361c996bfc36bf869cbc44cba
push id23545
push useremorley@mozilla.com
push dateThu, 27 Sep 2012 10:56:50 +0000
treeherdermozilla-central@aacf4867f830 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersblassey
bugs747629
milestone18.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 747629 - Part 1: Post (most) Gecko's IME callbacks from Gecko thread to UI thread. r=blassey
mobile/android/base/GeckoInputConnection.java
--- a/mobile/android/base/GeckoInputConnection.java
+++ b/mobile/android/base/GeckoInputConnection.java
@@ -498,26 +498,38 @@ class GeckoInputConnection
                     if (start < ca || start > cb || end < ca || end > cb) {
                         if (DEBUG) Log.d(LOGTAG, ". . . notifySelectionChange: removeComposingSpans");
                         removeComposingSpans(mEditable);
                     }
                 }
             }
         }
 
-        if (imm != null && imm.isFullscreenMode()) {
-            View v = getView();
-            if (hasCompositionString()) {
-                Span span = getComposingSpan();
-                imm.updateSelection(v, start, end, span.start, span.end);
-            } else {
-                imm.updateSelection(v, start, end, -1, -1);
+        // FIXME: Remove this postToUiThread() after bug 780543 is fixed.
+        final int oldStart = start;
+        final int oldEnd = end;
+        postToUiThread(new Runnable() {
+            public void run() {
+                InputMethodManager imm = getInputMethodManager();
+                if (imm != null && imm.isFullscreenMode()) {
+                    int newStart;
+                    int newEnd;
+                    if (hasCompositionString()) {
+                        Span span = getComposingSpan();
+                        newStart = span.start;
+                        newEnd = span.end;
+                    } else {
+                        newStart = -1;
+                        newEnd = -1;
+                    }
+                    View v = getView();
+                    imm.updateSelection(v, oldStart, oldEnd, newStart, newEnd);
+                }
             }
-
-        }
+        });
     }
 
     protected void resetCompositionState() {
         if (mBatchEditCount > 0) {
             Log.d(LOGTAG, "resetCompositionState: resetting mBatchEditCount "
                           + mBatchEditCount + " -> 0");
             mBatchEditCount = 0;
         }
@@ -1001,97 +1013,117 @@ class GeckoInputConnection
         return false;
     }
 
     public boolean isIMEEnabled() {
         // make sure this picks up PASSWORD and PLUGIN states as well
         return mIMEState != IME_STATE_DISABLED;
     }
 
-    public void notifyIME(int type, int state) {
-        View v = getView();
-        if (v == null)
-            return;
+    public void notifyIME(final int type, final int state) {
+        postToUiThread(new Runnable() {
+            public void run() {
+                View v = getView();
+                if (v == null)
+                    return;
 
-        switch (type) {
-        case NOTIFY_IME_RESETINPUTSTATE:
-            if (DEBUG) Log.d(LOGTAG, ". . . notifyIME: reset");
+                switch (type) {
+                    case NOTIFY_IME_RESETINPUTSTATE:
+                        if (DEBUG) Log.d(LOGTAG, ". . . notifyIME: reset");
 
-            // Gecko just cancelled the current composition from underneath us,
-            // so abandon our active composition string WITHOUT committing it!
-            resetCompositionState();
+                        // Gecko just cancelled the current composition from underneath us,
+                        // so abandon our active composition string WITHOUT committing it!
+                        resetCompositionState();
 
-            // Don't use IMEStateUpdater for reset.
-            // Because IME may not work showSoftInput()
-            // after calling restartInput() immediately.
-            // So we have to call showSoftInput() delay.
-            InputMethodManager imm = getInputMethodManager();
-            if (imm == null) {
-                // no way to reset IME status directly
-                IMEStateUpdater.resetIME();
-            } else {
-                imm.restartInput(v);
-            }
+                        // Don't use IMEStateUpdater for reset.
+                        // Because IME may not work showSoftInput()
+                        // after calling restartInput() immediately.
+                        // So we have to call showSoftInput() delay.
+                        InputMethodManager imm = getInputMethodManager();
+                        if (imm == null) {
+                            // no way to reset IME status directly
+                            IMEStateUpdater.resetIME();
+                        } else {
+                            imm.restartInput(v);
+                        }
 
-            // keep current enabled state
-            IMEStateUpdater.enableIME();
-            break;
+                        // keep current enabled state
+                        IMEStateUpdater.enableIME();
+                        break;
 
-        case NOTIFY_IME_CANCELCOMPOSITION:
-            if (DEBUG) Log.d(LOGTAG, ". . . notifyIME: cancel");
-            IMEStateUpdater.resetIME();
-            break;
+                    case NOTIFY_IME_CANCELCOMPOSITION:
+                        if (DEBUG) Log.d(LOGTAG, ". . . notifyIME: cancel");
+                        IMEStateUpdater.resetIME();
+                        break;
 
-        case NOTIFY_IME_FOCUSCHANGE:
-            if (DEBUG) Log.d(LOGTAG, ". . . notifyIME: focus");
-            IMEStateUpdater.resetIME();
-            break;
+                    case NOTIFY_IME_FOCUSCHANGE:
+                        if (DEBUG) Log.d(LOGTAG, ". . . notifyIME: focus");
+                        IMEStateUpdater.resetIME();
+                        break;
 
-        case NOTIFY_IME_SETOPENSTATE:
-        default:
-            if (DEBUG)
-                throw new IllegalArgumentException("Unexpected NOTIFY_IME=" + type);
-            break;
-        }
+                    case NOTIFY_IME_SETOPENSTATE:
+                    default:
+                        if (DEBUG)
+                            throw new IllegalArgumentException("Unexpected NOTIFY_IME=" + type);
+                        break;
+                }
+            }
+        });
     }
 
-    public void notifyIMEEnabled(int state, String typeHint, final String modeHint, String actionHint) {
+    public void notifyIMEEnabled(final int state, final String typeHint, final String modeHint, final String actionHint) {
         // For some input type we will use a  widget to display the ui, for those we must not
         // display the ime. We can display a widget for date and time types and, if the sdk version
         // is greater than 11, for datetime/month/week as well.
         if (typeHint.equals("date") || typeHint.equals("time") ||
             (Build.VERSION.SDK_INT > 10 &&
             (typeHint.equals("datetime") || typeHint.equals("month") ||
             typeHint.equals("week") || typeHint.equals("datetime-local")))) {
             return;
         }
 
-        View v = getView();
-
-        if (v == null)
-            return;
+        postToUiThread(new Runnable() {
+            public void run() {
+                View v = getView();
+                if (v == null)
+                    return;
 
-        /* When IME is 'disabled', IME processing is disabled.
-           In addition, the IME UI is hidden */
-        mIMEState = state;
-        mIMETypeHint = (typeHint == null) ? "" : typeHint;
-        mIMEModeHint = (modeHint == null) ? "" : modeHint;
-        mIMEActionHint = (actionHint == null) ? "" : actionHint;
-        IMEStateUpdater.enableIME();
+                /* When IME is 'disabled', IME processing is disabled.
+                   In addition, the IME UI is hidden */
+                mIMEState = state;
+                mIMETypeHint = (typeHint == null) ? "" : typeHint;
+                mIMEModeHint = (modeHint == null) ? "" : modeHint;
+                mIMEActionHint = (actionHint == null) ? "" : actionHint;
+                IMEStateUpdater.enableIME();
+            }
+        });
     }
 
-    public void notifyIMEChange(String text, int start, int end, int newEnd) {
-        InputMethodManager imm = getInputMethodManager();
-        if (imm == null)
-            return;
-
-        if (newEnd < 0)
-            notifySelectionChange(imm, start, end);
-        else
-            notifyTextChange(imm, text, start, end, newEnd);
+    public final void notifyIMEChange(final String text, final int start, final int end,
+                                      final int newEnd) {
+        if (newEnd < 0) {
+            // FIXME: Post notifySelectionChange() to UI thread after bug 780543 is fixed.
+            // notifyIMEChange() is called on the Gecko thread. We want to run all
+            // InputMethodManager code on the UI thread to avoid IME race conditions that cause
+            // crashes like bug 747629. However, if notifySelectionChange() is run on the UI thread,
+            // it causes mysterious problems with repeating characters like bug 780543. This
+            // band-aid fix is to run all InputMethodManager code on the UI thread except
+            // notifySelectionChange() until I can find the root cause.
+            InputMethodManager imm = getInputMethodManager();
+            if (imm != null)
+                notifySelectionChange(imm, start, end);
+        } else {
+            postToUiThread(new Runnable() {
+                public void run() {
+                    InputMethodManager imm = getInputMethodManager();
+                    if (imm != null)
+                        notifyTextChange(imm, text, start, end, newEnd);
+                }
+            });
+        }
     }
 
     /* Delay updating IME states (see bug 573800) */
     private static final class IMEStateUpdater extends TimerTask {
         private static IMEStateUpdater instance;
         private boolean mEnable;
         private boolean mReset;
 
@@ -1112,35 +1144,40 @@ class GeckoInputConnection
         }
 
         public void run() {
             if (DEBUG) Log.d(LOGTAG, "IME: run()");
             synchronized (IMEStateUpdater.class) {
                 instance = null;
             }
 
-            final View v = getView();
-            if (v == null)
+            // TimerTask.run() is running on a random background thread, so post to UI thread.
+            postToUiThread(new Runnable() {
+                public void run() {
+                    final View v = getView();
+                    if (v == null)
+                        return;
+
+                    final InputMethodManager imm = getInputMethodManager();
+                    if (imm == null)
                         return;
 
-            final InputMethodManager imm = getInputMethodManager();
-            if (imm == null)
-                return;
+                    if (mReset)
+                        imm.restartInput(v);
 
-            if (mReset)
-                imm.restartInput(v);
+                    if (!mEnable)
+                        return;
 
-            if (!mEnable)
-                return;
-
-            if (mIMEState != IME_STATE_DISABLED) {
-                imm.showSoftInput(v, 0);
-            } else if (imm.isActive(v)) {
-                imm.hideSoftInputFromWindow(v.getWindowToken(), 0);
-            }
+                    if (mIMEState != IME_STATE_DISABLED) {
+                        imm.showSoftInput(v, 0);
+                    } else if (imm.isActive(v)) {
+                        imm.hideSoftInputFromWindow(v.getWindowToken(), 0);
+                    }
+                }
+            });
         }
     }
 
     private void setEditable(String contents) {
         int prevLength = mEditable.length();
         mEditable.removeSpan(this);
         mEditable.replace(0, prevLength, contents);
         spanAndSelectEditable();