Prevent thread assertion for HTC mail apk

android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package com.android.webview.chromium;
 
 import android.annotation.SuppressLint;
 import android.annotation.TargetApi;
 import android.content.Context;
 import android.content.Intent;
@@ skipping 44 matching lines @@
 import org.chromium.android_webview.AwPrintDocumentAdapter;
 import org.chromium.android_webview.AwSettings;
 import org.chromium.android_webview.ResourcesContextWrapperFactory;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.content.browser.SmartClipProvider;
 import org.chromium.content_public.browser.NavigationHistory;
 
 import java.io.BufferedWriter;
 import java.io.File;
+import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.util.Map;
 import java.util.concurrent.Callable;
 
 /**
  * This class is the delegate to which WebViewProxy forwards all API calls.
  *
  * Most of the actual functionality is implemented by AwContents (or ContentViewCore within
  * it). This class also contains WebView-specific APIs that require the creation of other
  * adapters (otherwise org.chromium.content would depend on the webview.chromium package)
@@ skipping 20 matching lines @@
     private AwContents mAwContents;
     // Non-null if this webview is using the GL accelerated draw path.
     private DrawGLFunctor mGLfunctor;
 
     private final WebView.HitTestResult mHitTestResult;
 
     private final int mAppTargetSdkVersion;
 
     private WebViewChromiumFactoryProvider mFactory;
 
+    private final boolean mShouldDisableThreadChecking;
+
     private static boolean sRecordWholeDocumentEnabledByApi = false;
     static void enableSlowWholeDocumentDraw() {
         sRecordWholeDocumentEnabledByApi = true;
     }
 
     // This does not touch any global / non-threadsafe state, but note that
     // init is ofter called right after and is NOT threadsafe.
     public WebViewChromium(WebViewChromiumFactoryProvider factory, WebView webView,
-            WebView.PrivateAccess webViewPrivate) {
+            WebView.PrivateAccess webViewPrivate, boolean shouldDisableThreadChecking) {
         mWebView = webView;
         mWebViewPrivate = webViewPrivate;
         mHitTestResult = new WebView.HitTestResult();
         mContext = ResourcesContextWrapperFactory.get(mWebView.getContext());
         mAppTargetSdkVersion = mContext.getApplicationInfo().targetSdkVersion;
         mFactory = factory;
+        mShouldDisableThreadChecking = shouldDisableThreadChecking;
         factory.getWebViewDelegate().addWebViewAssetPath(mWebView.getContext());
     }
 
     static void completeWindowCreation(WebView parent, WebView child) {
         AwContents parentContents = ((WebViewChromium) parent.getWebViewProvider()).mAwContents;
         AwContents childContents =
                 child == null ? null : ((WebViewChromium) child.getWebViewProvider()).mAwContents;
         parentContents.supplyContentsForPopup(childContents);
     }
 
@@ skipping 59 matching lines @@
                 initForReal();
                 if (privateBrowsing) {
                     // Intentionally irreversibly disable the webview instance, so that private
                     // user data cannot leak through misuse of a non-privateBrowing WebView
                     // instance. Can't just null out mAwContents as we never null-check it
                     // before use.
                     destroy();
                 }
             }
         });
+        // This is called here because otherwise compiler optimization may call

[Torne, 2016/08/17 10:43:07]

I'm really suspicious of this comment. What do you mean exactly? Called here as
opposed to where else? Are we protected by a lock here that we aren't in the
other location?

[Changwan Ryu, 2016/08/17 14:07:41]

As opposed to do it inside WebViewChromiumFactoryProvider::createWebView().
Removed comment to avoid confusion.

[Torne, 2016/08/17 14:14:34]

I'm still confused about what this means, though. Why would it be a problem to
do it in createWebView? The memory model doesn't really work in a way that means
an unsafe ordering can be prevented by just moving code around; you need actual
barriers/locks. So, unless I'm missing something here, either it was safe in
either place, or it's not safe in either place.

[Changwan Ryu, 2016/08/18 09:31:02]

Hmm.. I had to look up a bit.
https://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html
JLS mentions reflection use case in Example 17.5.3-1, and it seems that
reflection does not give ordering guarantee, so there is still room that
reordering can happen between the line in WebView's constructor and the line
here.

The difference when I put the code in createWebView() and when I put it in
WebViewChromium.init() comes from, I think, ART's behavior under normal
optimization settings. I haven't tested it against official build optimization /
HTC's variation of ART, so there is uncertainty that this can actually work or
not.

BTW, I couldn't find a good way to set a memory barrier in this case. Do you
have any suggestion?

+        // disableThreadChecking before setting WebView#sEnforceThreadChecking.
+        if (mShouldDisableThreadChecking) disableThreadChecking();
+    }
+
+    // This is a workaround for https://crbug.com/622151.
+    // In HTC's email app, InputConnection.setComposingText() will call WebView.evaluateJavaScript,
+    // and thread assertion will occur. We turn off WebView thread assertion for this app.
+    private void disableThreadChecking() {
+        try {
+            Class<?> webViewClass = Class.forName("android.webkit.WebView");
+            Field field = webViewClass.getDeclaredField("sEnforceThreadChecking");
+            field.setAccessible(true);
+            field.setBoolean(null, false);
+            field.setAccessible(false);
+        } catch (ClassNotFoundException | NoSuchFieldException | IllegalAccessException
+                | IllegalArgumentException e) {
+            Log.w(TAG, "Failed to disable thread checking.");
+        }
     }
 
     private void initForReal() {
         AwContentsStatics.setRecordFullDocument(sRecordWholeDocumentEnabledByApi
                 || mAppTargetSdkVersion < Build.VERSION_CODES.LOLLIPOP);
 
         mAwContents = new AwContents(mFactory.getBrowserContextOnUiThread(), mWebView, mContext,
                 new InternalAccessAdapter(), new WebViewNativeDrawGLFunctorFactory(),
                 mContentsClientAdapter, mWebSettings.getAwSettings());
 
@@ skipping 324 matching lines @@
                 @Override
                 public void run() {
                     mAwContents.loadDataWithBaseURL(baseUrl, data, mimeType, encoding, historyUrl);
                 }
             });
             return;
         }
         mAwContents.loadDataWithBaseURL(baseUrl, data, mimeType, encoding, historyUrl);
     }
 
-    public void evaluateJavaScript(String script, ValueCallback<String> resultCallback) {
-        checkThread();
-        mAwContents.evaluateJavaScript(script, resultCallback);
+    @Override
+    public void evaluateJavaScript(
+            final String script, final ValueCallback<String> resultCallback) {
+        if (mShouldDisableThreadChecking) {
+            // This is a workaround for https://crbug.com/622151.
+            if (checkNeedsPost()) {

[Torne, 2016/08/17 10:43:07]

You need to combine this into the previous if condition, otherwise when
mShouldDisableThreadChecking is true but checkNeedsPost returns false, the call
will be entirely dropped.

[Changwan Ryu, 2016/08/17 14:07:41]

Good catch! Done.

+                mFactory.addTask(new Runnable() {
+                    @Override
+                    public void run() {
+                        mAwContents.evaluateJavaScript(script, resultCallback);
+                    }
+                });
+                return;
+            }
+        } else {
+            checkThread();
+            mAwContents.evaluateJavaScript(script, resultCallback);
+        }
     }
 
     @Override
     public void saveWebArchive(String filename) {
         saveWebArchive(filename, false, null);
     }
 
     @Override
     public void saveWebArchive(final String basename, final boolean autoname,
             final ValueCallback<String> callback) {
@@ skipping 1671 matching lines @@
         mAwContents.extractSmartClipData(x, y, width, height);
     }
 
     // Implements SmartClipProvider
     @Override
     public void setSmartClipResultHandler(final Handler resultHandler) {
         checkThread();
         mAwContents.setSmartClipResultHandler(resultHandler);
     }
 }