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

Commit e17ce5fb authored by Tom O'Neill's avatar Tom O'Neill
Browse files

Handle races caused by rapid settings changed broadcasts

- Fix b/10447517

Change-Id: I63ef98c9023cee1a15be61b966aed06dc35e9bd5
parent 32e016b5
Loading
Loading
Loading
Loading
+49 −5
Original line number Diff line number Diff line
@@ -17,12 +17,17 @@
package com.android.settings.location;

import android.content.Intent;
import android.text.TextUtils;
import android.util.Log;
import com.android.internal.annotations.Immutable;
import com.android.internal.util.Preconditions;

/**
 * Specifies a setting that is being injected into Settings > Location > Location services.
 *
 * @see android.location.SettingInjectorService
 */
@Immutable
class InjectedSetting {

    /**
@@ -53,13 +58,30 @@ class InjectedSetting {
     */
    public final String settingsActivity;

    public InjectedSetting(String packageName, String className,
    private InjectedSetting(String packageName, String className,
            String title, int iconId, String settingsActivity) {
        this.packageName = packageName;
        this.className = className;
        this.title = title;
        this.packageName = Preconditions.checkNotNull(packageName, "packageName");
        this.className = Preconditions.checkNotNull(className, "className");
        this.title = Preconditions.checkNotNull(title, "title");
        this.iconId = iconId;
        this.settingsActivity = settingsActivity;
        this.settingsActivity = Preconditions.checkNotNull(settingsActivity);
    }

    /**
     * Returns a new instance, or null.
     */
    public static InjectedSetting newInstance(String packageName, String className,
            String title, int iconId, String settingsActivity) {
        if (packageName == null || className == null ||
                TextUtils.isEmpty(title) || TextUtils.isEmpty(settingsActivity)) {
            if (Log.isLoggable(SettingsInjector.TAG, Log.WARN)) {
                Log.w(SettingsInjector.TAG, "Illegal setting specification: package="
                        + packageName + ", class=" + className
                        + ", title=" + title + ", settingsActivity=" + settingsActivity);
            }
            return null;
        }
        return new InjectedSetting(packageName, className, title, iconId, settingsActivity);
    }

    @Override
@@ -81,4 +103,26 @@ class InjectedSetting {
        intent.setClassName(packageName, className);
        return intent;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (!(o instanceof InjectedSetting)) return false;

        InjectedSetting that = (InjectedSetting) o;

        return packageName.equals(that.packageName) && className.equals(that.className)
                && title.equals(that.title) && iconId == that.iconId
                && settingsActivity.equals(that.settingsActivity);
    }

    @Override
    public int hashCode() {
        int result = packageName.hashCode();
        result = 31 * result + className.hashCode();
        result = 31 * result + title.hashCode();
        result = 31 * result + iconId;
        result = 31 * result + settingsActivity.hashCode();
        return result;
    }
}
+7 −2
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ import android.preference.PreferenceGroup;
import android.preference.PreferenceManager;
import android.preference.PreferenceScreen;
import android.provider.Settings;
import android.util.Log;
import android.view.Gravity;
import android.widget.CompoundButton;
import android.widget.Switch;
@@ -47,6 +48,9 @@ import java.util.List;
 */
public class LocationSettings extends LocationSettingsBase
        implements CompoundButton.OnCheckedChangeListener {

    private static final String TAG = "LocationSettings";

    /** Key for preference screen "Mode" */
    private static final String KEY_LOCATION_MODE = "location_mode";
    /** Key for preference category "Recent location requests" */
@@ -146,8 +150,6 @@ public class LocationSettings extends LocationSettingsBase
                    }
                });

        final PreferenceManager preferenceManager = getPreferenceManager();

        PreferenceCategory categoryRecentLocationRequests =
                (PreferenceCategory) root.findPreference(KEY_RECENT_LOCATION_REQUESTS);
        RecentLocationApps recentApps = new RecentLocationApps(activity, mStatsHelper);
@@ -172,6 +174,9 @@ public class LocationSettings extends LocationSettingsBase
        mReceiver = new BroadcastReceiver() {
            @Override
            public void onReceive(Context context, Intent intent) {
                if (Log.isLoggable(TAG, Log.DEBUG)) {
                    Log.d(TAG, "Received settings change intent: " + intent);
                }
                injector.reloadStatusMessages();
            }
        };
+175 −106
Original line number Diff line number Diff line
@@ -31,18 +31,20 @@ import android.os.Handler;
import android.os.Message;
import android.os.Messenger;
import android.preference.Preference;
import android.text.TextUtils;
import android.util.AttributeSet;
import android.util.Log;
import android.util.Xml;
import com.android.settings.R;
import org.xmlpull.v1.XmlPullParser;
import org.xmlpull.v1.XmlPullParserException;

import com.android.settings.R;

import java.io.IOException;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;

/**
 * Adds the preferences specified by the {@link InjectedSetting} objects to a preference group.
@@ -59,7 +61,7 @@ import java.util.List;
 * {@link SettingInjectorService#UPDATE_INTENT}.
 */
class SettingsInjector {
    private static final String TAG = "SettingsInjector";
    static final String TAG = "SettingsInjector";

    private static final long INJECTED_STATUS_UPDATE_TIMEOUT_MILLIS = 1000;

@@ -79,11 +81,35 @@ class SettingsInjector {
     */
    public static final String ATTRIBUTES_NAME = "injected-location-setting";

    private Context mContext;
    private StatusLoader mLoader = null;
    /**
     * {@link Message#what} value for starting to load status values
     * in case we aren't already in the process of loading them.
     */
    private static final int WHAT_RELOAD = 1;

    /**
     * {@link Message#what} value sent after receiving a status message.
     */
    private static final int WHAT_RECEIVED_STATUS = 2;

    /**
     * {@link Message#what} value sent after the timeout waiting for a status message.
     */
    private static final int WHAT_TIMEOUT = 3;

    private final Context mContext;

    /**
     * The settings that were injected
     */
    private final Set<Setting> mSettings;

    private final Handler mHandler;

    public SettingsInjector(Context context) {
        mContext = context;
        mSettings = new HashSet<Setting>();
        mHandler = new StatusLoadingHandler();
    }

    /**
@@ -168,6 +194,9 @@ class SettingsInjector {
        }
    }

    /**
     * Returns an immutable representation of the static attributes for the setting, or null.
     */
    private static InjectedSetting parseAttributes(
            String packageName, String className, Resources res, AttributeSet attrs) {

@@ -175,86 +204,22 @@ class SettingsInjector {
        try {
            // Note that to help guard against malicious string injection, we do not allow dynamic
            // specification of the label (setting title)
            final int labelId = sa.getResourceId(
                    android.R.styleable.InjectedLocationSetting_label, 0);
            final String label = sa.getString(android.R.styleable.InjectedLocationSetting_label);
            final int iconId = sa.getResourceId(
                    android.R.styleable.InjectedLocationSetting_icon, 0);
            final String settingsActivity =
                    sa.getString(android.R.styleable.InjectedLocationSetting_settingsActivity);
            if (Log.isLoggable(TAG, Log.DEBUG)) {
                Log.d(TAG, "parsed labelId: " + labelId + ", label: " + label
                        + ", iconId: " + iconId);
                Log.d(TAG, "parsed label: " + label + ", iconId: " + iconId
                        + ", settingsActivity: " + settingsActivity);
            }
            if (labelId == 0 || TextUtils.isEmpty(label) || TextUtils.isEmpty(settingsActivity)) {
                return null;
            }
            return new InjectedSetting(packageName, className,
            return InjectedSetting.newInstance(packageName, className,
                    label, iconId, settingsActivity);
        } finally {
            sa.recycle();
        }
    }

    private final class StatusLoader {
        private final Intent mIntent;
        private final StatusLoader mPrev;

        private boolean mLoaded = false;

        /**
         * Creates a loader and chains with the previous loader.
         */
        public StatusLoader(Intent intent, StatusLoader prev) {
            mIntent = intent;
            mPrev = prev;
        }

        /**
         * Clears the loaded flag for the whole chain.
         */
        private void setNotLoaded() {
            mLoaded = false;
            if (mPrev != null) {
                mPrev.setNotLoaded();
            }
        }

        /**
         * Reloads the whole chain.
         */
        public void reload() {
            setNotLoaded();
            loadIfNotLoaded();
        }

        /**
         * If the current message hasn't been loaded, loads the status messages
         * and set time out for the next message.
         */
        public void loadIfNotLoaded() {
            if (mLoaded) {
                return;
            }

            mContext.startService(mIntent);
            if (mPrev != null) {
                Handler handler = new Handler() {
                    @Override
                    public void handleMessage(Message msg) {
                        // Continue with the next item in the chain.
                        mPrev.loadIfNotLoaded();
                    }
                };
                // Ensure that we start loading the previous setting in the chain if the current
                // setting hasn't loaded before the timeout
                handler.sendMessageDelayed(
                        Message.obtain(handler), INJECTED_STATUS_UPDATE_TIMEOUT_MILLIS);
            }
            mLoaded = true;
        }
    }

    /**
     * Gets a list of preferences that other apps have injected.
     *
@@ -264,17 +229,12 @@ class SettingsInjector {
    public List<Preference> getInjectedSettings() {
        Iterable<InjectedSetting> settings = getSettings();
        ArrayList<Preference> prefs = new ArrayList<Preference>();
        mLoader = null;
        for (InjectedSetting setting : settings) {
            Preference pref = addServiceSetting(prefs, setting);
            Intent intent = createUpdatingIntent(pref, setting, mLoader);
            mLoader = new StatusLoader(intent, mLoader);
            mSettings.add(new Setting(setting, pref));
        }

        // Start a thread to load each list item status.
        if (mLoader != null) {
            mLoader.loadIfNotLoaded();
        }
        reloadStatusMessages();

        return prefs;
    }
@@ -283,9 +243,10 @@ class SettingsInjector {
     * Reloads the status messages for all the preference items.
     */
    public void reloadStatusMessages() {
        if (mLoader != null) {
            mLoader.reload();
        if (Log.isLoggable(TAG, Log.DEBUG)) {
            Log.d(TAG, "reloadingStatusMessages: " + mSettings);
        }
        mHandler.sendMessage(mHandler.obtainMessage(WHAT_RELOAD));
    }

    /**
@@ -308,12 +269,119 @@ class SettingsInjector {
    }

    /**
     * Creates an Intent to ask the receiver for the current status for the setting, and display it
     * when it replies.
     * Loads the setting status values one at a time. Each load starts a subclass of {@link
     * SettingInjectorService}, so to reduce memory pressure we don't want to load too many at
     * once.
     */
    private final class StatusLoadingHandler extends Handler {

        /**
         * Settings whose status values need to be loaded. A set is used to prevent redundant loads
         * even if {@link #reloadStatusMessages()} is called many times in rapid succession (for
         * example, if we receive a lot of
         * {@link android.location.SettingInjectorService#UPDATE_INTENT} broadcasts).
         * <p/>
         * We use a linked hash set to ensure that when {@link #reloadStatusMessages()} is called,
         * any settings that haven't been loaded yet will finish loading before any already-loaded
         * messages are loaded again.
         */
        private LinkedHashSet<Setting> mSettingsToLoad = new LinkedHashSet<Setting>();

        /**
         * Whether we're in the middle of loading settings.
         */
        private boolean mLoading;

        @Override
        public void handleMessage(Message msg) {
            if (Log.isLoggable(TAG, Log.DEBUG)) {
                Log.d(TAG, "handleMessage start: " + msg + ", mSettingsToLoad: " + mSettingsToLoad);
            }

            switch (msg.what) {
                case WHAT_RELOAD:
                    mSettingsToLoad.addAll(mSettings);
                    if (mLoading) {
                        // Already waiting for a service to return its status, don't ask a new one
                        return;
                    }
                    mLoading = true;
                    break;
                case WHAT_TIMEOUT:
                    if (Log.isLoggable(TAG, Log.WARN)) {
                        final Setting setting = (Setting) msg.obj;
                        setting.timedOut = true;
                        Log.w(TAG, "Timed out trying to get status for: " + setting);
                    }
                    break;
                case WHAT_RECEIVED_STATUS:
                    final Setting setting = (Setting) msg.obj;
                    if (setting.timedOut) {
                        // We've already restarted retrieving the next setting, don't start another
                        return;
                    }

                    // Received the setting without timeout, clear any previous timed out status
                    setting.timedOut = false;
                    break;
                default:
                    throw new IllegalArgumentException("Unexpected what: " + msg);
            }

            // Remove the next setting to load from the queue, if any
            Iterator<Setting> iter = mSettingsToLoad.iterator();
            if (!iter.hasNext()) {
                mLoading = false;
                return;
            }
            Setting setting = iter.next();
            iter.remove();

            // Request the status value
            Intent intent = setting.createUpdatingIntent();
            mContext.startService(intent);

            // Ensure that if receiving the status value takes too long, we start loading the
            // next value anyway
            Message timeoutMsg = obtainMessage(WHAT_TIMEOUT, setting);
            removeMessages(WHAT_TIMEOUT);
            sendMessageDelayed(timeoutMsg, INJECTED_STATUS_UPDATE_TIMEOUT_MILLIS);

            if (Log.isLoggable(TAG, Log.DEBUG)) {
                Log.d(TAG, "handleMessage end: " + msg + ", mSettingsToLoad: " + mSettingsToLoad);
            }
        }
    }

    /**
     * Represents an injected setting and the corresponding preference.
     */
    private static Intent createUpdatingIntent(
            final Preference pref, final InjectedSetting info, final StatusLoader prev) {
        final Intent receiverIntent = info.getServiceIntent();
    private final class Setting {

        public final InjectedSetting setting;
        public final Preference preference;
        public boolean timedOut = false;

        private Setting(InjectedSetting setting, Preference preference) {
            this.setting = setting;
            this.preference = preference;
        }

        @Override
        public String toString() {
            return "Setting{" +
                    "setting=" + setting +
                    ", preference=" + preference +
                    ", timedOut=" + timedOut +
                    '}';
        }

        /**
         * Creates an Intent to ask the receiver for the current status for the setting, and display
         * it when it replies.
         */
        public Intent createUpdatingIntent() {
            final Intent receiverIntent = setting.getServiceIntent();
            Handler handler = new Handler() {
                @Override
                public void handleMessage(Message msg) {
@@ -321,20 +389,21 @@ class SettingsInjector {
                    String status = bundle.getString(SettingInjectorService.STATUS_KEY);
                    boolean enabled = bundle.getBoolean(SettingInjectorService.ENABLED_KEY, true);
                    if (Log.isLoggable(TAG, Log.DEBUG)) {
                    Log.d(TAG, info + ": received " + msg + ", bundle: " + bundle);
                }
                pref.setSummary(status);
                pref.setEnabled(enabled);
                if (prev != null) {
                    prev.loadIfNotLoaded();
                        Log.d(TAG, setting + ": received " + msg + ", bundle: " + bundle);
                    }
                    preference.setSummary(status);
                    preference.setEnabled(enabled);
                    mHandler.sendMessage(
                            mHandler.obtainMessage(WHAT_RECEIVED_STATUS, Setting.this));
                }
            };
            Messenger messenger = new Messenger(handler);
            receiverIntent.putExtra(SettingInjectorService.MESSENGER_KEY, messenger);
            if (Log.isLoggable(TAG, Log.DEBUG)) {
            Log.d(TAG, info + ": sending rcv-intent: " + receiverIntent + ", handler: " + handler);
                Log.d(TAG, setting + ": sending rcv-intent: " + receiverIntent
                        + ", handler: " + handler);
            }
            return receiverIntent;
        }
    }
}