A few of those are probably okay since they are implemented entirely inside the framework and not by the webview APK: for example MimeTypeMap and URLUtil probably?

Not sure that I follow the reasoning - none of these are implemented by the WebView APK, we just want to remove these dependencies to avoid depending on the Android API, no? (and I guess to avoid apps having use any android.webkit* APIs with the support library - so for that reason we could leave MimeTypeMap and URLUtil be since they won't be used by the app together with the support library) 

Hm. What I mean is that those classes aren't really part of the WebView API and so there's no dependency issue; but you're right that it's still a bit annoying for the app to have to know which is which. I guess we could just put duplicates of these in the support library which either copy the implementations or delegate to the framework versions.. not sure if that's better or not.

after all that's fixed, we should have a top level DEPS ban of importing android.webkit.*, and carve out specific projects, and maybe specific allowed classes like torne mentioned.

> android.webkit.JavascriptInterface, not sure how this works (it's an annotation), the only class that uses this except tests and the glue-layer is AwContents.

We'll just pass the correct annotation Class object from glue. It's already passed from AwContents to ContentViewCore, so should be fine.

Right, yeah we could potentially just ignore the classes that are never passed between the existing glue and AwContents et al. (e.g. MimeTypeMap) since they won't have an app-developer-visible impact (unless someone changes their frameworks implementation). Though, then again, it would be nice to limit our dependencies on the framework :)

> We'll just pass the correct annotation Class object from glue. It's already passed from AwContents to ContentViewCore, so should be fine.

Ah, I see what you mean, nice :)

Prototyping the support library will take a while, so I'm gonna leave this bug as available for now in case anyone wants to throw some time on it (it should be fairly simple to fix this bug for new team members as well).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a4c2d94336dbd503dd996dcc28e271e75539e355

commit a4c2d94336dbd503dd996dcc28e271e75539e355
Author: Alex Chu <alexchu@google.com>
Date: Wed Sep 13 21:45:09 2017

Remove references to WebSettings.PluginState from AwSettings.

Change-Id: Ifadb017bd536758e75f8f664c3b74a9770e2bb42
Bug:  736726 
Reviewed-on: https://chromium-review.googlesource.com/651749
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Alex Chu <alexchu@google.com>
Cr-Commit-Position: refs/heads/master@{#501761}
[modify] https://crrev.com/a4c2d94336dbd503dd996dcc28e271e75539e355/android_webview/glue/java/src/com/android/webview/chromium/ContentSettingsAdapter.java
[modify] https://crrev.com/a4c2d94336dbd503dd996dcc28e271e75539e355/android_webview/java/src/org/chromium/android_webview/AwSettings.java

> after all that's fixed, we should have a top level DEPS ban of importing android.webkit.*, and carve out specific projects, and maybe specific allowed classes like torne mentioned.

As we saw in  issue 764479 , it turns out that a DEPS rule probably won't be the right solution. check_deps.py maps class names -> chromium file paths. android.webkit.* classes don't have real chromium file paths, so there's no way to block the import.

I added a presubmit check that blocks android.webkit.ValueCallback. When we're finished with this work, I can extend the presubmit for all android.webkit.* classes.

---

If we see any further conflicts with downstream (i.e. clank/), please let me know and I can help out.

> Note: android.webkit.* is used in several tests, and in remoting/ and chrome/ to create an actual WebView. These use-cases seem legit.

re c#0, is this actually legit? I thought we explicitly disallowed instantiating a WebView inside of Chrome for Android (because of Monochrome-related weirdness). Is this not the case?

remoting is a separate app I think. chrome itself definitely can't be creating webviews (and it doesn't), but tests is like.. a grey area.., there can be individual exceptions I guess

re #11, good question. It makes sense to use android.webkit in tests.

chrome/ uses the classes
android.webkit.MimeTypeMap;
android.webkit.URLUtil;

in several locations.
ExternalNavigationHandler.java uses android.webkit.WebView.SCHEME_TEL

other uses in chrome/ are all for tests.


haven't checked remoting/ right now.

I agree, tests are a grey area. It's probably fine to include the classes, but we'll need to handle this in our presubmit check.

Ok, if it's just frameworks classes that don't go into chromium (I think that's the case), then that's ok. Still, tricky to write the presubmit.

Ack regarding remoting/. I'll make sure the final presubmit ignores that directory.

MimeTypeMap and URLUtil are both just static utility classes implemented entirely in the framework, and don't pose any risk. Constants are also fine like SCHEME_TEL.

It may be better to get rid of some of these if they don't make a lot of sense, though, rather than exclude them from the presubmit. Case-by-case basis..

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0244e9267b324946f391eaa88a558aa49494c7ca

commit 0244e9267b324946f391eaa88a558aa49494c7ca
Author: Alex Chu <alexchu@google.com>
Date: Mon Sep 18 22:25:13 2017

Remove references to ConsoleMessage from non-glue-layers.

Change-Id: Ibd0353fdb76cbe490eae9d6ae7b3c2ae3851b074
Bug:  736726 
Reviewed-on: https://chromium-review.googlesource.com/651751
Commit-Queue: Alex Chu <alexchu@google.com>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502693}
[modify] https://crrev.com/0244e9267b324946f391eaa88a558aa49494c7ca/android_webview/BUILD.gn
[modify] https://crrev.com/0244e9267b324946f391eaa88a558aa49494c7ca/android_webview/glue/java/src/com/android/webview/chromium/WebViewContentsClientAdapter.java
[add] https://crrev.com/0244e9267b324946f391eaa88a558aa49494c7ca/android_webview/java/src/org/chromium/android_webview/AwConsoleMessage.java
[modify] https://crrev.com/0244e9267b324946f391eaa88a558aa49494c7ca/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
[modify] https://crrev.com/0244e9267b324946f391eaa88a558aa49494c7ca/android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java
[modify] https://crrev.com/0244e9267b324946f391eaa88a558aa49494c7ca/android_webview/javatests/src/org/chromium/android_webview/test/ConsoleMessagesForBlockedLoadsTest.java
[modify] https://crrev.com/0244e9267b324946f391eaa88a558aa49494c7ca/android_webview/javatests/src/org/chromium/android_webview/test/TestAwContentsClient.java
[modify] https://crrev.com/0244e9267b324946f391eaa88a558aa49494c7ca/android_webview/test/shell/src/org/chromium/android_webview/test/NullContentsClient.java

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/87bf275c9e6acb87f663952412c9d4af83d5fb98

commit 87bf275c9e6acb87f663952412c9d4af83d5fb98
Author: Alex Chu <alexchu@google.com>
Date: Tue Sep 19 01:00:30 2017

Remove references to GeolocationPermissions.Callback from non-glue
layers.

Change-Id: I22890aa54b375e4d40b9acbd9c5b47056be7432c
Bug:  736726 
Reviewed-on: https://chromium-review.googlesource.com/651752
Commit-Queue: Alex Chu <alexchu@google.com>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502742}
[modify] https://crrev.com/87bf275c9e6acb87f663952412c9d4af83d5fb98/android_webview/glue/java/src/com/android/webview/chromium/WebViewContentsClientAdapter.java
[modify] https://crrev.com/87bf275c9e6acb87f663952412c9d4af83d5fb98/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
[modify] https://crrev.com/87bf275c9e6acb87f663952412c9d4af83d5fb98/android_webview/java/src/org/chromium/android_webview/AwGeolocationPermissions.java
[modify] https://crrev.com/87bf275c9e6acb87f663952412c9d4af83d5fb98/android_webview/java/src/org/chromium/android_webview/permission/AwGeolocationCallback.java
[modify] https://crrev.com/87bf275c9e6acb87f663952412c9d4af83d5fb98/android_webview/javatests/src/org/chromium/android_webview/test/GeolocationTest.java
[modify] https://crrev.com/87bf275c9e6acb87f663952412c9d4af83d5fb98/android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellActivity.java
[modify] https://crrev.com/87bf275c9e6acb87f663952412c9d4af83d5fb98/android_webview/test/shell/src/org/chromium/android_webview/test/NullContentsClient.java

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22f9a4d0b5a9a2167ccac4cf9cc5348874f5d6d0

commit 22f9a4d0b5a9a2167ccac4cf9cc5348874f5d6d0
Author: Alex Chu <alexchu@google.com>
Date: Tue Sep 19 03:52:09 2017

Remove references to WebChromeClient.CustomViewCallback from non-glue
layers.

Change-Id: I87749f4a3160da78c8443b6d1e97a64e0cce507d
Bug:  736726 
Reviewed-on: https://chromium-review.googlesource.com/651753
Commit-Queue: Alex Chu <alexchu@google.com>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502778}
[modify] https://crrev.com/22f9a4d0b5a9a2167ccac4cf9cc5348874f5d6d0/android_webview/glue/java/src/com/android/webview/chromium/WebViewContentsClientAdapter.java
[modify] https://crrev.com/22f9a4d0b5a9a2167ccac4cf9cc5348874f5d6d0/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
[modify] https://crrev.com/22f9a4d0b5a9a2167ccac4cf9cc5348874f5d6d0/android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java
[modify] https://crrev.com/22f9a4d0b5a9a2167ccac4cf9cc5348874f5d6d0/android_webview/javatests/src/org/chromium/android_webview/test/FullScreenVideoTestAwContentsClient.java
[modify] https://crrev.com/22f9a4d0b5a9a2167ccac4cf9cc5348874f5d6d0/android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellActivity.java
[modify] https://crrev.com/22f9a4d0b5a9a2167ccac4cf9cc5348874f5d6d0/android_webview/test/shell/src/org/chromium/android_webview/test/NullContentsClient.java

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c0d03630d6983a75f33df7de01c7f5ef1118558

commit 5c0d03630d6983a75f33df7de01c7f5ef1118558
Author: Alex Chu <alexchu@google.com>
Date: Tue Sep 19 05:18:02 2017

Remove references to WebChromeClient.FileChooserParams from non-glue
layers.

Change-Id: I9e3119a11f8949f1452eeca19da45bbd9775222e
Bug:  736726 
Reviewed-on: https://chromium-review.googlesource.com/651755
Commit-Queue: Alex Chu <alexchu@google.com>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502783}
[modify] https://crrev.com/5c0d03630d6983a75f33df7de01c7f5ef1118558/android_webview/glue/java/src/com/android/webview/chromium/WebViewContentsClientAdapter.java
[modify] https://crrev.com/5c0d03630d6983a75f33df7de01c7f5ef1118558/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java

The public interface of AwContents and related classes should be free of android.webkit now.

The remaining references are int constants, static utils (ie. URLUtil) and JavascriptInterface:

src/org/chromium/android_webview/AwServiceWorkerSettings.java
10:import android.webkit.WebSettings;

src/org/chromium/android_webview/AwContents.java
48:import android.webkit.JavascriptInterface;

src/org/chromium/android_webview/AwSettings.java
15:import android.webkit.WebSettings;

src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java
20:import android.webkit.URLUtil;

src/org/chromium/android_webview/ErrorCodeConversionHelper.java
7:import android.webkit.WebViewClient;

JavascriptInterface is not a constant and definitely needs to be moved up to glue layer

others are kinda lower priority I guess

JavascriptInterface is just an annotation - the only thing that matters is what type it is. There's nothing to "convert" here; you can't change the types of the annotations on classes at runtime. It effectively *is* just a constant: it's just the binary name "Landroid/webkit/JavascriptInterface;" in the .class file.

right, I don't mean convert it. I just mean AwContents will need to just take an annotation type like ContentViewCore does.

Presumably the support library will have its own copy of JavascriptInterface, in which case the value of that "constant" is different between webview and support lib :p

Oh, I see what you mean; yeah, I guess it should work that way.

> he remaining references are int constants, static utils (ie. URLUtil) and JavascriptInterface

gsennton@ do you think we need further work here? I think constants filled-in during compile time, URLUtil is in API level 1, and JavascriptInterface is in API level 17 (which is pre-L). Now that we're using an augmenting support lib, I think the remaining android.webkit references can be postponed as cleanup work.

Hmm, I can't quite get my head around this at the moment. I don't think that the augmenting support lib means we don't want to remove references to older Android classes - but on the other hand these are very few cases and I don't think we've touched e.g. UrlUtil for a long time, (as long as the implementation wasn't updated since L removing the reference is not urgent). I think we have more urgents tasks at hand yes :)

I'm going to close this:

 * We've removed enough android.webkit imports to unblock support lib
 * I'm not convinced there's a benefit to removing all the imports