SharedPreferences$Editor.startCommit()

Adds a fire-and-forget save method (startCommit) to the
SharedPreferences.Editor, which is the way most people use it anyway.

This commit adds the implementation.  The previous commit added the
interface and docs:

   previous change: Idf9934b445da1fb72b79f0192218b47c0a7f5a34
        git commit: edf32d01316bd3432c023f17747461b08ae36375

In addition, this change:

-- adds a generic "runPendingWorkFinishers" mechanism to
   ActivityThread to wait on async operations that are still
   in flight and use it for this.

-- ties runPendingWorkFinishers into Activity.onPause,
   BroadcastReceiver, and Service.

-- makes sSharedPreferences keyed on name, not File, to avoid
   unnnecessary allocations

-- documents and guarantees what thread
   OnSharedPreferenceChangeListener callbacks run on

-- makes a few things in frameworks/base use startCommit(), notably
   Preference.java (which was ignoring the return value anyway)

Change-Id: I1c8db60ad45643226fe6d246d3e513eeb7bd0ebd

8 files changed, 326 insertions, 81 deletions

diff --git a/core/java/android/app/Activity.java b/core/java/android/app/Activity.java
index a3a8f09..773ff7c 100644
--- a/core/java/android/app/Activity.java
+++ b/core/java/android/app/Activity.java
@@ -1162,6 +1162,7 @@ public class Activity extends ContextThemeWrapper
      */
     protected void onPause() {
         mCalled = true;
+        QueuedWork.waitToFinish();
     }
 
     /**
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index c07e3d3..084f637 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -152,7 +152,7 @@ public final class ActivityThread {
             = new ArrayList<Application>();
     // set of instantiated backup agents, keyed by package name
     final HashMap<String, BackupAgent> mBackupAgents = new HashMap<String, BackupAgent>();
-    static final ThreadLocal sThreadLocal = new ThreadLocal();
+    static final ThreadLocal<ActivityThread> sThreadLocal = new ThreadLocal();
     Instrumentation mInstrumentation;
     String mInstrumentationAppDir = null;
     String mInstrumentationAppPackage = null;
@@ -186,6 +186,8 @@ public final class ActivityThread {
     final GcIdler mGcIdler = new GcIdler();
     boolean mGcIdlerScheduled = false;
 
+    static Handler sMainThreadHandler;  // set once in main()
+
     private static final class ActivityClientRecord {
         IBinder token;
         int ident;
@@ -1111,7 +1113,7 @@ public final class ActivityThread {
     }
 
     public static final ActivityThread currentActivityThread() {
-        return (ActivityThread)sThreadLocal.get();
+        return sThreadLocal.get();
     }
 
     public static final String currentPackageName() {
@@ -1780,6 +1782,8 @@ public final class ActivityThread {
             }
         }
 
+        QueuedWork.waitToFinish();
+
         try {
             if (data.sync) {
                 if (DEBUG_BROADCAST) Slog.i(TAG,
@@ -2007,6 +2011,9 @@ public final class ActivityThread {
                     data.args.setExtrasClassLoader(s.getClassLoader());
                 }
                 int res = s.onStartCommand(data.args, data.flags, data.startId);
+
+                QueuedWork.waitToFinish();
+
                 try {
                     ActivityManagerNative.getDefault().serviceDoneExecuting(
                             data.token, 1, data.startId, res);
@@ -2035,6 +2042,9 @@ public final class ActivityThread {
                     final String who = s.getClassName();
                     ((ContextImpl) context).scheduleFinalCleanup(who, "Service");
                 }
+
+                QueuedWork.waitToFinish();
+
                 try {
                     ActivityManagerNative.getDefault().serviceDoneExecuting(
                             token, 0, 0, 0);
@@ -3598,6 +3608,9 @@ public final class ActivityThread {
         Process.setArgV0("<pre-initialized>");
 
         Looper.prepareMainLooper();
+        if (sMainThreadHandler == null) {
+            sMainThreadHandler = new Handler();
+        }
 
         ActivityThread thread = new ActivityThread();
         thread.attach(false);
diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java
index 7b35e7f..0cd1f3a 100644
--- a/core/java/android/app/ContextImpl.java
+++ b/core/java/android/app/ContextImpl.java
@@ -119,9 +119,12 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.WeakHashMap;
-import java.util.Map.Entry;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 class ReceiverRestrictedContext extends ContextWrapper {
     ReceiverRestrictedContext(Context base) {
@@ -170,8 +173,8 @@ class ContextImpl extends Context {
     private static ThrottleManager sThrottleManager;
     private static WifiManager sWifiManager;
     private static LocationManager sLocationManager;
-    private static final HashMap<File, SharedPreferencesImpl> sSharedPrefs =
-            new HashMap<File, SharedPreferencesImpl>();
+    private static final HashMap<String, SharedPreferencesImpl> sSharedPrefs =
+            new HashMap<String, SharedPreferencesImpl>();
 
     private AudioManager mAudioManager;
     /*package*/ LoadedApk mPackageInfo;
@@ -210,7 +213,7 @@ class ContextImpl extends Context {
     private File mCacheDir;
     private File mExternalFilesDir;
     private File mExternalCacheDir;
-    
+
     private static long sInstanceCount = 0;
 
     private static final String[] EMPTY_FILE_LIST = {};
@@ -335,15 +338,15 @@ class ContextImpl extends Context {
     @Override
     public SharedPreferences getSharedPreferences(String name, int mode) {
         SharedPreferencesImpl sp;
-        File f = getSharedPrefsFile(name);
         synchronized (sSharedPrefs) {
-            sp = sSharedPrefs.get(f);
+            sp = sSharedPrefs.get(name);
             if (sp != null && !sp.hasFileChanged()) {
                 //Log.i(TAG, "Returning existing prefs " + name + ": " + sp);
                 return sp;
             }
         }
-        
+        File f = getSharedPrefsFile(name);
+
         FileInputStream str = null;
         File backup = makeBackupFile(f);
         if (backup.exists()) {
@@ -355,7 +358,7 @@ class ContextImpl extends Context {
         if (f.exists() && !f.canRead()) {
             Log.w(TAG, "Attempt to read preferences file " + f + " without permission");
         }
-        
+
         Map map = null;
         if (f.exists() && f.canRead()) {
             try {
@@ -376,10 +379,10 @@ class ContextImpl extends Context {
                 //Log.i(TAG, "Updating existing prefs " + name + " " + sp + ": " + map);
                 sp.replace(map);
             } else {
-                sp = sSharedPrefs.get(f);
+                sp = sSharedPrefs.get(name);
                 if (sp == null) {
                     sp = new SharedPreferencesImpl(f, mode, map);
-                    sSharedPrefs.put(f, sp);
+                    sSharedPrefs.put(name, sp);
                 }
             }
             return sp;
@@ -2698,10 +2701,12 @@ class ContextImpl extends Context {
         private final File mFile;
         private final File mBackupFile;
         private final int mMode;
-        private Map mMap;
-        private final FileStatus mFileStatus = new FileStatus();
-        private long mTimestamp;
 
+        private Map<String, Object> mMap;  // guarded by 'this'
+        private long mTimestamp;  // guarded by 'this'
+        private int mDiskWritesInFlight = 0;  // guarded by 'this'
+
+        private final Object mWritingToDiskLock = new Object();
         private static final Object mContent = new Object();
         private WeakHashMap<OnSharedPreferenceChangeListener, Object> mListeners;
 
@@ -2710,22 +2715,24 @@ class ContextImpl extends Context {
             mFile = file;
             mBackupFile = makeBackupFile(file);
             mMode = mode;
-            mMap = initialContents != null ? initialContents : new HashMap();
-            if (FileUtils.getFileStatus(file.getPath(), mFileStatus)) {
-                mTimestamp = mFileStatus.mtime;
+            mMap = initialContents != null ? initialContents : new HashMap<String, Object>();
+            FileStatus stat = new FileStatus();
+            if (FileUtils.getFileStatus(file.getPath(), stat)) {
+                mTimestamp = stat.mtime;
             }
             mListeners = new WeakHashMap<OnSharedPreferenceChangeListener, Object>();
         }
 
         public boolean hasFileChanged() {
+            FileStatus stat = new FileStatus();
+            if (!FileUtils.getFileStatus(mFile.getPath(), stat)) {
+                return true;
+            }
             synchronized (this) {
-                if (!FileUtils.getFileStatus(mFile.getPath(), mFileStatus)) {
-                    return true;
-                }
-                return mTimestamp != mFileStatus.mtime;
+                return mTimestamp != stat.mtime;
             }
         }
-        
+
         public void replace(Map newContents) {
             if (newContents != null) {
                 synchronized (this) {
@@ -2733,7 +2740,7 @@ class ContextImpl extends Context {
                 }
             }
         }
-        
+
         public void registerOnSharedPreferenceChangeListener(OnSharedPreferenceChangeListener listener) {
             synchronized(this) {
                 mListeners.put(listener, mContent);
@@ -2749,7 +2756,7 @@ class ContextImpl extends Context {
         public Map<String, ?> getAll() {
             synchronized(this) {
                 //noinspection unchecked
-                return new HashMap(mMap);
+                return new HashMap<String, Object>(mMap);
             }
         }
 
@@ -2768,7 +2775,7 @@ class ContextImpl extends Context {
         }
         public long getLong(String key, long defValue) {
             synchronized (this) {
-                Long v = (Long) mMap.get(key);
+                Long v = (Long)mMap.get(key);
                 return v != null ? v : defValue;
             }
         }
@@ -2791,10 +2798,31 @@ class ContextImpl extends Context {
             }
         }
 
+        public Editor edit() {
+            return new EditorImpl();
+        }
+
+        // Return value from EditorImpl#commitToMemory()
+        private static class MemoryCommitResult {
+            public boolean changesMade;  // any keys different?
+            public List<String> keysModified;  // may be null
+            public Set<OnSharedPreferenceChangeListener> listeners;  // may be null
+            public Map<?, ?> mapToWriteToDisk;
+            public final CountDownLatch writtenToDiskLatch = new CountDownLatch(1);
+            public volatile boolean writeToDiskResult = false;
+
+            public void setDiskWriteResult(boolean result) {
+                writeToDiskResult = result;
+                writtenToDiskLatch.countDown();
+            }
+        }
+
         public final class EditorImpl implements Editor {
             private final Map<String, Object> mModified = Maps.newHashMap();
             private boolean mClear = false;
 
+            private AtomicBoolean mCommitInFlight = new AtomicBoolean(false);
+
             public Editor putString(String key, String value) {
                 synchronized (this) {
                     mModified.put(key, value);
@@ -2841,30 +2869,67 @@ class ContextImpl extends Context {
             }
 
             public void startCommit() {
-                // TODO: implement
-                commit();
-            }
+                if (!mCommitInFlight.compareAndSet(false, true)) {
+                    throw new IllegalStateException("can't call startCommit() twice");
+                }
 
-            public boolean commit() {
-                boolean returnValue;
+                final MemoryCommitResult mcr = commitToMemory();
+                final Runnable awaitCommit = new Runnable() {
+                        public void run() {
+                            try {
+                                mcr.writtenToDiskLatch.await();
+                            } catch (InterruptedException ignored) {
+                            }
+                        }
+                    };
+
+                QueuedWork.add(awaitCommit);
+
+                Runnable postWriteRunnable = new Runnable() {
+                        public void run() {
+                            awaitCommit.run();
+                            mCommitInFlight.set(false);
+                            QueuedWork.remove(awaitCommit);
+                        }
+                    };
 
-                boolean hasListeners;
-                boolean changesMade = false;
-                List<String> keysModified = null;
-                Set<OnSharedPreferenceChangeListener> listeners = null;
+                SharedPreferencesImpl.this.enqueueDiskWrite(mcr, postWriteRunnable);
 
+                // Okay to notify the listeners before it's hit disk
+                // because the listeners should always get the same
+                // SharedPreferences instance back, which has the
+                // changes reflected in memory.
+                notifyListeners(mcr);
+            }
+
+            // Returns true if any changes were made
+            private MemoryCommitResult commitToMemory() {
+                MemoryCommitResult mcr = new MemoryCommitResult();
                 synchronized (SharedPreferencesImpl.this) {
-                    hasListeners = mListeners.size() > 0;
+                    // We optimistically don't make a deep copy until
+                    // a memory commit comes in when we're already
+                    // writing to disk.
+                    if (mDiskWritesInFlight > 0) {
+                        // We can't modify our mMap as a currently
+                        // in-flight write owns it.  Clone it before
+                        // modifying it.
+                        // noinspection unchecked
+                        mMap = new HashMap<String, Object>(mMap);
+                    }
+                    mcr.mapToWriteToDisk = mMap;
+                    mDiskWritesInFlight++;
+
+                    boolean hasListeners = mListeners.size() > 0;
                     if (hasListeners) {
-                        keysModified = new ArrayList<String>();
-                        listeners =
-                                new HashSet<OnSharedPreferenceChangeListener>(mListeners.keySet());
+                        mcr.keysModified = new ArrayList<String>();
+                        mcr.listeners =
+                            new HashSet<OnSharedPreferenceChangeListener>(mListeners.keySet());
                     }
 
                     synchronized (this) {
                         if (mClear) {
                             if (!mMap.isEmpty()) {
-                                changesMade = true;
+                                mcr.changesMade = true;
                                 mMap.clear();
                             }
                             mClear = false;
@@ -2874,53 +2939,122 @@ class ContextImpl extends Context {
                             String k = e.getKey();
                             Object v = e.getValue();
                             if (v == this) {  // magic value for a removal mutation
-                                if (mMap.containsKey(k)) {
-                                    mMap.remove(k);
-                                    changesMade = true;
+                                if (!mMap.containsKey(k)) {
+                                    continue;
                                 }
+                                mMap.remove(k);
                             } else {
                                 boolean isSame = false;
                                 if (mMap.containsKey(k)) {
                                     Object existingValue = mMap.get(k);
-                                    isSame = existingValue != null && existingValue.equals(v);
-                                }
-                                if (!isSame) {
-                                    mMap.put(k, v);
-                                    changesMade = true;
+                                    if (existingValue != null && existingValue.equals(v)) {
+                                        continue;
+                                    }
                                 }
+                                mMap.put(k, v);
                             }
 
+                            mcr.changesMade = true;
                             if (hasListeners) {
-                                keysModified.add(k);
+                                mcr.keysModified.add(k);
                             }
                         }
 
                         mModified.clear();
                     }
+                }
+                return mcr;
+            }
 
-                    returnValue = writeFileLocked(changesMade);
+            public boolean commit() {
+                MemoryCommitResult mcr = commitToMemory();
+                SharedPreferencesImpl.this.enqueueDiskWrite(
+                    mcr, null /* sync write on this thread okay */);
+                try {
+                    mcr.writtenToDiskLatch.await();
+                } catch (InterruptedException e) {
+                    return false;
                 }
+                notifyListeners(mcr);
+                return mcr.writeToDiskResult;
+            }
 
-                if (hasListeners) {
-                    for (int i = keysModified.size() - 1; i >= 0; i--) {
-                        final String key = keysModified.get(i);
-                        for (OnSharedPreferenceChangeListener listener : listeners) {
+            private void notifyListeners(final MemoryCommitResult mcr) {
+                if (mcr.listeners == null || mcr.keysModified == null ||
+                    mcr.keysModified.size() == 0) {
+                    return;
+                }
+                if (Looper.myLooper() == Looper.getMainLooper()) {
+                    for (int i = mcr.keysModified.size() - 1; i >= 0; i--) {
+                        final String key = mcr.keysModified.get(i);
+                        for (OnSharedPreferenceChangeListener listener : mcr.listeners) {
                             if (listener != null) {
                                 listener.onSharedPreferenceChanged(SharedPreferencesImpl.this, key);
                             }
                         }
                     }
+                } else {
+                    // Run this function on the main thread.
+                    ActivityThread.sMainThreadHandler.post(new Runnable() {
+                            public void run() {
+                                notifyListeners(mcr);
+                            }
+                        });
                 }
-
-                return returnValue;
             }
         }
 
-        public Editor edit() {
-            return new EditorImpl();
+        /**
+         * Enqueue an already-committed-to-memory result to be written
+         * to disk.
+         *
+         * They will be written to disk one-at-a-time in the order
+         * that they're enqueued.
+         *
+         * @param postWriteRunnable if non-null, we're being called
+         *   from startCommit() and this is the runnable to run after
+         *   the write proceeds.  if null (from a regular commit()),
+         *   then we're allowed to do this disk write on the main
+         *   thread (which in addition to reducing allocations and
+         *   creating a background thread, this has the advantage that
+         *   we catch them in userdebug StrictMode reports to convert
+         *   them where possible to startCommit...)
+         */
+        private void enqueueDiskWrite(final MemoryCommitResult mcr,
+                                      final Runnable postWriteRunnable) {
+            final Runnable writeToDiskRunnable = new Runnable() {
+                    public void run() {
+                        synchronized (mWritingToDiskLock) {
+                            writeToFile(mcr);
+                        }
+                        synchronized (SharedPreferencesImpl.this) {
+                            mDiskWritesInFlight--;
+                        }
+                        if (postWriteRunnable != null) {
+                            postWriteRunnable.run();
+                        }
+                    }
+                };
+
+            final boolean isFromSyncCommit = (postWriteRunnable == null);
+
+            // Typical #commit() path with fewer allocations, doing a write on
+            // the current thread.
+            if (isFromSyncCommit) {
+                boolean wasEmpty = false;
+                synchronized (SharedPreferencesImpl.this) {
+                    wasEmpty = mDiskWritesInFlight == 1;
+                }
+                if (wasEmpty) {
+                    writeToDiskRunnable.run();
+                    return;
+                }
+            }
+
+            QueuedWork.singleThreadExecutor().execute(writeToDiskRunnable);
         }
 
-        private FileOutputStream createFileOutputStream(File file) {
+        private static FileOutputStream createFileOutputStream(File file) {
             FileOutputStream str = null;
             try {
                 str = new FileOutputStream(file);
@@ -2943,49 +3077,56 @@ class ContextImpl extends Context {
             return str;
         }
 
-        private boolean writeFileLocked(boolean changesMade) {
+        // Note: must hold mWritingToDiskLock
+        private void writeToFile(MemoryCommitResult mcr) {
             // Rename the current file so it may be used as a backup during the next read
             if (mFile.exists()) {
-                if (!changesMade) {
+                if (!mcr.changesMade) {
                     // If the file already exists, but no changes were
                     // made to the underlying map, it's wasteful to
                     // re-write the file.  Return as if we wrote it
                     // out.
-                    return true;
+                    mcr.setDiskWriteResult(true);
+                    return;
                 }
                 if (!mBackupFile.exists()) {
                     if (!mFile.renameTo(mBackupFile)) {
                         Log.e(TAG, "Couldn't rename file " + mFile
                                 + " to backup file " + mBackupFile);
-                        return false;
+                        mcr.setDiskWriteResult(false);
+                        return;
                     }
                 } else {
                     mFile.delete();
                 }
             }
-            
+
             // Attempt to write the file, delete the backup and return true as atomically as
             // possible.  If any exception occurs, delete the new file; next time we will restore
             // from the backup.
             try {
                 FileOutputStream str = createFileOutputStream(mFile);
                 if (str == null) {
-                    return false;
+                    mcr.setDiskWriteResult(false);
+                    return;
                 }
-                XmlUtils.writeMapXml(mMap, str);
+                XmlUtils.writeMapXml(mcr.mapToWriteToDisk, str);
                 str.close();
                 setFilePermissionsFromMode(mFile.getPath(), mMode, 0);
-                if (FileUtils.getFileStatus(mFile.getPath(), mFileStatus)) {
-                    mTimestamp = mFileStatus.mtime;
+                FileStatus stat = new FileStatus();
+                if (FileUtils.getFileStatus(mFile.getPath(), stat)) {
+                    synchronized (this) {
+                        mTimestamp = stat.mtime;
+                    }
                 }
-                
                 // Writing was successful, delete the backup file if there is one.
                 mBackupFile.delete();
-                return true;
+                mcr.setDiskWriteResult(true);
+                return;
             } catch (XmlPullParserException e) {
-                Log.w(TAG, "writeFileLocked: Got exception:", e);
+                Log.w(TAG, "writeToFile: Got exception:", e);
             } catch (IOException e) {
-                Log.w(TAG, "writeFileLocked: Got exception:", e);
+                Log.w(TAG, "writeToFile: Got exception:", e);
             }
             // Clean up an unsuccessfully written file
             if (mFile.exists()) {
@@ -2993,7 +3134,7 @@ class ContextImpl extends Context {
                     Log.e(TAG, "Couldn't clean up partially-written file " + mFile);
                 }
             }
-            return false;
+            mcr.setDiskWriteResult(false);
         }
     }
 }
diff --git a/core/java/android/app/QueuedWork.java b/core/java/android/app/QueuedWork.java
new file mode 100644
index 0000000..af6bb1b
--- /dev/null
+++ b/core/java/android/app/QueuedWork.java
@@ -0,0 +1,91 @@
+/*
+ * Copyright (C) 2010 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.app;
+
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+/**
+ * Internal utility class to keep track of process-global work that's
+ * outstanding and hasn't been finished yet.
+ *
+ * This was created for writing SharedPreference edits out
+ * asynchronously so we'd have a mechanism to wait for the writes in
+ * Activity.onPause and similar places, but we may use this mechanism
+ * for other things in the future.
+ *
+ * @hide
+ */
+public class QueuedWork {
+
+    // The set of Runnables that will finish or wait on any async
+    // activities started by the application.
+    private static final ConcurrentLinkedQueue<Runnable> sPendingWorkFinishers =
+            new ConcurrentLinkedQueue<Runnable>();
+
+    private static ExecutorService sSingleThreadExecutor = null; // lazy, guarded by class
+
+    /**
+     * Returns a single-thread Executor shared by the entire process,
+     * creating it if necessary.
+     */
+    public static ExecutorService singleThreadExecutor() {
+        synchronized (QueuedWork.class) {
+            if (sSingleThreadExecutor == null) {
+                // TODO: can we give this single thread a thread name?
+                sSingleThreadExecutor = Executors.newSingleThreadExecutor();
+            }
+            return sSingleThreadExecutor;
+        }
+    }
+
+    /**
+     * Add a runnable to finish (or wait for) a deferred operation
+     * started in this context earlier.  Typically finished by e.g.
+     * an Activity#onPause.  Used by SharedPreferences$Editor#startCommit().
+     *
+     * Note that this doesn't actually start it running.  This is just
+     * a scratch set for callers doing async work to keep updated with
+     * what's in-flight.  In the common case, caller code
+     * (e.g. SharedPreferences) will pretty quickly call remove()
+     * after an add().  The only time these Runnables are run is from
+     * waitToFinish(), below.
+     */
+    public static void add(Runnable finisher) {
+        sPendingWorkFinishers.add(finisher);
+    }
+
+    public static void remove(Runnable finisher) {
+        sPendingWorkFinishers.remove(finisher);
+    }
+
+    /**
+     * Finishes or waits for async operations to complete.
+     * (e.g. SharedPreferences$Editor#startCommit writes)
+     *
+     * Is called from the Activity base class's onPause(), after
+     * BroadcastReceiver's onReceive, after Service command handling,
+     * etc.  (so async work is never lost)
+     */
+    public static void waitToFinish() {
+        Runnable toFinish;
+        while ((toFinish = sPendingWorkFinishers.poll()) != null) {
+            toFinish.run();
+        }
+    }
+}
diff --git a/core/java/android/content/SharedPreferences.java b/core/java/android/content/SharedPreferences.java
index f1b1490..b3db2ac 100644
--- a/core/java/android/content/SharedPreferences.java
+++ b/core/java/android/content/SharedPreferences.java
@@ -40,7 +40,9 @@ public interface SharedPreferences {
         /**
          * Called when a shared preference is changed, added, or removed. This
          * may be called even if a preference is set to its existing value.
-         * 
+         *
+         * <p>This callback will be run on your main thread.
+         *
          * @param sharedPreferences The {@link SharedPreferences} that received
          *            the change.
          * @param key The key of the preference that was changed, added, or
@@ -187,9 +189,6 @@ public interface SharedPreferences {
          * <p>If you call this from an {@link android.app.Activity},
          * the base class will wait for any async commits to finish in
          * its {@link android.app.Activity#onPause}.</p>
-         *
-         * @return Returns true if the new values were successfully written
-         * to persistent storage.
          */
         void startCommit();
     }
diff --git a/core/java/android/os/Handler.java b/core/java/android/os/Handler.java
index 2a32e54..3b2bf1e 100644
--- a/core/java/android/os/Handler.java
+++ b/core/java/android/os/Handler.java
@@ -563,13 +563,13 @@ public class Handler {
             return mMessenger;
         }
     }
-    
+
     private final class MessengerImpl extends IMessenger.Stub {
         public void send(Message msg) {
             Handler.this.sendMessage(msg);
         }
     }
-    
+
     private final Message getPostMessage(Runnable r) {
         Message m = Message.obtain();
         m.callback = r;
diff --git a/core/java/android/preference/Preference.java b/core/java/android/preference/Preference.java
index 197d976..1453329 100644
--- a/core/java/android/preference/Preference.java
+++ b/core/java/android/preference/Preference.java
@@ -1195,7 +1195,7 @@ public class Preference implements Comparable<Preference>, OnDependencyChangeLis
 
     private void tryCommit(SharedPreferences.Editor editor) {
         if (mPreferenceManager.shouldCommit()) {
-            editor.commit();
+            editor.startCommit();
         }
     }
     
diff --git a/core/java/android/server/BluetoothService.java b/core/java/android/server/BluetoothService.java
index f00389b..abd66ae 100644
--- a/core/java/android/server/BluetoothService.java
+++ b/core/java/android/server/BluetoothService.java
@@ -1737,7 +1737,7 @@ public class BluetoothService extends IBluetooth.Stub {
                         mContext.getSharedPreferences(SHARED_PREFERENCES_NAME,
                                 mContext.MODE_PRIVATE).edit();
                     editor.putBoolean(SHARED_PREFERENCE_DOCK_ADDRESS + mDockAddress, true);
-                    editor.commit();
+                    editor.startCommit();
                 }
             }
         }