Actually, it looks like this regressed in M60.

Looks like this was broken by the change which separated locale paks for chrome and webview: WebView extracts the compressed locale paks for chrome. I don't know whether it ends up actually using them or not, but it extracts them on startup anyway.

This needs fixing, and we need to clean up the existing extracted files as well ideally (delete the directory if it exists).

:( indeed.

Also scary is that the resource extractor's clean-up logic will be running, which does a "rm -r $APP_DATA/paks". Hopefully no other app out there has such a directory... 

As for a fix, I think we should just remove the call to ResourceExtractor from BrowserStartupController. There's even a comment there saying it's not necessary for Chrome. Other content apps can just store .pak files uncompressed. I'd even go as far as to say we should move ResourceExtractor out of base and into chrome/.

As for cleanup code, let's maybe put code in AwBrowserProcess.start() to remove paks/*.pak, and also remove paks/ if empty. This should be done on a background thread.

Adding RBS for M61, as I think we should consider fixing this asap.

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

commit 7b5d2466ed0b2dc453b93c542ce6a6846f2ffa0a
Author: Felix <zpeng@chromium.org>
Date: Mon Aug 14 20:31:43 2017

Removes unnecessary resource extraction step for Android assets

This CL removes unnecessary resource extraction step from
BrowserStartupController. This would stop Monochrome from automatically
extracting Chrome locale paks for WebView applications.

Previously, WebView's AwBrowserProcess would invoke
BrowserStartupController.startBrowserProcessesSync, which involved
the necessary resource extraction. Meanwhile, Chrome's resource
extraction is handled by ChromeBrowserInitializer. And as content_shell
now stores their resource pak uncompressed, this step is unnecssary
for non-test APKs.

This CL also modifies NativeLibraryTestCommon so that test APKs that
use Chrome's locale paks extract compressed locale paks at start up.

Bug:  752510 
Change-Id: I638f0e76c86df95f4f7d3ef817c401cea3932c85
Reviewed-on: https://chromium-review.googlesource.com/610824
Commit-Queue: Felix <zpeng@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494169}
[modify] https://crrev.com/7b5d2466ed0b2dc453b93c542ce6a6846f2ffa0a/base/android/java/src/org/chromium/base/ResourceExtractor.java
[modify] https://crrev.com/7b5d2466ed0b2dc453b93c542ce6a6846f2ffa0a/content/public/android/java/src/org/chromium/content/browser/BrowserStartupController.java
[modify] https://crrev.com/7b5d2466ed0b2dc453b93c542ce6a6846f2ffa0a/content/public/test/android/javatests/src/org/chromium/content/browser/test/NativeLibraryTestCommon.java

@torne, would you please verify that this indeed stops WebView from extracting Chrome locale paks? Thanks

FYI note: This is what looks like now for M60 stable (60.0.3112.105)

./app_webview/paks
./app_webview/paks/en-GB.pak@15de69e1c5e
./app_webview/paks/en-US.pak@15de69e1c5e

Just adding it here for possible future use

Verified with Felix that with his patch the pak files no longer extract and webview can still load pages (and error pages)

Merge approved for M61 branch 3163.

Created  crbug.com/755675  to track clean up task.

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

commit c0b1d49f210a826355499a9dc84ce78d06b2da0b
Author: Felix <zpeng@chromium.org>
Date: Thu Aug 17 15:34:21 2017

Removes unnecessary resource extraction step for Android assets

This CL removes unnecessary resource extraction step from
BrowserStartupController. This would stop Monochrome from automatically
extracting Chrome locale paks for WebView applications.

Previously, WebView's AwBrowserProcess would invoke
BrowserStartupController.startBrowserProcessesSync, which involved
the necessary resource extraction. Meanwhile, Chrome's resource
extraction is handled by ChromeBrowserInitializer. And as content_shell
now stores their resource pak uncompressed, this step is unnecssary
for non-test APKs.

This CL also modifies NativeLibraryTestCommon so that test APKs that
use Chrome's locale paks extract compressed locale paks at start up.

TBR=zpeng@chromium.org

(cherry picked from commit 7b5d2466ed0b2dc453b93c542ce6a6846f2ffa0a)

Bug:  752510 
Change-Id: I638f0e76c86df95f4f7d3ef817c401cea3932c85
Reviewed-on: https://chromium-review.googlesource.com/610824
Commit-Queue: Felix <zpeng@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#494169}
Reviewed-on: https://chromium-review.googlesource.com/619008
Reviewed-by: Felix <zpeng@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#629}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/c0b1d49f210a826355499a9dc84ce78d06b2da0b/base/android/java/src/org/chromium/base/ResourceExtractor.java
[modify] https://crrev.com/c0b1d49f210a826355499a9dc84ce78d06b2da0b/content/public/android/java/src/org/chromium/content/browser/BrowserStartupController.java
[modify] https://crrev.com/c0b1d49f210a826355499a9dc84ce78d06b2da0b/content/public/test/android/javatests/src/org/chromium/content/browser/test/NativeLibraryTestCommon.java

 Issue 758914  has been merged into this issue.

 Issue 758984  has been merged into this issue.

As well as additional ANRs caused by this there are also some crashes being reported; they're possibly from devices where the disk is too full to extract the pak files and so it fails with an IOException? At least, I'm not sure what else it could be.. :/

I can't get a hold of zpeng@ to have this merged into M60 branch. Can someone else help here?

I can help with this. M60 is already at 100% though, no?

Thanks agrieve@! Yes, M60 is already at 100% but since Clank has to respin, we are willing to also include this fix for WebView. Please request merge for M60.

Gotcha. Merge-Request label added then.

Thanks for considering the fix for M60.
When will the fix to M60 reach out to 100% users? What's the schedule ?

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

commit 1c6f399e3b8add497f161394b98cdbb8dbd7b514
Author: Andrew Grieve <agrieve@chromium.org>
Date: Wed Aug 30 20:49:57 2017

Removes unnecessary resource extraction step for Android assets

This CL removes unnecessary resource extraction step from
BrowserStartupController. This would stop Monochrome from automatically
extracting Chrome locale paks for WebView applications.

Previously, WebView's AwBrowserProcess would invoke
BrowserStartupController.startBrowserProcessesSync, which involved
the necessary resource extraction. Meanwhile, Chrome's resource
extraction is handled by ChromeBrowserInitializer. And as content_shell
now stores their resource pak uncompressed, this step is unnecssary
for non-test APKs.

This CL also modifies NativeLibraryTestCommon so that test APKs that
use Chrome's locale paks extract compressed locale paks at start up.

TBR=zpeng@chromium.org

(cherry picked from commit 7b5d2466ed0b2dc453b93c542ce6a6846f2ffa0a)

Bug:  752510 
Change-Id: I638f0e76c86df95f4f7d3ef817c401cea3932c85
Reviewed-on: https://chromium-review.googlesource.com/610824
Commit-Queue: Felix <zpeng@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#494169}
Reviewed-on: https://chromium-review.googlesource.com/644350
Cr-Commit-Position: refs/branch-heads/3112@{#749}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}
[modify] https://crrev.com/1c6f399e3b8add497f161394b98cdbb8dbd7b514/base/android/java/src/org/chromium/base/ResourceExtractor.java
[modify] https://crrev.com/1c6f399e3b8add497f161394b98cdbb8dbd7b514/content/public/android/java/src/org/chromium/content/browser/BrowserStartupController.java
[modify] https://crrev.com/1c6f399e3b8add497f161394b98cdbb8dbd7b514/content/public/test/android/javatests/src/org/chromium/content/browser/test/NativeLibraryTestCommon.java

The plan is to released to 10% tomorrow and monitor how things go and probably go to 100% on Friday.

Glad we're picking this up now as there have been a number of bugs filed that indicate that this is somehow actually crashing apps with permission errors that should be impossible :/ I'm looking into how that happened, but not calling this code will probably help.. it seems very surprising though.

Tried to repro in production 60.0.2112.107 having 2 Sony xperia Z5(33.3.A.2.33/7.0) and Sony Xperia Z5 (32.4.A.0.160/7.1.1) with messaging app sending messages with many apps installed in both the devices as per steps mentioned in comment #1 in  crbug/758914 but could not reproduce. 

Tried same steps in 60.0.3112.116 and issue could not verifiable.

The ANR reported in  issue 758914  is not the problem I'm concerned about - the ANR is the result of the "sync tasks added to startup" I mentioned in this issue. This code caused extra synchronous work which can potentially deadlock/delay app startup in rare cases - now that we've removed it again, that's resolved.

The issue I'm concerned about is that some app developers are reporting that this code was actually *crashing* their app with an IOException, which shouldn't have been a possible symptom of the bug. We don't have a repro for this, because all the reports are just based on third party crash reporting systems collecting data from users, not developers who've actually seen the issue themselves. I'm concerned about this even though we've removed the code in question, because if it's not possible for webview to write some data to its data directory during startup in ResourceExtractor then it likely also won't be possible for webview to write other data which is still needed.

I don't think the test team can do anything to help here as we don't have any hint of how to repro the crashing behaviour; I'm working with the developers on internal bugs to try to narrow things down.

 Issue 761218  has been merged into this issue.

To prevent regressions like this causing ANRs in future we need to address  issue 542151 , which is on my todo list.