New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 764479 link

Starred by 1 user
Status: Verified
Owner:
ntfschr@chromium.org
Closed: Sep 2017
Cc:
█ gsennton@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 736726



Sign in to add a comment

Use Callback.java instead of ValueCallback beneath glue layer

Project Member Reported by ntfschr@chromium.org, Sep 12 2017 
In preparation for our support library, we should remove imports of android.webkit.* from all non-glue layer classes (aka chromium layer).

One class we use all over is android.webkit.ValueCallback.

We need to keep this in the glue layer, since it's used as part of WebView APIs. However, we can remove its use in implementation-only code and replace it with Callback.java.

We should do the conversion from ValueCallback <-> Callback in the glue layer.

Comment 1 by gsennton@chromium.org, Sep 13 2017

Blocking: -725019 736726
Ah cool, I didn't know we already had Callback.java :)
Thanks Nate!
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 15 2017 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/3ad333de0f8d829eb508749abae5970b1b7532c3

commit 3ad333de0f8d829eb508749abae5970b1b7532c3
Author: Nate Fischer <ntfschr@google.com>
Date: Fri Sep 15 22:43:14 2017
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 16 2017 

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

commit 535972b3b509dd28b58982b4a150a44435a87956
Author: Nate Fischer <ntfschr@chromium.org>
Date: Sat Sep 16 01:06:18 2017

AW: prefer Callback over ValueCallback

This replaces uses of ValueCallback with org.chromium.base.Callback
everywhere outside of android_webview/glue/ and adds code in the glue
layer to convert between the types.

This is needed for WebView's support library. We intend to put all
android.webkit.* classes into a support library under a different
package name. This requires non-glue code to work without a dependency
on android.webkit.ValueCallback.

This also adds a presubmit check to enforce that we don't add any more
imports of android.webkit.ValueCallback outside the glue layer. The
check is modeled after _CheckAndroidToastUsage.

This fixes a presubmit warning regarding the @param javadoc annotation
for a test class.

Bug:  764479 
Test: git cl presubmit -u, also manually tested several affected APIs
Change-Id: I267e187326cc288f4e1d3f467e85a54a8cc01e58
Reviewed-on: https://chromium-review.googlesource.com/664182
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502458}
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/PRESUBMIT.py
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/glue/BUILD.gn
[add] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/glue/java/src/com/android/webview/chromium/CallbackConverter.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/glue/java/src/com/android/webview/chromium/CookieManagerAdapter.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/glue/java/src/com/android/webview/chromium/GeolocationPermissionsAdapter.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/glue/java/src/com/android/webview/chromium/TokenBindingManagerAdapter.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/glue/java/src/com/android/webview/chromium/WebStorageAdapter.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/glue/java/src/com/android/webview/chromium/WebViewContentsClientAdapter.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/java/src/org/chromium/android_webview/AwContentsClientCallbackHelper.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/java/src/org/chromium/android_webview/AwContentsStatics.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/java/src/org/chromium/android_webview/AwCookieManager.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/java/src/org/chromium/android_webview/AwGeolocationPermissions.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/java/src/org/chromium/android_webview/AwQuotaManagerBridge.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/java/src/org/chromium/android_webview/AwSafeBrowsingConfigHelper.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/java/src/org/chromium/android_webview/AwTokenBindingManager.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/javatests/src/org/chromium/android_webview/test/ArchiveTest.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientVisitedHistoryTest.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/javatests/src/org/chromium/android_webview/test/CookieManagerTest.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/javatests/src/org/chromium/android_webview/test/SafeBrowsingTest.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/javatests/src/org/chromium/android_webview/test/TestAwContentsClient.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/javatests/src/org/chromium/android_webview/test/crash/MinidumpUploaderTest.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/javatests/src/org/chromium/android_webview/test/util/CookieUtils.java
[modify] https://crrev.com/535972b3b509dd28b58982b4a150a44435a87956/android_webview/test/shell/src/org/chromium/android_webview/test/NullContentsClient.java

Comment 4 by ntfschr@chromium.org, Sep 16 2017 

The basic work is done. All that remains is to clean up the TODOs in downstream. I'll take care of that next week.

Comment 5 by ntfschr@chromium.org, Sep 18 2017

Labels: WebView-Support-Lib
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 19 2017 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/904bab3497e7350bb8dfe5d53f5d2f6cad32641e

commit 904bab3497e7350bb8dfe5d53f5d2f6cad32641e
Author: Nate Fischer <ntfschr@google.com>
Date: Tue Sep 19 00:20:44 2017

Comment 7 by ntfschr@chromium.org, Sep 19 2017

Status: Fixed (was: Assigned)

Comment 8 by ntfschr@chromium.org, Aug 24

Status: Verified (was: Fixed)
Bulk edit: marking stale 'fixed' bugs as 'verified' since they don't need verification at this point.
►

Sign in to add a comment