[Android WebView] Fix thread unsafety in accessing Java side getters

android_webview/java/src/org/chromium/android_webview/AwSettings.java

Index: android_webview/java/src/org/chromium/android_webview/AwSettings.java
diff --git a/android_webview/java/src/org/chromium/android_webview/AwSettings.java b/android_webview/java/src/org/chromium/android_webview/AwSettings.java
index a7cd1989bb4df690064b538a71371a8dcc9ee63e..c2713566b517a342eb5a73dda16237e2193567fe 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwSettings.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwSettings.java
@@ -119,8 +119,6 @@ public class AwSettings {
     // The native side of this object. It's lifetime is bounded by the WebContent it is attached to.
     private long mNativeAwSettings = 0;
 
-    // A flag to avoid sending superfluous synchronization messages.
-    private boolean mIsUpdateWebkitPrefsMessagePending = false;
     // Custom handler that queues messages to call native code on the UI thread.
     private final EventHandler mEventHandler;
 
@@ -129,10 +127,12 @@ public class AwSettings {
 
     // Class to handle messages to be processed on the UI thread.
     private class EventHandler {
-        // Message id for updating Webkit preferences
-        private static final int UPDATE_WEBKIT_PREFERENCES = 0;
+        // Message id for running a Runnable with mAwSettingsLock held.
+        private static final int RUN_RUNNABLE_BLOCKING = 0;
         // Actual UI thread handler
         private Handler mHandler;
+        // Synchronization flag.
+        private boolean mSynchronizationPending = false;
 
         EventHandler() {
         }
@@ -143,10 +143,12 @@ public class AwSettings {
                 @Override
                 public void handleMessage(Message msg) {
                     switch (msg.what) {
-                        case UPDATE_WEBKIT_PREFERENCES:
+                        case RUN_RUNNABLE_BLOCKING:
                             synchronized (mAwSettingsLock) {
-                                updateWebkitPreferencesOnUiThreadLocked();
-                                mIsUpdateWebkitPrefsMessagePending = false;
+                                if (mNativeAwSettings != 0) {
+                                    ((Runnable)msg.obj).run();
+                                }
+                                mSynchronizationPending = false;
                                 mAwSettingsLock.notifyAll();
                             }
                             break;
@@ -155,42 +157,40 @@ public class AwSettings {
             };
         }
 
-        void maybeRunOnUiThreadBlocking(Runnable r) {
-            if (mHandler != null) {
-                ThreadUtils.runOnUiThreadBlocking(r);
-            }
-        }
-
-        void maybePostOnUiThread(Runnable r) {
-            if (mHandler != null) {
-                mHandler.post(r);
-            }
-        }
-
-        private void updateWebkitPreferencesLocked() {
+        void runOnUiThreadBlockingAndLocked(Runnable r) {
             assert Thread.holdsLock(mAwSettingsLock);
-            if (mNativeAwSettings == 0) return;
             if (mHandler == null) return;
             if (ThreadUtils.runningOnUiThread()) {
-                updateWebkitPreferencesOnUiThreadLocked();
+                r.run();
             } else {
-                // We're being called on a background thread, so post a message.
-                if (mIsUpdateWebkitPrefsMessagePending) {
-                    return;
-                }
-                mIsUpdateWebkitPrefsMessagePending = true;
-                mHandler.sendMessage(Message.obtain(null, UPDATE_WEBKIT_PREFERENCES));
-                // We must block until the settings have been sync'd to native to
-                // ensure that they have taken effect.
+                assert !mSynchronizationPending;
+                mSynchronizationPending = true;
+                mHandler.sendMessage(Message.obtain(null, RUN_RUNNABLE_BLOCKING, r));
                 try {
-                    while (mIsUpdateWebkitPrefsMessagePending) {
+                    while (mSynchronizationPending) {
                         mAwSettingsLock.wait();
                     }
                 } catch (InterruptedException e) {
-                    Log.e(TAG, "Interrupted waiting to sync settings to native", e);
+                    Log.e(TAG, "Interrupted waiting a Runnable to complete", e);
+                    mSynchronizationPending = false;
                 }
             }
         }
+
+        void maybePostOnUiThread(Runnable r) {
+            if (mHandler != null) {
+                mHandler.post(r);
+            }
+        }
+
+        void updateWebkitPreferencesLocked() {
+            runOnUiThreadBlockingAndLocked(new Runnable() {
+                @Override
+                public void run() {
+                    updateWebkitPreferencesOnUiThreadLocked();
+                }
+            });
+        }
     }
 
     interface ZoomSupportChangeListener {
@@ -239,6 +239,7 @@ public class AwSettings {
 
     @CalledByNative
     private double getDIPScaleLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mDIPScale;
     }
 
@@ -370,12 +371,10 @@ public class AwSettings {
         synchronized (mAwSettingsLock) {
             if (mInitialPageScalePercent != scaleInPercent) {
                 mInitialPageScalePercent = scaleInPercent;
-                mEventHandler.maybeRunOnUiThreadBlocking(new Runnable() {
+                mEventHandler.runOnUiThreadBlockingAndLocked(new Runnable() {
                     @Override
                     public void run() {
-                        if (mNativeAwSettings != 0) {
-                            nativeUpdateInitialPageScaleLocked(mNativeAwSettings);
-                        }
+                        nativeUpdateInitialPageScaleLocked(mNativeAwSettings);
                     }
                 });
             }
@@ -384,6 +383,7 @@ public class AwSettings {
 
     @CalledByNative
     private float getInitialPageScalePercentLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mInitialPageScalePercent;
     }
 
@@ -398,6 +398,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getSpatialNavigationLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mSpatialNavigationEnabled;
     }
 
@@ -412,6 +413,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getEnableSupportedHardwareAcceleratedFeaturesLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mEnableSupportedHardwareAcceleratedFeatures;
     }
 
@@ -451,12 +453,10 @@ public class AwSettings {
         synchronized (mAwSettingsLock) {
             if (mAutoCompleteEnabled != enable) {
                 mAutoCompleteEnabled = enable;
-                mEventHandler.maybeRunOnUiThreadBlocking(new Runnable() {
+                mEventHandler.runOnUiThreadBlockingAndLocked(new Runnable() {
                     @Override
                     public void run() {
-                        if (mNativeAwSettings != 0) {
-                            nativeUpdateFormDataPreferencesLocked(mNativeAwSettings);
-                        }
+                        nativeUpdateFormDataPreferencesLocked(mNativeAwSettings);
                     }
                 });
             }
@@ -474,6 +474,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getSaveFormDataLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mAutoCompleteEnabled;
     }
 
@@ -497,12 +498,10 @@ public class AwSettings {
                 mUserAgent = ua;
             }
             if (!oldUserAgent.equals(mUserAgent)) {
-                mEventHandler.maybeRunOnUiThreadBlocking(new Runnable() {
+                mEventHandler.runOnUiThreadBlockingAndLocked(new Runnable() {
                     @Override
                     public void run() {
-                        if (mNativeAwSettings != 0) {
-                            nativeUpdateUserAgentLocked(mNativeAwSettings);
-                        }
+                        nativeUpdateUserAgentLocked(mNativeAwSettings);
                     }
                 });
             }
@@ -520,6 +519,7 @@ public class AwSettings {
 
     @CalledByNative
     private String getUserAgentLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mUserAgent;
     }
 
@@ -530,13 +530,11 @@ public class AwSettings {
         synchronized (mAwSettingsLock) {
             if (mLoadWithOverviewMode != overview) {
                 mLoadWithOverviewMode = overview;
-                mEventHandler.updateWebkitPreferencesLocked();
-                mEventHandler.maybeRunOnUiThreadBlocking(new Runnable() {
+                mEventHandler.runOnUiThreadBlockingAndLocked(new Runnable() {
                     @Override
                     public void run() {
-                        if (mNativeAwSettings != 0) {
-                            nativeResetScrollAndScaleState(mNativeAwSettings);
-                        }
+                        updateWebkitPreferencesOnUiThreadLocked();
+                        nativeResetScrollAndScaleState(mNativeAwSettings);
                     }
                 });
             }
@@ -554,6 +552,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getLoadWithOverviewModeLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mLoadWithOverviewMode;
     }
 
@@ -581,6 +580,7 @@ public class AwSettings {
 
     @CalledByNative
     private int getTextSizePercentLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mTextSizePercent;
     }
 
@@ -607,6 +607,7 @@ public class AwSettings {
 
     @CalledByNative
     private String getStandardFontFamilyLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mStandardFontFamily;
     }
 
@@ -633,6 +634,7 @@ public class AwSettings {
 
     @CalledByNative
     private String getFixedFontFamilyLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mFixedFontFamily;
     }
 
@@ -659,6 +661,7 @@ public class AwSettings {
 
     @CalledByNative
     private String getSansSerifFontFamilyLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mSansSerifFontFamily;
     }
 
@@ -685,6 +688,7 @@ public class AwSettings {
 
     @CalledByNative
     private String getSerifFontFamilyLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mSerifFontFamily;
     }
 
@@ -711,6 +715,7 @@ public class AwSettings {
 
     @CalledByNative
     private String getCursiveFontFamilyLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mCursiveFontFamily;
     }
 
@@ -737,6 +742,7 @@ public class AwSettings {
 
     @CalledByNative
     private String getFantasyFontFamilyLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mFantasyFontFamily;
     }
 
@@ -764,6 +770,7 @@ public class AwSettings {
 
     @CalledByNative
     private int getMinimumFontSizeLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mMinimumFontSize;
     }
 
@@ -791,6 +798,7 @@ public class AwSettings {
 
     @CalledByNative
     private int getMinimumLogicalFontSizeLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mMinimumLogicalFontSize;
     }
 
@@ -818,6 +826,7 @@ public class AwSettings {
 
     @CalledByNative
     private int getDefaultFontSizeLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mDefaultFontSize;
     }
 
@@ -845,6 +854,7 @@ public class AwSettings {
 
     @CalledByNative
     private int getDefaultFixedFontSizeLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mDefaultFixedFontSize;
     }
 
@@ -907,6 +917,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getLoadsImagesAutomaticallyLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mLoadsImagesAutomatically;
     }
 
@@ -933,6 +944,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getImagesEnabledLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mImagesEnabled;
     }
 
@@ -947,6 +959,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getJavaScriptEnabledLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mJavaScriptEnabled;
     }
 
@@ -961,6 +974,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getAllowUniversalAccessFromFileURLsLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mAllowUniversalAccessFromFileURLs;
     }
 
@@ -975,6 +989,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getAllowFileAccessFromFileURLsLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mAllowFileAccessFromFileURLs;
     }
 
@@ -1009,10 +1024,10 @@ public class AwSettings {
     /**
      * Return true if plugins are disabled.
      * @return True if plugins are disabled.
-     * @hide
      */
     @CalledByNative
     private boolean getPluginsDisabledLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mPluginState == PluginState.OFF;
     }
 
@@ -1049,6 +1064,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getJavaScriptCanOpenWindowsAutomaticallyLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mJavaScriptCanOpenWindowsAutomatically;
     }
 
@@ -1077,10 +1093,10 @@ public class AwSettings {
      * Gets whether Text Auto-sizing layout algorithm is enabled.
      *
      * @return true if Text Auto-sizing layout algorithm is enabled
-     * @hide
      */
     @CalledByNative
     private boolean getTextAutosizingEnabledLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mLayoutAlgorithm == LayoutAlgorithm.TEXT_AUTOSIZING;
     }
 
@@ -1107,11 +1123,13 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getSupportMultipleWindowsLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mSupportMultipleWindows;
     }
 
     @CalledByNative
     private boolean getSupportLegacyQuirksLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mSupportLegacyQuirks;
     }
 
@@ -1140,11 +1158,13 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getUseWideViewportLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mUseWideViewport;
     }
 
     @CalledByNative
     private boolean getPasswordEchoEnabledLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mPasswordEchoEnabled;
     }
 
@@ -1186,10 +1206,10 @@ public class AwSettings {
      * Gets whether Application Cache is enabled.
      *
      * @return true if Application Cache is enabled
-     * @hide
      */
     @CalledByNative
     private boolean getAppCacheEnabledLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         if (!mAppCacheEnabled) {
             return false;
         }
@@ -1221,6 +1241,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getDomStorageEnabledLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mDomStorageEnabled;
     }
 
@@ -1247,6 +1268,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getDatabaseEnabledLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mDatabaseEnabled;
     }
 
@@ -1273,6 +1295,7 @@ public class AwSettings {
 
     @CalledByNative
     private String getDefaultTextEncodingLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mDefaultTextEncoding;
     }
 
@@ -1299,6 +1322,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean getMediaPlaybackRequiresUserGestureLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mMediaPlaybackRequiresUserGesture;
     }
 
@@ -1326,6 +1350,7 @@ public class AwSettings {
 
     @CalledByNative
     private String getDefaultVideoPosterURLLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mDefaultVideoPosterURL;
     }
 
@@ -1409,6 +1434,7 @@ public class AwSettings {
 
     @CalledByNative
     private boolean supportsDoubleTapZoomLocked() {
+        assert Thread.holdsLock(mAwSettingsLock);
         return mSupportZoom && mBuiltInZoomControls && mUseWideViewport;
     }
 
@@ -1445,18 +1471,25 @@ public class AwSettings {
         }
     }
 
-    private void updateWebkitPreferencesOnUiThreadLocked() {
-        if (mNativeAwSettings != 0) {
-            assert mEventHandler.mHandler != null;
-            ThreadUtils.assertOnUiThread();
-            nativeUpdateWebkitPreferencesLocked(mNativeAwSettings);
+    @CalledByNative
+    private void populateWebPreferences(long webPrefsPtr) {
+        synchronized (mAwSettingsLock) {
+            nativePopulateWebPreferencesLocked(mNativeAwSettings, webPrefsPtr);
         }
     }
 
+    private void updateWebkitPreferencesOnUiThreadLocked() {
+        assert mEventHandler.mHandler != null;
+        ThreadUtils.assertOnUiThread();
+        nativeUpdateWebkitPreferencesLocked(mNativeAwSettings);
+    }
+
     private native long nativeInit(long webContentsPtr);
 
     private native void nativeDestroy(long nativeAwSettings);
 
+    private native void nativePopulateWebPreferencesLocked(long nativeAwSettings, long webPrefsPtr);
+
     private native void nativeResetScrollAndScaleState(long nativeAwSettings);
 
     private native void nativeUpdateEverythingLocked(long nativeAwSettings);