New issue
Advanced search Search tips

Issue 767641 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Add version check to TextClassifier related call sites.

Project Member Reported by ctzsm@chromium.org, Sep 21 2017

Issue description

Although currently it works fine because class loader do lazy loading, but the code is hard to read and confuses people.
 
Are you doing this now? I was just working on a change.

Comment 2 by ctzsm@chromium.org, Sep 22 2017

Cc: -paulmiller@chromium.org ctzsm@chromium.org
Owner: paulmiller@chromium.org
I haven't started yet, assigning to you Paul. :)
Project Member

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

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

commit 9037d3eeb2ef0f33010f5c37cb5f7908437bdce4
Author: Paul Miller <paulmiller@google.com>
Date: Fri Sep 22 21:23:48 2017

WebView: Fix brittle reference to TextClassifier on pre-O

When a relevant WebView API is called, the TextClassifier plumbed
through the following layers, in order:

- WebViewChromium
- AwContents
- ContentViewCore
- SelectionPopupController
- SmartSelectionClient (implements SelectionClient)
- SmartSelectionProvider

by the following functions:

- setTextClassifier()
- getTextClassifier()
- getCustomTextClassifier()

While most of these functions only run on >= O,
AwContents.setNewAwContents() runs on < O, and calls setTextClassifier()
and getCustomTextClassifier(). This works on < O because the
TextClassifier is null, but the JVM may still attempt to load the class
under surprising circumstances, such as casting a null Object to a
TextClassifier.

Ensure setNewAwContents() doesn't call those on < O, and add some
assertions to make it more obvious that certain codepaths should only
happen on >= O.

BUG= 767641 

Change-Id: I6525936ca51ca758d9447500115700db32b4be7c
Reviewed-on: https://chromium-review.googlesource.com/679286
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Paul Miller <paulmiller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503858}
[modify] https://crrev.com/9037d3eeb2ef0f33010f5c37cb5f7908437bdce4/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/9037d3eeb2ef0f33010f5c37cb5f7908437bdce4/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/9037d3eeb2ef0f33010f5c37cb5f7908437bdce4/content/public/android/java/src/org/chromium/content/browser/SmartSelectionClient.java

Status: Fixed (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 23 2017

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

commit 79ec628c655e29592957b70fcd76882aa2b1269e
Author: Paul Miller <paulmiller@google.com>
Date: Sat Sep 23 00:50:20 2017

WebView: Assert >= O in AwAutofillProvider

to help avoid confusion, along the lines of
9037d3eeb2ef0f33010f5c37cb5f7908437bdce4.

BUG= 767641 

Change-Id: I2ebef3380cf3bcc517201b73cf6ca8a7e613f08b
Reviewed-on: https://chromium-review.googlesource.com/679815
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Paul Miller <paulmiller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503922}
[modify] https://crrev.com/79ec628c655e29592957b70fcd76882aa2b1269e/android_webview/java/src/org/chromium/android_webview/AwAutofillProvider.java

Sign in to add a comment