Move implementation of load methods to chromium layer

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

Index: android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
diff --git a/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java b/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
index 79424ec12ab1fba5441e6f62eb569781043c7d72..f36742c6b2d3d6d43a13089f1abc6da5e64397fb 100644
--- a/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
+++ b/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
@@ -19,8 +19,6 @@ import android.os.Handler;
 import android.os.Looper;
 import android.os.Message;
 import android.print.PrintDocumentAdapter;
-import android.text.TextUtils;
-import android.util.Base64;
 import android.util.Log;
 import android.view.KeyEvent;
 import android.view.MotionEvent;
@@ -50,12 +48,10 @@ import org.chromium.android_webview.AwPrintDocumentAdapter;
 import org.chromium.android_webview.AwSettings;
 import org.chromium.base.ThreadUtils;
 import org.chromium.content.browser.SmartClipProvider;
-import org.chromium.content_public.browser.LoadUrlParams;
 
 import java.io.BufferedWriter;
 import java.io.File;
 import java.lang.annotation.Annotation;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.Queue;
 import java.util.concurrent.Callable;
@@ -506,117 +502,76 @@ class WebViewChromium implements WebViewProvider, WebViewProvider.ScrollDelegate
     }
 
     @Override
-    public void loadUrl(final String url, Map<String, String> additionalHttpHeaders) {
-        // TODO: We may actually want to do some sanity checks here (like filter about://chrome).
-
-        // For backwards compatibility, apps targeting less than K will have JS URLs evaluated
-        // directly and any result of the evaluation will not replace the current page content.
-        // Matching Chrome behavior more closely; apps targetting >= K that load a JS URL will
-        // have the result of that URL replace the content of the current page.
-        final String javaScriptScheme = "javascript:";
-        if (mAppTargetSdkVersion < Build.VERSION_CODES.KITKAT && url != null
-                && url.startsWith(javaScriptScheme)) {
-            mFactory.startYourEngines(true);
-            if (checkNeedsPost()) {
-                mRunQueue.addTask(new Runnable() {
-                    @Override
-                    public void run() {
-                        mAwContents.evaluateJavaScriptEvenIfNotYetNavigated(
-                                url.substring(javaScriptScheme.length()));
-                    }
-                });
-            } else {
-                mAwContents.evaluateJavaScriptEvenIfNotYetNavigated(
-                        url.substring(javaScriptScheme.length()));
-            }
+    public void loadUrl(final String url, final Map<String, String> additionalHttpHeaders) {
+        mFactory.startYourEngines(true);
+        if (checkNeedsPost()) {
+            // Disallowed in WebView API for apps targetting a new SDK
+            assert mAppTargetSdkVersion < Build.VERSION_CODES.JELLY_BEAN_MR2;

[Torne, 2015/03/09 10:57:39]

This isn't a new restriction, right? I can't see where this was checked in the
old code.

We almost certainly shouldn't use assert for this kind of check in any case; it
throws a very generic exception and can be disabled. assert is only for
debugging assertions checking internal state, not for validating input/caller
behaviour.

[boliu, 2015/03/09 14:35:43]

It's not new. Copied from Line 623 from the left side.

Although I'm not sure whether it did anything. I think we don't compile asserts
in, right?

+            mRunQueue.addTask(new Runnable() {
+                @Override
+                public void run() {
+                    mAwContents.loadUrl(url, additionalHttpHeaders);
+                }
+            });
             return;
         }
-
-        LoadUrlParams params = new LoadUrlParams(url);
-        if (additionalHttpHeaders != null) params.setExtraHeaders(additionalHttpHeaders);
-        loadUrlOnUiThread(params);
+        mAwContents.loadUrl(url, additionalHttpHeaders);
     }
 
     @Override
-    public void loadUrl(String url) {
-        // Early out to match old WebView implementation
-        if (url == null) {

[boliu, 2015/03/09 03:33:56]

This refactor does change behavior. Eg this check will happen *after*
startYourEngines or addTask now.

[Torne, 2015/03/09 10:57:39]

Do you know why we had this check in the first place? It seems like if someone
was actually relying on loadUrl(null) being a no-op, then making it start chrome
might well break their logic :(

[boliu, 2015/03/09 14:35:43]

http://b/10688422

I think whether to lock in the UI thread is a orthogonal issue though, and
really for us to decide.

+    public void loadUrl(final String url) {
+        mFactory.startYourEngines(true);
+        if (checkNeedsPost()) {
+            // Disallowed in WebView API for apps targetting a new SDK
+            assert mAppTargetSdkVersion < Build.VERSION_CODES.JELLY_BEAN_MR2;
+            mRunQueue.addTask(new Runnable() {
+                @Override
+                public void run() {
+                    mAwContents.loadUrl(url);
+                }
+            });
             return;
         }
-        loadUrl(url, null);
+        mAwContents.loadUrl(url);
     }
 
     @Override
-    public void postUrl(String url, byte[] postData) {
-        LoadUrlParams params = LoadUrlParams.createLoadHttpPostParams(url, postData);
-        Map<String, String> headers = new HashMap<String, String>();
-        headers.put("Content-Type", "application/x-www-form-urlencoded");
-        params.setExtraHeaders(headers);
-        loadUrlOnUiThread(params);
-    }
-
-    private static String fixupMimeType(String mimeType) {
-        return TextUtils.isEmpty(mimeType) ? "text/html" : mimeType;
-    }
-
-    private static String fixupData(String data) {
-        return TextUtils.isEmpty(data) ? "" : data;
-    }
-
-    private static String fixupBase(String url) {
-        return TextUtils.isEmpty(url) ? "about:blank" : url;
-    }
-
-    private static String fixupHistory(String url) {
-        return TextUtils.isEmpty(url) ? "about:blank" : url;
-    }
-
-    private static boolean isBase64Encoded(String encoding) {
-        return "base64".equals(encoding);
-    }
-
-    @Override
-    public void loadData(String data, String mimeType, String encoding) {
-        loadUrlOnUiThread(LoadUrlParams.createLoadDataParams(
-                fixupData(data), fixupMimeType(mimeType), isBase64Encoded(encoding)));
+    public void postUrl(final String url, final byte[] postData) {
+        mFactory.startYourEngines(true);
+        if (checkNeedsPost()) {
+            // Disallowed in WebView API for apps targetting a new SDK
+            assert mAppTargetSdkVersion < Build.VERSION_CODES.JELLY_BEAN_MR2;
+            mRunQueue.addTask(new Runnable() {
+                @Override
+                public void run() {
+                    mAwContents.postUrl(url, postData);
+                }
+            });
+            return;
+        }
+        mAwContents.postUrl(url, postData);
     }
 
     @Override
-    public void loadDataWithBaseURL(String baseUrl, String data, String mimeType, String encoding,
-            String historyUrl) {
-        data = fixupData(data);
-        mimeType = fixupMimeType(mimeType);
-        LoadUrlParams loadUrlParams;
-        baseUrl = fixupBase(baseUrl);
-        historyUrl = fixupHistory(historyUrl);
-
-        if (baseUrl.startsWith("data:")) {
-            // For backwards compatibility with WebViewClassic, we use the value of |encoding|
-            // as the charset, as long as it's not "base64".
-            boolean isBase64 = isBase64Encoded(encoding);
-            loadUrlParams = LoadUrlParams.createLoadDataParamsWithBaseUrl(
-                    data, mimeType, isBase64, baseUrl, historyUrl, isBase64 ? null : encoding);
-        } else {
-            // When loading data with a non-data: base URL, the classic WebView would effectively
-            // "dump" that string of data into the WebView without going through regular URL
-            // loading steps such as decoding URL-encoded entities. We achieve this same behavior by
-            // base64 encoding the data that is passed here and then loading that as a data: URL.
-            try {
-                loadUrlParams = LoadUrlParams.createLoadDataParamsWithBaseUrl(
-                        Base64.encodeToString(data.getBytes("utf-8"), Base64.DEFAULT), mimeType,
-                        true, baseUrl, historyUrl, "utf-8");
-            } catch (java.io.UnsupportedEncodingException e) {
-                Log.wtf(TAG, "Unable to load data string " + data, e);
-                return;
-            }
+    public void loadData(final String data, final String mimeType, final String encoding) {
+        mFactory.startYourEngines(true);
+        if (checkNeedsPost()) {
+            // Disallowed in WebView API for apps targetting a new SDK
+            assert mAppTargetSdkVersion < Build.VERSION_CODES.JELLY_BEAN_MR2;
+            mRunQueue.addTask(new Runnable() {
+                @Override
+                public void run() {
+                    mAwContents.loadData(data, mimeType, encoding);
+                }
+            });
+            return;
         }
-        loadUrlOnUiThread(loadUrlParams);
+        mAwContents.loadData(data, mimeType, encoding);
     }
 
-    private void loadUrlOnUiThread(final LoadUrlParams loadUrlParams) {
-        // This is the last point that we can delay starting the Chromium backend up
-        // and if the app has not caused us to bind the Chromium UI thread to a background thread
-        // we now bind Chromium's notion of the UI thread to the app main thread.
+    @Override
+    public void loadDataWithBaseURL(final String baseUrl, final String data, final String mimeType,
+            final String encoding, final String historyUrl) {
         mFactory.startYourEngines(true);
         if (checkNeedsPost()) {
             // Disallowed in WebView API for apps targetting a new SDK
@@ -624,12 +579,12 @@ class WebViewChromium implements WebViewProvider, WebViewProvider.ScrollDelegate
             mRunQueue.addTask(new Runnable() {
                 @Override
                 public void run() {
-                    mAwContents.loadUrl(loadUrlParams);
+                    mAwContents.loadDataWithBaseURL(baseUrl, data, mimeType, encoding, historyUrl);
                 }
             });
             return;
         }
-        mAwContents.loadUrl(loadUrlParams);
+        mAwContents.loadDataWithBaseURL(baseUrl, data, mimeType, encoding, historyUrl);
     }
 
     public void evaluateJavaScript(String script, ValueCallback<String> resultCallback) {