Nate: the upstreaming instructions should probably call out the need to check the downstream system_webview_manifest and system_webview_apk targets, since that's one more place where downstream overrides to the public build config might be happening.

Ideally we would have a process where we could verify that system_webview_apk produced the same output from an upstream checkout vs a downstream checkout (as long as you're building a public release - it will be different for a development release but that's fine), but right now I doubt our build is deterministic enough for this to be reliable even if there was an easy way to do it (which I'm not sure there is).

Added some pointers to the instructions: https://sites.google.com/a/google.com/clank/engineering/android-webview/next-layers

> but right now I doubt our build is deterministic enough for this to be reliable even if there was an easy way to do it (which I'm not sure there is).

While a file diff probably won't work, we could hypothetically parse the XML and diff the two trees. This would permit any order for attributes, but would be sensitive to extra/missing attributes/elements, out of order elements, whitespace, etc. Although, we could relax these equivalency-checks if needed (e.g. whitespace).

I meant everything in the APK, not just the manifest. Thanks for the note there.

We don't currently have an automated way to build the upstream vs downstream versions at the same exact commits, so hey.

> I meant everything in the APK

Oh yeah, not a chance...

A manifest-verifier would still be valuable (for example, it would've caught this bug and 0d2b66b37ec1c4247b0850f980834a9c978fbce7).

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

commit 5f887bacbf00943eb426f4c9c948b1a96b949286
Author: Torne (Richard Coles) <torne@google.com>
Date: Tue Jan 02 16:23:52 2018

Add missing visibleToInstantApps to webview manifest.

This was missed when upstreaming O-specific changes to WebView, and is
required for Instant Apps to be able to use WebView. Add it as a jinja
variable instead of to the template to avoid a downstream breakage (will
clean up in followup.

Bug:  797014 
Change-Id: I21f94eb5f84388421804562d820fbfcdec1ca19b
Reviewed-on: https://chromium-review.googlesource.com/843167
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Commit-Queue: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526487}
[modify] https://crrev.com/5f887bacbf00943eb426f4c9c948b1a96b949286/android_webview/BUILD.gn
[modify] https://crrev.com/5f887bacbf00943eb426f4c9c948b1a96b949286/chrome/android/monochrome_android_manifest_jinja_variables.gni

Torne is this now fixed?

It'd be ideal to merge this to M64.

This bug requires manual review: M64 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

To be clear: this isn't intended to have any effect on our official builds which already have the correct attribute, but merging it to M64 will enable people building from the open source tree to have a properly functioning webview.

Is this a regression in M64?

Please omit the question in 12.

This isn't a regression in M64, it was broken since the upstreaming of the O code.

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

commit 8bec4176812ce435755f0238db6a6ff00229e93c
Author: Torne (Richard Coles) <torne@google.com>
Date: Mon Jan 08 22:13:55 2018

Add missing visibleToInstantApps to webview manifest.

This was missed when upstreaming O-specific changes to WebView, and is
required for Instant Apps to be able to use WebView. Add it as a jinja
variable instead of to the template to avoid a downstream breakage (will
clean up in followup.

(cherry picked from commit 5f887bacbf00943eb426f4c9c948b1a96b949286)

Bug:  797014 
Change-Id: I21f94eb5f84388421804562d820fbfcdec1ca19b
Reviewed-on: https://chromium-review.googlesource.com/843167
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Commit-Queue: Richard Coles <torne@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#526487}
Reviewed-on: https://chromium-review.googlesource.com/855078
Reviewed-by: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#450}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/8bec4176812ce435755f0238db6a6ff00229e93c/android_webview/BUILD.gn
[modify] https://crrev.com/8bec4176812ce435755f0238db6a6ff00229e93c/chrome/android/monochrome_android_manifest_jinja_variables.gni

Keeping this open to clean up the gn files on master a bit, but the important part is done.

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

commit f7649e81a495811dc076c79bf7bbfc0c257376fb
Author: Torne (Richard Coles) <torne@google.com>
Date: Mon Feb 26 19:16:21 2018

Clean up AndroidManifest variable.

sandboxed_service_extra_flags is only used to set
visibleToInstantApps=true, which should always be true for both WebView
and Monochrome. Just set the attribute in the manifest directly and
remove the variable entirely to simplify the configuration and remove
the possibility that someone makes a new target which forgets to set it
(since the breakage caused by not setting this is subtle and easy to
miss).

Bug:  797014 
Change-Id: Iec2075c23c4e71302a10d1f15b83b290618f1a32
Reviewed-on: https://chromium-review.googlesource.com/920262
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539225}
[modify] https://crrev.com/f7649e81a495811dc076c79bf7bbfc0c257376fb/android_webview/BUILD.gn
[modify] https://crrev.com/f7649e81a495811dc076c79bf7bbfc0c257376fb/android_webview/apk/java/AndroidManifest.xml
[modify] https://crrev.com/f7649e81a495811dc076c79bf7bbfc0c257376fb/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/f7649e81a495811dc076c79bf7bbfc0c257376fb/chrome/android/monochrome_android_manifest_jinja_variables.gni

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

commit a79f781c1b2ba241875c36f2559060b6ccac7d15
Author: Torne (Richard Coles) <torne@google.com>
Date: Wed Feb 28 23:46:32 2018

GN files simplified, the variable that caused this mistake is entirely gone :)

Please add the manual verification steps if necessary .Thanks

I'm not sure how to test this manually, but it should be possible. Without this fix, any android Instant App that uses webview is completely broken on O+ devices and simply crashes. So, if you can find a suitable instant app it's easy to test..

Paul, do you know of any good instant apps to check with?

Nope. Sorry. Maybe mariakhomenko@?

You can use buzzfeed.com/tasty - pretty sure it uses a webview.

We tested on few instant apps like buzzfeed.com/tasty , Nytimes Crossword and unable to repro the crash on 66.0.3359.14 on Pixel XL/ OPM1.171019.028 however could not repro the app launch crash on 64.0.3282.137 / 61 as well . Please clarify the verification steps .Thanks in advance