New issue
Advanced search Search tips

Issue 774112 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Add DEPS files ensuring WebView crash services don't depend on other android_webview/ code.

Project Member Reported by gsennton@chromium.org, Oct 12 2017

Issue description

Add DEPS files ensuring WebView crash services (related to Silent Feedback) don't depend on other android_webview/ code, to minimize the risk of e.g. accidentally trying to use native code in the crash services. 
 
Shall we then combine //android_webview:android_webview_crash_services_java into //android_webview:android_webview_java, and merge the downstream ones too?

Comment 2 by torne@chromium.org, Oct 12 2017

I don't think we'd want to merge those targets; to enforce the dependencies properly we'd have to have android_webview_crash_services_java depend only on specific other java targets that we know don't break the rules here either.

(also as I said in the thread we might want to rename this target to something not crash-specific and also put the license viewer activity/contentprovider in there, since that has the same restrictions as it also runs in the apk context).
Issue 785066 has been merged into this issue.
Cc: ntfschr@chromium.org
I think the main concern here is "if we accidentally call into native code in the service, we'll crash." Is DEPS a powerful enough mechanism to handle this?

Paul, James, and I discussed today and decided that it's acceptable to merge downstream GMS targets because:

 1. We want PlatformServiceBridgeGoogle to reference AwSafeBrowsingApiHandler
 2. The GN trick didn't seem to accomplish the goal: it depends on :base_java, and there's no mechanism to ensure that doesn't descend into native

---

The other concern I perceive is "if we accidentally load the native lib, which bloats our service." I think we're reasonably safe from this as long as we never depend on the glue layer.
Re #2 here, and #2 on the dup'd bug:

From the "Java classes loaded for crash Service" email thread, it sounds like we can't enforce that even if we do separate the targets. Since separating the targets doesn't reliably achieve our goal, and separating them is blocking the refactor, it seems we should combine the targets and try to come up with a better mechanism. Maybe a test that asserts native isn't present? Or maybe the service can set a boolean in the native loading code to indicate that any loading is a bug?
Status: Available (was: Assigned)
Owner: ----

Sign in to add a comment