for reference  crbug.com/881572  as it may be related (or may not be)..

Status update:

* Checked the SystemWebViewShell with the fix for android  crbug.com/881572 , i.e. with https://chromium-review.googlesource.com/1225606 compiled in.

* As noted on the patch multiprocess tests appear to fail when OOP. WebView runs multiprocess by default on O+.

Results:

* NetworkService + IP -> startup OK
* NetworkService + OOP -> startup crash [1]
* NetworkService + OOP + singleprocess -> startup OK

it looks like the multiprocess + OOP is an issue here.

(notes: IP=in-process, OOP=out-of-process)

--------------------------------------------------------------

[1] partial crash stack:

09-21 20:09:30.709  1675  6779 W ActivityManager: Permission Denial: Accessing service ComponentInfo{com.google.android.apps.chrome/org.chromium.content.app.PrivilegedProcessService0} from pid=20609, uid=10152 that is not exported from uid 10156
09-21 20:09:30.710 20609 20630 W System.err: java.lang.SecurityException: Not allowed to bind to service Intent { cmp=com.google.android.apps.chrome/org.chromium.content.app.PrivilegedProcessService0 (has extras) }
09-21 20:09:30.710 20609 20630 W System.err: 	at android.app.ContextImpl.bindServiceCommon(ContextImpl.java:1677)
09-21 20:09:30.711 20609 20630 W System.err: 	at android.app.ContextImpl.bindService(ContextImpl.java:1612)
09-21 20:09:30.711 20609 20630 W System.err: 	at android.content.ContextWrapper.bindService(ContextWrapper.java:698)
09-21 20:09:30.711 20609 20630 W System.err: 	at android.content.ContextWrapper.bindService(ContextWrapper.java:698)
09-21 20:09:30.711 20609 20630 W System.err: 	at org.chromium.base.process_launcher.ChildProcessConnection$ChildServiceConnectionImpl.bind(ChildProcessConnection.java:122)
09-21 20:09:30.711 20609 20630 W System.err: 	at org.chromium.base.process_launcher.ChildProcessConnection.bind(ChildProcessConnection.java:552)
09-21 20:09:30.711 20609 20630 W System.err: 	at org.chromium.base.process_launcher.ChildProcessConnection.start(ChildProcessConnection.java:364)
09-21 20:09:30.711 20609 20630 W System.err: 	at org.chromium.base.process_launcher.ChildConnectionAllocator.allocate(ChildConnectionAllocator.java:222)
09-21 20:09:30.711 20609 20630 W System.err: 	at org.chromium.base.process_launcher.ChildProcessLauncher.allocateAndSetupConnection(ChildProcessLauncher.java:196)
09-21 20:09:30.711 20609 20630 W System.err: 	at org.chromium.base.process_launcher.ChildProcessLauncher.start(ChildProcessLauncher.java:170)
09-21 20:09:30.712 20609 20630 W System.err: 	at org.chromium.content.browser.ChildProcessLauncherHelperImpl.start(ChildProcessLauncherHelperImpl.java:411)
09-21 20:09:30.712  1675  1703 I ActivityManager: Start proc 20655:com.google.android.apps.chrome:sandboxed_process0/u0i3 for webview_service org.chromium.webview_shell/org.chromium.content.app.SandboxedProcessService0
09-21 20:09:30.712 20609 20630 W System.err: 	at org.chromium.content.browser.ChildProcessLauncherHelperImpl.createAndStart(ChildProcessLauncherHelperImpl.java:225)
09-21 20:09:30.712 20609 20630 W System.err: 	at android.os.MessageQueue.nativePollOnce(Native Method)
09-21 20:09:30.712 20609 20630 W System.err: 	at android.os.MessageQueue.next(MessageQueue.java:326)
09-21 20:09:30.712 20609 20630 W System.err: 	at android.os.Looper.loop(Looper.java:160)
09-21 20:09:30.712 20609 20630 W System.err: 	at android.os.HandlerThread.run(HandlerThread.java:65)
09-21 20:09:30.714 20655 20655 I dboxed_process: Late-enabling -Xcheck:jni
09-21 20:09:30.746 20609 20630 F chromium: [FATAL:jni_android.cc(249)] Please include Java exception stack in crash report
[..]

+cc:torne@ : in case something here looks familiar (or needs more info)

You will need to change the manifest for the the privileged process services such that they are exported="true" externalService="true", the same as the unprivileged services. Otherwise, they can't be used by webview apps, which are running in a different package.

If this is just for testing you can hardcode it; if you intend to actually land this then you need to make setting exported conditional in the manifest template, as it does for the sandboxed services. Add me to a review if you're going to land a CL for this.

Also: maybe I'm missing something here, but I thought our intention was to run the network service in-process on WebView, even on OS versions where we have multiprocess renderers. Making sure the network service *does* work OOP would still be nice, though :)

Also you need to declare privileged services in the standalone webview manifest, in case you are building that instead of monochrome for testing.

OOP network service isn't ready to ship on clank in production yet, due to child process launcher / priority things. So for webview, OOP network service is more than a few steps away. In fact, I have an argument for not running network service OOP in webview, even if there is zero memory or perf regression. So I'd say this is pretty low priority.

Thanks Torne and Bo! I'll have a look into the manifest approach..

As I understood from jam@ running tests with NetworkService OOP guarantees proper coverage (IP is a more relaxed version). That's why the intention is to run webview tests with NS OOP on the android NS bot together with clank's tests.

On that bot the multiprocess targets for the webview instrumentation tests in particular are failing, which may be related to this issue (although may be something else as it's not directly systemwebviewshell related). But then there is also the CTS tests which do use real system webview.

Not sure if we would want to relax running tests for webview in particular with NetworkService IP only..

Running tests is fine

For instrumentation tests, there is a third manifest you need to update. So
monochrome is chrome/android/java/AndroidManifest.xml
standalone webview is android_webview/apk/java/AndroidManifest.xml
instrumentation test is android_webview/test/shell/AndroidManifest.xml

Right, thanks, I've tried making the PrivilegedProcessService exported and external (and isolated) but now getting the following exception:

09-24 22:28:57.055 12125 12147 W System.err: java.lang.SecurityException: BIND_EXTERNAL_SERVICE required for ComponentInfo{com.google.android.apps.chrome/org.chromium.content.app.PrivilegedProcessService0}

it seems like we might need to change some chrome code as well or is there maybe something else I might be missing?

Yes, all the places that bind to the changed services need to be changed to include BIND_EXTERNAL_SERVICE in the flags, as the error suggests.

The line to change is here:
https://cs.chromium.org/chromium/src/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncherHelperImpl.java?rcl=739789ea4ae75ce90851b965278c580ab15d7fad&l=335

Also you don't want to declare PrivilegedProcessService to be isolated

Thanks for the quick replies. Running the privileged process with [1].
However it appears the Network Service process is explicitly forced to be *not sandboxed* in [2] (patch [3]), which forces the bindAsExternalService variable to false in [4]. Hmm, ideas?


[1]
android:exported="true"
android:externalService="true"

[2]
https://cs.chromium.org/chromium/src/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncherHelperImpl.java?l=211-215

[3]
https://chromium-review.googlesource.com/c/chromium/src/+/807152/

[4] https://cs.chromium.org/chromium/src/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncherHelperImpl.java?l=335

You have to change [4], drop the sandboxed condition there

Oh, wait. There's a much bigger problem here and you can't fix it: services *must* be isolated to be external services. This simply cannot work; WebView cannot ever have a privileged service as a result.

(well, it can't have a service that is instantiated on a per-app basis, to be specific. There can still be non-external services in the WebView APK, but they will run as part of the WebView app, with the WebView's UID and permissions, and there will only be one global instance of each service on the device - this is how the crash handling and finch services work)

right, so doing the suggestion in comment #15 results in [1] as Torne indicated in #16. However making the privileged process android:isolatedProcess="true" does result in successful startup..


[1]
09-25 18:23:47.139 11929 11968 W System.err: java.lang.SecurityException: BIND_EXTERNAL_SERVICE failed, ComponentInfo{com.google.android.apps.chrome/org.chromium.content.app.PrivilegedProcessService0} is not an isolatedProcess

being isolated does not seem like a proper solution as it then has no permissions of its own..

Right - it won't be able to make network connections if it's isolated. There's no way to make this work. The platform simply does not support external services that aren't isolated; the mechanism for making the service exist per-client only works with isolated processes.

So, you'll have to abandon this and only run it in-process, unless you want to hack together something really weird that only works for local testing purposes for a single application and can't actually be landed. (you could do this by just making it a regular exported service but then it would be the *same* instance of the service for every app, meaning you can't run more than one webview app safely at once, or run webview + chrome at once in monochrome)

I defer to WebView experts about all this, I just have one reply to Bo in comment 8. Can you elaborate or sync up with cudvall on the issues you listed? AFAIK OOP network service does work fine on Clank.

I've been using Clank canary with OOP network service enabled for the last few days and it seems to work fine. Bo, is there any bugs tracking the issues you mentioned in comment 8?

The clank issue is crbug.com/872343. I think crbug.com/872343 should be considered a blocker for running experiments on clank with real users. If you are just running tests on bots, then it's not a big deal.

That bug references your comment which says "Android currently assumes the only unsandboxed child process is GPU, and needs some work before supporting arbitrary unsandboxed utility processes.". Can you expand on what's broken, i.e. given that we're launching unsandboxed and it works today?

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

commit 30c0fb35768bb601ff2c240de390088816c97ae1
Author: Tim Volodine <timvolodine@chromium.org>
Date: Thu Oct 25 19:20:28 2018

Update fyi mojo bots to run webview_instrumentation_tests with NetworkServiceInProcess

Multiprocess WebView currently does not support running network service
out of process ( crbug.com/882650 ). Therefore the intention is to run
WebView with network service in-process.

This patch makes sure the webview instrumentation tests are also running
with the network service enabled in-process to match the configuration
we are targeting for production.

Also updates the filter with some explanation and adds references to
the relevant crbugs.

BUG= 882650 ,841556

Change-Id: I73feafed259ef3d85609a949ebb0fe7f82574bfb
Reviewed-on: https://chromium-review.googlesource.com/c/1296605
Commit-Queue: Tim Volodine <timvolodine@chromium.org>
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602825}
[modify] https://crrev.com/30c0fb35768bb601ff2c240de390088816c97ae1/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/30c0fb35768bb601ff2c240de390088816c97ae1/testing/buildbot/filters/mojo.fyi.network_webview_instrumentation_test_apk.filter
[modify] https://crrev.com/30c0fb35768bb601ff2c240de390088816c97ae1/testing/buildbot/test_suites.pyl

I think we've all agreed that:
 * We will always ship at least L-P as IP, due to technical restraints in the framework
 * We have no immediate desire to change the framework to support OOP, since OOP would incur memory overhead
 * If we were to ship OOP on future Android versions, this comes at the cost of (1) implementing the framework support, (2) maintaining and testing both modes on CQ, (3) the extra process memory overhead

Closing this, because I don't think there's any work this bug needs to track? But please reopen if I'm mistaken and we do want to push for OOP on future Android releases.

Yes, indeed, at this stage targeting Network Service IP for WebView. Nate thanks for the summary.

Additionally, as Torne mentioned in comment #17 there is also the possibility of having one single service per device (similar to crash handling and finch).

Looks like other issues raised on this bug are covered in other crbugs, e.g. crbug.com/872343.

I didn't mean to suggest we could actually do that for the network service. I don't think it's intended to have a single mojo service supporting multiple separate browser processes at once; I doubt the network service can actually do that. All I meant is that's the *only* kind of service we can have, so if that kind of service doesn't work for a given use case that use case is impossible :(