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 108323 4d33341a7ec39e99734a5e111132363f9c2fa69b
parent 108322 19da37cf65ae77ac5254276d83030dfdd091ebee
child 108324 153fc8564818712361c996bfc36bf869cbc44cba
push id82
push usershu@rfrn.org
push dateFri, 05 Oct 2012 13:20:22 +0000
reviewersblassey
bugs747629
milestone18.0a1
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();