Commit a9a76fcd authored by Gustav Sennton's avatar Gustav Sennton Committed by Commit Bot

Fix deadlock during WebView initialization.

We cannot hold onto a lock that could potentially block the UI thread,
while calling into any getter in WebViewChromiumAwInit related to
chromium initialization. This is because those getters
1. hold a lock,
2. post a chromium-initialization task to the UI thread, and then
3. wait for the task to finish after releasing the lock in 1
So if thread X holds another lock, and calls a WebViewChromiumAwInit
getter, a deadlock occurs if the UI thread tries to hold onto that other
lock before running the chromium-initialization task.

We fix this issue by providing a getLock() method on
WebViewChromiumAwInit for WebViewChromiumFactoryProvider to re-use the
existing lock to guard its member variables (instead of using another
lock).

Bug: 812203
Change-Id: I2e8d69f2667bce1e4312cc821fc804d44b6deaba
Reviewed-on: https://chromium-review.googlesource.com/922823
Commit-Queue: Gustav Sennton <gsennton@chromium.org>
Reviewed-by: default avatarBo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537298}
parent 55f740be
......@@ -264,6 +264,15 @@ class WebViewChromiumAwInit {
return mBrowserContext;
}
/**
* Returns the lock used for guarding chromium initialization.
* We make this package-public to let higher-level classes use this lock to guard variables
* dependent on this class, to avoid introducing new locks (which can cause deadlocks).
*/
Object getLock() {
return mLock;
}
public SharedStatics getStatics() {
synchronized (mLock) {
if (mSharedStatics == null) {
......
......@@ -115,9 +115,10 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider {
boolean mShouldDisableThreadChecking;
// Initialization guarded by mAdapterLock
// Initialization guarded by mAwInit.getLock()
private Statics mStaticsAdapter;
// TODO(gsennton) remove this when downstream doesn't depend on it anymore
// Guards accees to adapters.
// This member is not private only because the downstream subclass needs to access it,
// it shouldn't be accessed from anywhere else.
......@@ -315,9 +316,9 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider {
@Override
public Statics getStatics() {
synchronized (mAdapterLock) {
synchronized (mAwInit.getLock()) {
SharedStatics sharedStatics = mAwInit.getStatics();
if (mStaticsAdapter == null) {
SharedStatics sharedStatics = mAwInit.getStatics();
mStaticsAdapter = new WebViewChromiumFactoryProvider.Statics() {
@Override
public String findAddress(String addr) {
......
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