From b2d6ed3cd125d616ab44b89e36af34a52756c47c Mon Sep 17 00:00:00 2001 From: Geoffrey Pitsch Date: Wed, 24 Aug 2016 10:37:19 -0400 Subject: Prevent notifyScreenshotError() from calling twice Found this while investigating the attached bug. Fixes potential race condition between onServiceDisconnected and mScreenshotTimeout both calling notifyScreenshotError. If onServiceDisconnected is going to show an error to the user, I think it should also unbind, as we would expect the user to re-take the shot at that point. Ideally, this would replace the timeout handler entirely. Not exactly sure how to test this - I tried crashing the TakeScreenshotService locally, but onServiceDisconnected didn't trigger. Bug: 28982719 Change-Id: I5093c61735b0661f9874c686e197cd894c3ef7ae --- .../java/com/android/server/policy/PhoneWindowManager.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/policy/PhoneWindowManager.java b/services/core/java/com/android/server/policy/PhoneWindowManager.java index a39add80ba8f..4cc46370afe1 100644 --- a/services/core/java/com/android/server/policy/PhoneWindowManager.java +++ b/services/core/java/com/android/server/policy/PhoneWindowManager.java @@ -5511,7 +5511,14 @@ public class PhoneWindowManager implements WindowManagerPolicy { @Override public void onServiceDisconnected(ComponentName name) { - notifyScreenshotError(); + synchronized (mScreenshotLock) { + if (mScreenshotConnection != null) { + mContext.unbindService(mScreenshotConnection); + mScreenshotConnection = null; + mHandler.removeCallbacks(mScreenshotTimeout); + notifyScreenshotError(); + } + } } }; if (mContext.bindServiceAsUser(serviceIntent, conn, @@ -7748,7 +7755,7 @@ public class PhoneWindowManager implements WindowManagerPolicy { int delta = newRotation - oldRotation; if (delta < 0) delta += 4; // Likewise we don't rotate seamlessly for 180 degree rotations - // in this case the surfaces never resize, and our logic to + // in this case the surfaces never resize, and our logic to // revert the transformations on size change will fail. We could // fix this in the future with the "tagged" frames idea. if (delta == Surface.ROTATION_180) { -- cgit v1.2.3-59-g8ed1b