From 785bcfa858511649b0d08040e25c131b76d9bf72 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Mon, 17 Mar 2014 19:39:40 -0700 Subject: [PATCH] Bug 975838 - Part 1: add methods to log or throw if called on the wrong thread, pepper GeckoMenu with thread affinities. r=wesj --- mobile/android/base/GeckoEditable.java | 3 +- mobile/android/base/GeckoInputConnection.java | 17 +++--- mobile/android/base/menu/GeckoMenu.java | 21 ++++++- mobile/android/base/util/ThreadUtils.java | 55 ++++++++++++++----- 4 files changed, 72 insertions(+), 24 deletions(-) diff --git a/mobile/android/base/GeckoEditable.java b/mobile/android/base/GeckoEditable.java index 41962694aff..aff8aa08e83 100644 --- a/mobile/android/base/GeckoEditable.java +++ b/mobile/android/base/GeckoEditable.java @@ -8,6 +8,7 @@ package org.mozilla.gecko; import org.mozilla.gecko.gfx.InputConnectionHandler; import org.mozilla.gecko.gfx.LayerView; import org.mozilla.gecko.util.ThreadUtils; +import org.mozilla.gecko.util.ThreadUtils.AssertBehavior; import android.os.Build; import android.os.Handler; @@ -361,7 +362,7 @@ final class GeckoEditable } private void assertOnIcThread() { - ThreadUtils.assertOnThread(mIcRunHandler.getLooper().getThread()); + ThreadUtils.assertOnThread(mIcRunHandler.getLooper().getThread(), AssertBehavior.THROW); } private void geckoPostToIc(Runnable runnable) { diff --git a/mobile/android/base/GeckoInputConnection.java b/mobile/android/base/GeckoInputConnection.java index 706f456e954..aac8e3f8891 100644 --- a/mobile/android/base/GeckoInputConnection.java +++ b/mobile/android/base/GeckoInputConnection.java @@ -5,10 +5,16 @@ package org.mozilla.gecko; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.util.concurrent.SynchronousQueue; + import org.mozilla.gecko.gfx.InputConnectionHandler; import org.mozilla.gecko.util.Clipboard; import org.mozilla.gecko.util.GamepadUtils; import org.mozilla.gecko.util.ThreadUtils; +import org.mozilla.gecko.util.ThreadUtils.AssertBehavior; import android.R; import android.content.Context; @@ -33,11 +39,6 @@ import android.view.inputmethod.ExtractedTextRequest; import android.view.inputmethod.InputConnection; import android.view.inputmethod.InputMethodManager; -import java.lang.reflect.InvocationHandler; -import java.lang.reflect.Method; -import java.lang.reflect.Proxy; -import java.util.concurrent.SynchronousQueue; - class GeckoInputConnection extends BaseInputConnection implements InputConnectionHandler, GeckoEditableListener { @@ -115,7 +116,7 @@ class GeckoInputConnection public void waitForUiThread(Handler icHandler) { if (DEBUG) { - ThreadUtils.assertOnThread(icHandler.getLooper().getThread()); + ThreadUtils.assertOnThread(icHandler.getLooper().getThread(), AssertBehavior.THROW); Log.d(LOGTAG, "waitForUiThread() blocking on thread " + icHandler.getLooper().getThread().getName()); } @@ -154,7 +155,7 @@ class GeckoInputConnection public Editable getEditableForUiThread(final Handler uiHandler, final GeckoEditableClient client) { if (DEBUG) { - ThreadUtils.assertOnThread(uiHandler.getLooper().getThread()); + ThreadUtils.assertOnThread(uiHandler.getLooper().getThread(), AssertBehavior.THROW); } final Handler icHandler = client.getInputConnectionHandler(); if (icHandler.getLooper() == uiHandler.getLooper()) { @@ -171,7 +172,7 @@ class GeckoInputConnection final Method method, final Object[] args) throws Throwable { if (DEBUG) { - ThreadUtils.assertOnThread(uiHandler.getLooper().getThread()); + ThreadUtils.assertOnThread(uiHandler.getLooper().getThread(), AssertBehavior.THROW); Log.d(LOGTAG, "UiEditable." + method.getName() + "() blocking"); } synchronized (icHandler) { diff --git a/mobile/android/base/menu/GeckoMenu.java b/mobile/android/base/menu/GeckoMenu.java index ba69b68f1d1..e2aabc816dc 100644 --- a/mobile/android/base/menu/GeckoMenu.java +++ b/mobile/android/base/menu/GeckoMenu.java @@ -4,7 +4,10 @@ package org.mozilla.gecko.menu; +import org.mozilla.gecko.AppConstants; import org.mozilla.gecko.R; +import org.mozilla.gecko.util.ThreadUtils; +import org.mozilla.gecko.util.ThreadUtils.AssertBehavior; import android.content.ComponentName; import android.content.Context; @@ -35,6 +38,12 @@ public class GeckoMenu extends ListView GeckoMenuItem.OnShowAsActionChangedListener { private static final String LOGTAG = "GeckoMenu"; + /** + * Controls whether off-UI-thread method calls in this class cause an + * exception or just logging. + */ + private static final AssertBehavior THREAD_ASSERT_BEHAVIOR = AppConstants.RELEASE_BUILD ? AssertBehavior.NONE : AssertBehavior.THROW; + /* * A callback for a menu item selected event. */ @@ -52,7 +61,7 @@ public class GeckoMenu extends ListView // Open the menu. public void openMenu(); - // Show the actual view contaning the menu items. This can either be a parent or sub-menu. + // Show the actual view containing the menu items. This can either be a parent or sub-menu. public void showMenu(View menu); // Close the menu. @@ -130,6 +139,10 @@ public class GeckoMenu extends ListView mSecondaryActionItemBar = (DefaultActionItemBar) LayoutInflater.from(context).inflate(R.layout.menu_secondary_action_bar, null); } + private static void assertOnUiThread() { + ThreadUtils.assertOnUiThread(THREAD_ASSERT_BEHAVIOR); + } + @Override public MenuItem add(CharSequence title) { GeckoMenuItem menuItem = new GeckoMenuItem(this, NO_ID, 0, title); @@ -159,12 +172,14 @@ public class GeckoMenu extends ListView } private void addItem(GeckoMenuItem menuItem) { + assertOnUiThread(); menuItem.setOnShowAsActionChangedListener(this); mAdapter.addMenuItem(menuItem); mItems.add(menuItem); } private boolean addActionItem(final GeckoMenuItem menuItem) { + assertOnUiThread(); menuItem.setOnShowAsActionChangedListener(this); final View actionView = menuItem.getActionView(); @@ -272,6 +287,7 @@ public class GeckoMenu extends ListView @Override public void clear() { + assertOnUiThread(); for (GeckoMenuItem menuItem : mItems) { if (menuItem.hasSubMenu()) { SubMenu sub = menuItem.getSubMenu(); @@ -355,6 +371,7 @@ public class GeckoMenu extends ListView @Override public boolean hasVisibleItems() { + assertOnUiThread(); for (GeckoMenuItem menuItem : mItems) { if (menuItem.isVisible() && !mPrimaryActionItems.containsKey(menuItem) && @@ -386,6 +403,7 @@ public class GeckoMenu extends ListView @Override public void removeItem(int id) { + assertOnUiThread(); GeckoMenuItem item = (GeckoMenuItem) findItem(id); if (item == null) return; @@ -473,6 +491,7 @@ public class GeckoMenu extends ListView } public void onItemChanged(GeckoMenuItem item) { + assertOnUiThread(); if (item.isActionItem()) { final View actionView; if (item.getActionEnum() == GeckoMenuItem.SHOW_AS_ACTION_ALWAYS) { diff --git a/mobile/android/base/util/ThreadUtils.java b/mobile/android/base/util/ThreadUtils.java index a646f1ae4e9..48c8657d0c6 100644 --- a/mobile/android/base/util/ThreadUtils.java +++ b/mobile/android/base/util/ThreadUtils.java @@ -5,15 +5,24 @@ package org.mozilla.gecko.util; +import java.util.Map; + import android.os.Handler; import android.os.MessageQueue; import android.util.Log; -import java.util.Map; - public final class ThreadUtils { private static final String LOGTAG = "ThreadUtils"; + /** + * Controls the action taken when a method like + * {@link ThreadUtils#assertOnUiThread(AssertBehavior)} detects a problem. + */ + public static enum AssertBehavior { + NONE, + THROW, + } + private static Thread sUiThread; private static Thread sBackgroundThread; @@ -101,28 +110,46 @@ public final class ThreadUtils { GeckoBackgroundThread.post(runnable); } + public static void assertOnUiThread(final AssertBehavior assertBehavior) { + assertOnThread(getUiThread(), assertBehavior); + } + public static void assertOnUiThread() { - assertOnThread(getUiThread()); + assertOnThread(getUiThread(), AssertBehavior.THROW); } public static void assertOnGeckoThread() { - assertOnThread(sGeckoThread); + assertOnThread(sGeckoThread, AssertBehavior.THROW); } public static void assertOnBackgroundThread() { - assertOnThread(getBackgroundThread()); + assertOnThread(getBackgroundThread(), AssertBehavior.THROW); } - public static void assertOnThread(Thread expectedThread) { - Thread currentThread = Thread.currentThread(); - long currentThreadId = currentThread.getId(); - long expectedThreadId = expectedThread.getId(); + public static void assertOnThread(final Thread expectedThread) { + assertOnThread(expectedThread, AssertBehavior.THROW); + } - if (currentThreadId != expectedThreadId) { - throw new IllegalThreadStateException("Expected thread " + expectedThreadId + " (\"" - + expectedThread.getName() - + "\"), but running on thread " + currentThreadId - + " (\"" + currentThread.getName() + ")"); + public static void assertOnThread(final Thread expectedThread, AssertBehavior behavior) { + final Thread currentThread = Thread.currentThread(); + final long currentThreadId = currentThread.getId(); + final long expectedThreadId = expectedThread.getId(); + + if (currentThreadId == expectedThreadId) { + return; + } + + final String message = "Expected thread " + + expectedThreadId + " (\"" + expectedThread.getName() + + "\"), but running on thread " + + currentThreadId + " (\"" + currentThread.getName() + ")"; + final IllegalThreadStateException e = new IllegalThreadStateException(message); + + switch (behavior) { + case THROW: + throw e; + default: + Log.e(LOGTAG, "Method called on wrong thread!", e); } }