WebView: don't allow processes to share data dir.

WebView: don't allow processes to share data dir.

When two processes in the same app use WebView at once, they share a
data directory, which is unsafe. Change the existing data directory
locking code so that it throws a fatal exception in this case instead of
only printing a warning.

Preserve the current warning-only behaviour for a small number of
applications that are known to rely on this unsupported behaviour, until
we've been able to confirm that they no longer need this.

Reinstate the disabled test which verifies this behaviour is forbidden.

BUG=558377
Committed: https://crrev.com/b077d802dd71ab917f22c2be4afe07a3564d150b
Cr-Commit-Position: refs/heads/master@{#435209}

[Torne, 2016-11-22 19:41:15]

torne@chromium.org changed reviewers:
+ sgurun@chromium.org

[Torne, 2016-11-22 19:41:15]

Given that we've just discovered that this kind of usage breaks multiprocess
(see bug) I want to push ahead with making this crash again, and see what
feedback we get from QA, beta and incremental rollout to find out if there are
other apps that depend on this or not. We can revert this if we get more reports
of apps that crash on it.

Verified that the test passes with my change.

https://codereview.chromium.org/2522053002/diff/1/android_webview/java/src/or...
File android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java
(right):

https://codereview.chromium.org/2522053002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java:128:
if (checkMinAppVersion(context, "com.facebook.katana", 34592776)
Minimum versions here were provided by facebook.

[sgurun-gerrit only, 2016-11-22 21:42:07]

https://codereview.chromium.org/2522053002/diff/1/android_webview/java/src/or...
File android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java
(right):

https://codereview.chromium.org/2522053002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java:128:
if (checkMinAppVersion(context, "com.facebook.katana", 34592776)
On 2016/11/22 19:41:15, Torne wrote:
> Minimum versions here were provided by facebook.

is our plan to extend this list similarly if more applications have the same
issue? or from previous experience do we only expect facebook/instagram GMSCore
to have that?

[Torne, 2016-11-23 11:48:35]

On 2016/11/22 21:42:07, sgurun wrote:
>
https://codereview.chromium.org/2522053002/diff/1/android_webview/java/src/or...
> File
android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java
> (right):
> 
>
https://codereview.chromium.org/2522053002/diff/1/android_webview/java/src/or...
>
android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java:128:
> if (checkMinAppVersion(context, "com.facebook.katana", 34592776)
> On 2016/11/22 19:41:15, Torne wrote:
> > Minimum versions here were provided by facebook.
> 
> is our plan to extend this list similarly if more applications have the same
> issue? or from previous experience do we only expect facebook/instagram
GMSCore
> to have that?

When we tried to ship this before we had to roll it back very quickly because it
broke facebook and GMSCore. I want to try to ship it again so that we can see if
it breaks any other major apps. We can decide what to do depending on which apps
we see break; if lots of things are broken that's important to know because of
the multiprocess issue. I'm not intending to add any other apps to this list
unless it's just a matter of one or two major apps that we can reach out to and
get them to release a fixed version.

[sgurun-gerrit only, 2016-11-23 17:21:27]

On 2016/11/23 11:48:35, Torne wrote:
> On 2016/11/22 21:42:07, sgurun wrote:
> >
>
https://codereview.chromium.org/2522053002/diff/1/android_webview/java/src/or...
> > File
> android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/2522053002/diff/1/android_webview/java/src/or...
> >
>
android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java:128:
> > if (checkMinAppVersion(context, "com.facebook.katana", 34592776)
> > On 2016/11/22 19:41:15, Torne wrote:
> > > Minimum versions here were provided by facebook.
> > 
> > is our plan to extend this list similarly if more applications have the same
> > issue? or from previous experience do we only expect facebook/instagram
> GMSCore
> > to have that?
> 
> When we tried to ship this before we had to roll it back very quickly because
it
> broke facebook and GMSCore. I want to try to ship it again so that we can see
if
> it breaks any other major apps. We can decide what to do depending on which
apps
> we see break; if lots of things are broken that's important to know because of
> the multiprocess issue. I'm not intending to add any other apps to this list
> unless it's just a matter of one or two major apps that we can reach out to
and
> get them to release a fixed version.

we may need to talk on this on GVC.

[sgurun-gerrit only, 2016-11-29 21:47:37]

On 2016/11/23 17:21:27, sgurun wrote:
> On 2016/11/23 11:48:35, Torne wrote:
> > On 2016/11/22 21:42:07, sgurun wrote:
> > >
> >
>
https://codereview.chromium.org/2522053002/diff/1/android_webview/java/src/or...
> > > File
> > android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2522053002/diff/1/android_webview/java/src/or...
> > >
> >
>
android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java:128:
> > > if (checkMinAppVersion(context, "com.facebook.katana", 34592776)
> > > On 2016/11/22 19:41:15, Torne wrote:
> > > > Minimum versions here were provided by facebook.
> > > 
> > > is our plan to extend this list similarly if more applications have the
same
> > > issue? or from previous experience do we only expect facebook/instagram
> > GMSCore
> > > to have that?
> > 
> > When we tried to ship this before we had to roll it back very quickly
because
> it
> > broke facebook and GMSCore. I want to try to ship it again so that we can
see
> if
> > it breaks any other major apps. We can decide what to do depending on which
> apps
> > we see break; if lots of things are broken that's important to know because
of
> > the multiprocess issue. I'm not intending to add any other apps to this list
> > unless it's just a matter of one or two major apps that we can reach out to
> and
> > get them to release a fixed version.
> 
> we may need to talk on this on GVC.

lgtm, however if somebody else asks for the same type of whitelisting, let's
carefully consider it.

[Torne, 2016-11-30 10:11:21]

Sure, the intention is that we'll roll this back if it breaks other apps.

[Torne, 2016-11-30 10:11:24]

The CQ bit was checked by torne@chromium.org

[commit-bot: I haz the power, 2016-11-30 10:11:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 10:48:50]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1480500684606590, "parent_rev":
"12bd118b6baf96b8d32219c63d55cf9219237a03", "commit_rev":
"ae23ffa4616b8857c70bdefd74ec7198c3e885b0"}

[commit-bot: I haz the power, 2016-11-30 10:49:21]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-30 10:50:59]

Description was changed from

==========
WebView: don't allow processes to share data dir.

When two processes in the same app use WebView at once, they share a
data directory, which is unsafe. Change the existing data directory
locking code so that it throws a fatal exception in this case instead of
only printing a warning.

Preserve the current warning-only behaviour for a small number of
applications that are known to rely on this unsupported behaviour, until
we've been able to confirm that they no longer need this.

Reinstate the disabled test which verifies this behaviour is forbidden.

BUG=558377
==========

to

==========
WebView: don't allow processes to share data dir.

When two processes in the same app use WebView at once, they share a
data directory, which is unsafe. Change the existing data directory
locking code so that it throws a fatal exception in this case instead of
only printing a warning.

Preserve the current warning-only behaviour for a small number of
applications that are known to rely on this unsupported behaviour, until
we've been able to confirm that they no longer need this.

Reinstate the disabled test which verifies this behaviour is forbidden.

BUG=558377

Committed: https://crrev.com/b077d802dd71ab917f22c2be4afe07a3564d150b
Cr-Commit-Position: refs/heads/master@{#435209}
==========

[commit-bot: I haz the power, 2016-11-30 10:51:00]

Patchset 1 (id:??) landed as
https://crrev.com/b077d802dd71ab917f22c2be4afe07a3564d150b
Cr-Commit-Position: refs/heads/master@{#435209}