Commit a5e9ce9f authored by Torne (Richard Coles)'s avatar Torne (Richard Coles) Committed by Commit Bot

WebView: Fetch resource package ID on main thread.

WebViewDelegate.getPackageId sometimes throws an exception complaining
that it's unable to find the WebView package in the resource table. This
looks like it might be some kind of obscure race caused by doing it on a
background thread at the same time as WebView initialisation (which in
some cases can also be manipulating the resource table).

The goal of the background thread is to parallelize work for a startup
time benefit, but the call to getPackageId is actually very fast (the
expensive part is calling R.onResourcesLoaded). So, just do the package
ID lookup on the main thread first and pass the ID in to the background
task, in the hope that this avoids this hypothetical race.

Bug: 995400
Change-Id: Id4479cdda54ee7b1aec273a491fa0f0f64ed348c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1854466Reviewed-by: default avatarChangwan Ryu <changwan@chromium.org>
Commit-Queue: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705102}
parent 4fe42c05
...@@ -6,7 +6,6 @@ package com.android.webview.chromium; ...@@ -6,7 +6,6 @@ package com.android.webview.chromium;
import android.Manifest; import android.Manifest;
import android.content.Context; import android.content.Context;
import android.content.pm.PackageInfo;
import android.content.pm.PackageManager; import android.content.pm.PackageManager;
import android.os.Build; import android.os.Build;
import android.os.Looper; import android.os.Looper;
...@@ -218,7 +217,7 @@ public class WebViewChromiumAwInit { ...@@ -218,7 +217,7 @@ public class WebViewChromiumAwInit {
* Set up resources on a background thread. * Set up resources on a background thread.
* @param context The context. * @param context The context.
*/ */
public void setUpResourcesOnBackgroundThread(PackageInfo webViewPackageInfo, Context context) { public void setUpResourcesOnBackgroundThread(int packageId, Context context) {
try (ScopedSysTraceEvent e = ScopedSysTraceEvent.scoped( try (ScopedSysTraceEvent e = ScopedSysTraceEvent.scoped(
"WebViewChromiumAwInit.setUpResourcesOnBackgroundThread")) { "WebViewChromiumAwInit.setUpResourcesOnBackgroundThread")) {
assert mSetUpResourcesThread == null : "This method shouldn't be called twice."; assert mSetUpResourcesThread == null : "This method shouldn't be called twice.";
...@@ -228,7 +227,7 @@ public class WebViewChromiumAwInit { ...@@ -228,7 +227,7 @@ public class WebViewChromiumAwInit {
@Override @Override
public void run() { public void run() {
// Run this in parallel as it takes some time. // Run this in parallel as it takes some time.
setUpResources(webViewPackageInfo, context); setUpResources(packageId, context);
} }
}); });
mSetUpResourcesThread.start(); mSetUpResourcesThread.start();
...@@ -244,17 +243,10 @@ public class WebViewChromiumAwInit { ...@@ -244,17 +243,10 @@ public class WebViewChromiumAwInit {
} }
} }
private void setUpResources(PackageInfo webViewPackageInfo, Context context) { private void setUpResources(int packageId, Context context) {
try (ScopedSysTraceEvent e = try (ScopedSysTraceEvent e =
ScopedSysTraceEvent.scoped("WebViewChromiumAwInit.setUpResources")) { ScopedSysTraceEvent.scoped("WebViewChromiumAwInit.setUpResources")) {
String packageName = webViewPackageInfo.packageName; R.onResourcesLoaded(packageId);
if (webViewPackageInfo.applicationInfo.metaData != null) {
packageName = webViewPackageInfo.applicationInfo.metaData.getString(
"com.android.webview.WebViewDonorPackage", packageName);
}
R.onResourcesLoaded(mFactory.getWebViewDelegate().getPackageId(
context.getResources(), packageName));
AwResource.setResources(context.getResources()); AwResource.setResources(context.getResources());
AwResource.setConfigKeySystemUuidMapping(android.R.array.config_keySystemUuidMapping); AwResource.setConfigKeySystemUuidMapping(android.R.array.config_keySystemUuidMapping);
......
...@@ -266,8 +266,18 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider { ...@@ -266,8 +266,18 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider {
// WebView needs to make sure to always use the wrapped application context. // WebView needs to make sure to always use the wrapped application context.
ContextUtils.initApplicationContext(ResourcesContextWrapperFactory.get(ctx)); ContextUtils.initApplicationContext(ResourcesContextWrapperFactory.get(ctx));
// Find the package ID for the package that WebView's resources come from.
// This will be the donor package if there is one, not our main package.
String resourcePackage = packageInfo.packageName;
if (packageInfo.applicationInfo.metaData != null) {
resourcePackage = packageInfo.applicationInfo.metaData.getString(
"com.android.webview.WebViewDonorPackage", resourcePackage);
}
int packageId = webViewDelegate.getPackageId(
ContextUtils.getApplicationContext().getResources(), resourcePackage);
mAwInit.setUpResourcesOnBackgroundThread( mAwInit.setUpResourcesOnBackgroundThread(
packageInfo, ContextUtils.getApplicationContext()); packageId, ContextUtils.getApplicationContext());
try (ScopedSysTraceEvent e2 = ScopedSysTraceEvent.scoped( try (ScopedSysTraceEvent e2 = ScopedSysTraceEvent.scoped(
"WebViewChromiumFactoryProvider.initCommandLine")) { "WebViewChromiumFactoryProvider.initCommandLine")) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment