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;
+import android.content.pm.PackageInfo;
 import android.content.res.Configuration;
 import android.graphics.Bitmap;
 import android.graphics.Canvas;
 import android.graphics.Paint;
 import android.graphics.Picture;
 import android.graphics.Rect;
 import android.graphics.drawable.Drawable;
 import android.net.Uri;
 import android.net.http.SslCertificate;
 import android.os.Build;
@@ skipping 34 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;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
  * 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)
  * and a small set of no-op deprecated APIs.
  */
 @SuppressWarnings("deprecation")
@@ skipping 17 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 static volatile AtomicBoolean sIsHtcMail = new AtomicBoolean(false);

[Torne, 2016/08/12 09:24:23]

For future reference, don't use volatile for this - make it final, but per my
comments later there's no need for atomics (or a boolean) at all. :)

[Changwan Ryu, 2016/08/12 11:14:06]

Acknowledged.

[Torne, 2016/08/12 13:25:41]

Your new CL still has this as volatile. If we actually need this and aren't
going to remove it entirely, it should be final.

[Changwan Ryu, 2016/08/16 10:30:24]

My bad. Fixed as final.

+
     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) {
         mWebView = webView;
@@ skipping 13 matching lines @@
     }
 
     // WebViewProvider methods --------------------------------------------------------------------
 
     @Override
     // BUG=6790250 |javaScriptInterfaces| was only ever used by the obsolete DumpRenderTree
     // so is ignored. TODO: remove it from WebViewProvider.
     @TargetApi(Build.VERSION_CODES.LOLLIPOP)
     public void init(final Map<String, Object> javaScriptInterfaces,
             final boolean privateBrowsing) {
+        sIsHtcMail.set(isHtcMail(mWebView));

[Torne, 2016/08/12 09:24:23]

There's no need to do this in WebViewChromium and have to keep testing the
package and so on - it's much easier to just do this in
WebViewChromiumFactoryProvider.initialize() which will execute only once.

[Changwan Ryu, 2016/08/12 11:14:06]

Unfortunately, disableThreadChecking should be called every time
WebViewChromium.init() gets called because sEnforceThreadChecking will be set
every time WebView gets instantiated.

Having this logic spread out between WebViewChromium and WebViewChromiumProvider
is probably not a good idea. Isn't it?

[Torne, 2016/08/12 13:25:41]

Ah, I didn't realise it was reset in the constructor. That's terrible, and
actually broken in some vanishingly unlikely cases (multiple apps in the same
process with different target sdk levels) :(
It is if it avoids having to do atomics, or recheck package names over and over
redundantly. However, you don't have to spread it out in this case: you could
check whether it's HTC Mail once in global initialisation, and then set
sEnforceThreadChecking back to the right value in
WebViewChromiumFactoryProvider.createWebView.

[Changwan Ryu, 2016/08/16 10:30:24]

Moved the checking logic to factory and replaced atomicboolean by final.
BTW, I assume that sEnforceThreadChecking in HTC mail would be set to false
while other apps should have the right value. Why do we need to set
sEnforceThreadChecking back to the right value? 

+        if (sIsHtcMail.get()) disableThreadChecking(mWebView);
         if (privateBrowsing) {
             mFactory.startYourEngines(true);
             final String msg = "Private browsing is not supported in WebView.";
             if (mAppTargetSdkVersion >= Build.VERSION_CODES.KITKAT) {
                 throw new IllegalArgumentException(msg);
             } else {
                 Log.w(TAG, msg);
                 TextView warningLabel = new TextView(mContext);
                 warningLabel.setText(mContext.getString(
                         org.chromium.android_webview.R.string.private_browsing_warning));
@@ skipping 43 matching lines @@
                     // 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();
                 }
             }
         });
     }
 
+    private boolean isHtcMail(View view) {
+        if (mAppTargetSdkVersion > Build.VERSION_CODES.M) return false;
+        final String htcMailPackageId = "com.htc.android.mail";
+        if (view.getClass().getName() == null

[Torne, 2016/08/12 09:24:23]

Instead of checking the class name of a view, just call
Context.getPackageName().

[Changwan Ryu, 2016/08/12 11:14:06]

Done.

+                || !view.getClass().getName().startsWith(htcMailPackageId)) {
+            return false;
+        }
+
+        try {
+            PackageInfo packageInfo =
+                    view.getContext().getPackageManager().getPackageInfo(htcMailPackageId, 0);
+            // These values are provided by HTC.
+            if ((mAppTargetSdkVersion < Build.VERSION_CODES.M
+                    && packageInfo.versionCode < 864021756)
+                    || packageInfo.versionCode < 866001861) {
+                Log.w(TAG, "Disabling thread check in WebView (http://crbug.com/622151). "
+                        + "APK name: " + htcMailPackageId + ", versionCode: "
+                        + packageInfo.versionCode);
+                return true;
+            }
+        } catch (Exception e) {
+            e.printStackTrace();

[Torne, 2016/08/12 09:24:23]

Printing a stack trace and throwing the exception away is never really useful.
Catch more specific exception types here, and just swallow them silently; if the
package manager can't tell us the version of the mail app then we're not in a
state where enabling this workaround is going to help us :)

[Changwan Ryu, 2016/08/12 11:14:06]

Changed not to print stack trace.

+        }
+        return false;
+    }
+
+    // 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(View view) {
+        for (Class<?> webViewClass = view.getClass();

[Torne, 2016/08/12 09:24:23]

No need to search the class hierarchy here (and this is fragile, since it's only
doing a substring match): just use Class.forName("android.webkit.WebView"). It's
also a static field, so it's sufficient to do this once. So, it can just be done
immediately after the check in WebViewChromiumFactoryProvider.initialize()

[Changwan Ryu, 2016/08/12 11:14:06]

Done. BTW, this should be done for each WebView.init() call.

+                webViewClass != null;
+                webViewClass = webViewClass.getSuperclass()) {
+            String name = webViewClass.getName();
+            if (name != null && name.contains("WebView")) {
+                try {
+                    Field field = webViewClass.getDeclaredField("sEnforceThreadChecking");
+                    field.setAccessible(true);
+                    field.setBoolean(null, false);
+                    field.setAccessible(false);
+                } catch (NoSuchFieldException | IllegalAccessException
+                        | IllegalArgumentException e) {
+                    e.printStackTrace();

[Torne, 2016/08/12 09:24:23]

If we failed here, probably just print a single-line error saying we couldn't
turn it off. The exception is unlikely to be interesting.

[Changwan Ryu, 2016/08/12 11:14:06]

Done.

+                }
+                return;
+            }
+        }
+    }
+
     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());
 
         if (mAppTargetSdkVersion >= Build.VERSION_CODES.KITKAT) {
             // On KK and above, favicons are automatically downloaded as the method
@@ skipping 322 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) {
+    public void evaluateJavaScript(
+            final String script, final ValueCallback<String> resultCallback) {
+        if (sIsHtcMail.get()) {
+            // This is a workaround for https://crbug.com/622151.
+            ThreadUtils.postOnUiThread(new Runnable() {

[Torne, 2016/08/12 09:24:23]

Use checkNeedsPost() instead of doing it unconditionally, and just call
mAwContents.evaluateJavaScript directly instead of introducing the extra method.

[Changwan Ryu, 2016/08/12 11:14:06]

Changed to runOnUiThread and removed extra method.

+                @Override
+                public void run() {
+                    evaluateJavaScriptInternal(script, resultCallback);
+                }
+            });
+        } else {
+            evaluateJavaScriptInternal(script, resultCallback);
+        }
+    }
+
+    private void evaluateJavaScriptInternal(String script, ValueCallback<String> resultCallback) {
         checkThread();
         mAwContents.evaluateJavaScript(script, resultCallback);
     }
 
     @Override
     public void saveWebArchive(String filename) {
         saveWebArchive(filename, false, null);
     }
 
     @Override
@@ skipping 1673 matching lines @@
         mAwContents.extractSmartClipData(x, y, width, height);
     }
 
     // Implements SmartClipProvider
     @Override
     public void setSmartClipResultHandler(final Handler resultHandler) {
         checkThread();
         mAwContents.setSmartClipResultHandler(resultHandler);
     }
 }