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

Issue 803228 link

Starred by 0 users

Issue metadata

Status: Verified
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Task



Sign in to add a comment

Remove J & K support from WebView

Project Member Reported by paulmiller@chromium.org, Jan 17 2018

Issue description

We have a few references to J and K in WebView code:

https://cs.chromium.org/search/?q=file:%5Esrc/android_webview/+lang:java+%5CbBuild%5C.VERSION_CODES%5C.(JELLY_BEAN(_MR%5Cd)?%7CKITKAT)%5Cb

Is there any reason not to remove these codepaths?
 

Comment 1 by boliu@chromium.org, Jan 17 2018

The app target version checks can't change. Those are independent of what OS version webview supports.

Other ones can be cleaned up. We should no longer even run tests on kitkat, so you could bump the minsdk version for all the webview targets (instrumentation test shell in particular) to L, and see if there are things that can be cleaned up
Cc: gsennton@chromium.org
Cc: ntfschr@chromium.org
I looked through this code search link [1] (includes LOLLIPOP) and I think things fall into these categories:

 1. Comparing with targetSdk (don't remove this)
 2. `Build.VERSION.SDK_INT <= Build.VERSION_CODES.LOLLIPOP` (we could swap `<=` with `==`, but we don't gain anything from this)
 3. @TargetApi annotation (if we can implicitly specify we run on >= L, then we could remove some of these)
 4. @MinAndroidSdkLevel(Build.VERSION_CODES.KITKAT) (maybe we can bump this to L, but do we gain anything?)
 5. Build.VERSION.SDK_INT comparisons in WebView shell browser: app has minSdk=Kitkat, so I opened http://crrev/c/930361 for this.

I opened a CL for #5. I think #3 would have only a small benefit for us. Do we need to tackle the others?

[1] https://cs.chromium.org/search/?q=file:%5Esrc/android_webview/+lang:java+%5CbBuild%5C.VERSION_CODES%5C.(JELLY_BEAN%7CKITKAT%7CLOLLIPOP)&type=cs

Comment 4 by boliu@chromium.org, Feb 22 2018

3. and 4. both involve bumping the (instrumentation test shell) manifest min sdk level from K to L

5. is orthogonal to this. though probably not much use supporting K in the webview shell these days
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 22 2018

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

commit d36a6f4c4327df2ff703c95261ff929ecd085e15
Author: Nate Fischer <ntfschr@chromium.org>
Date: Thu Feb 22 05:20:47 2018

AW: remove a check for Kitkat in test shell

No change to WebView, this only affects the test shell (WebView shell
browser). This is not expected to have any impact on the shell, it just
removes a redundant check.

This comparison should be safe to remove because the AndroidManifest.xml
already specifies minSdkVersion="19" (19 is Kitkat).

Bug:  803228 
Test: Check that the shell still compiles without error messages.
Change-Id: I8db593324a7aeb3624065a3af68a81495c467698
Reviewed-on: https://chromium-review.googlesource.com/930361
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538354}
[modify] https://crrev.com/d36a6f4c4327df2ff703c95261ff929ecd085e15/android_webview/tools/system_webview_shell/apk/src/org/chromium/webview_shell/WebViewBrowserActivity.java

Owner: ntfschr@chromium.org
Status: Assigned (was: Untriaged)
I have a CL out for 3. and 4. in the instrumentation test shell, so I'll take this issue.

 - I don't see real incentive to bump shell browser's minSdk, so I think that's wontfix.
 - 1. and 2. are wontfix
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 22 2018

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

commit 1cf758f803a320fd2554b1e7aab8e2be550fb2cf
Author: Nate Fischer <ntfschr@chromium.org>
Date: Thu Feb 22 23:36:58 2018

AW: bump minSdk for test shell and remove annotations

No change to logic, this only affects annotations (and minSdk for test shell)

This bumps the minSdkVersion for the test shell to LOLLIPOP because we
don't run tests on KITKAT anymore. Also, this removes some
MinAndroidSdkLevel/TargetApi annotations, because they shouldn't be necessary
anymore.

This also removes TargetApi annotations from most of production code.

Bug:  803228 
Test: run_webview_instrumentation_test_apk -f AwVariationsConfigurationServiceTest#*
Change-Id: Ia8f0cd0d5b169596c1ebc038097487e65cecf3df
Reviewed-on: https://chromium-review.googlesource.com/929872
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538618}
[modify] https://crrev.com/1cf758f803a320fd2554b1e7aab8e2be550fb2cf/android_webview/BUILD.gn
[modify] https://crrev.com/1cf758f803a320fd2554b1e7aab8e2be550fb2cf/android_webview/glue/java/src/com/android/webview/chromium/ContentSettingsAdapter.java
[modify] https://crrev.com/1cf758f803a320fd2554b1e7aab8e2be550fb2cf/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/1cf758f803a320fd2554b1e7aab8e2be550fb2cf/android_webview/glue/java/src/com/android/webview/chromium/WebViewContentsClientAdapter.java
[modify] https://crrev.com/1cf758f803a320fd2554b1e7aab8e2be550fb2cf/android_webview/glue/java/src/com/android/webview/chromium/WebViewDelegateFactory.java
[modify] https://crrev.com/1cf758f803a320fd2554b1e7aab8e2be550fb2cf/android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java
[modify] https://crrev.com/1cf758f803a320fd2554b1e7aab8e2be550fb2cf/android_webview/java/src/org/chromium/android_webview/crash/AwMinidumpUploadJobService.java
[modify] https://crrev.com/1cf758f803a320fd2554b1e7aab8e2be550fb2cf/android_webview/java/src/org/chromium/android_webview/crash/CrashReceiverService.java
[modify] https://crrev.com/1cf758f803a320fd2554b1e7aab8e2be550fb2cf/android_webview/java/src/org/chromium/android_webview/variations/AwVariationsConfigurationService.java
[modify] https://crrev.com/1cf758f803a320fd2554b1e7aab8e2be550fb2cf/android_webview/java/src/org/chromium/android_webview/variations/AwVariationsSeedFetchService.java
[modify] https://crrev.com/1cf758f803a320fd2554b1e7aab8e2be550fb2cf/android_webview/javatests/AndroidManifest.xml
[modify] https://crrev.com/1cf758f803a320fd2554b1e7aab8e2be550fb2cf/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientOnUnhandledKeyEventTest.java
[modify] https://crrev.com/1cf758f803a320fd2554b1e7aab8e2be550fb2cf/android_webview/javatests/src/org/chromium/android_webview/test/variations/AwVariationsConfigurationServiceTest.java

Status: Fixed (was: Assigned)
I think this is mostly everything. Unless someone thinks we need to go further, I'm closing this.
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