From b53670aecf2699022dffbf9400267ccf147eba64 Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Thu, 25 May 2017 18:20:49 -0700 Subject: [PATCH 1/2] SurfaceView: Avoid over-reporting DRAW_FINISHED. To some extent we rely on the app to call the draw callback if they want the app to work properly, but this case is fairly easy to detect, so why not prevent it and provide a helpful log. Bug: 62051758 Test: Warm start camera a few times. go/wm-smoke. Change-Id: I39f4e015bfa15a1e0c37dba70b4a700803a6a274 --- core/java/android/view/SurfaceView.java | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java index 435675ba01e57..b57ac66e10cc1 100644 --- a/core/java/android/view/SurfaceView.java +++ b/core/java/android/view/SurfaceView.java @@ -264,6 +264,22 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb updateSurface(); } + private void performDrawFinished() { + if (mPendingReportDraws > 0) { + mDrawFinished = true; + if (mAttachedToWindow) { + mParent.requestTransparentRegion(SurfaceView.this); + + notifyDrawFinished(); + invalidate(); + } + } else { + Log.e(TAG, System.identityHashCode(this) + "finished drawing" + + " but no pending report draw (extra call" + + " to draw completion runnable?)"); + } + } + void notifyDrawFinished() { ViewRootImpl viewRoot = getViewRootImpl(); if (viewRoot != null) { @@ -729,12 +745,7 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb } runOnUiThread(() -> { - mDrawFinished = true; - if (mAttachedToWindow) { - mParent.requestTransparentRegion(SurfaceView.this); - notifyDrawFinished(); - invalidate(); - } + performDrawFinished(); }); } From 49cd9f882c3853a615c517b00311a9512d68edf9 Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Thu, 25 May 2017 18:24:42 -0700 Subject: [PATCH 2/2] ViewRootImpl: More careful draw accounting. The strategy of beginning the count at 1 isn't enough and we need to explicitly debt each request to report draw. Imagine the case where a ViewRootImpl reports draw multiple times, while we are waiting for a SurfaceView draw completion callback. Without the explicit draw debt, the ViewRoot would be counted as drawing for the SurfaceView and we could notify the WM too early. Test: Open Chrome a few hundred times. go/wm-smoke. Bug: 38324871 Change-Id: I5762a98d1cc808125033ba3d1db8a3ea39a9e071 --- core/java/android/view/ViewRootImpl.java | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java index 28ded55c54fad..a720aae0d1be0 100644 --- a/core/java/android/view/ViewRootImpl.java +++ b/core/java/android/view/ViewRootImpl.java @@ -2330,7 +2330,7 @@ public final class ViewRootImpl implements ViewParent, // Remember if we must report the next draw. if ((relayoutResult & WindowManagerGlobal.RELAYOUT_RES_FIRST_TIME) != 0) { - mReportNextDraw = true; + reportNextDraw(); } boolean cancelDraw = mAttachInfo.mTreeObserver.dispatchOnPreDraw() || !isViewVisible; @@ -2731,11 +2731,8 @@ public final class ViewRootImpl implements ViewParent, /** * A count of the number of calls to pendingDrawFinished we * require to notify the WM drawing is complete. - * - * This starts at 1, for the ViewRootImpl surface itself. - * Subsurfaces may debt the value with drawPending. */ - int mDrawsNeededToReport = 1; + int mDrawsNeededToReport = 0; /** * Delay notifying WM of draw finished until @@ -2761,7 +2758,7 @@ public final class ViewRootImpl implements ViewParent, private void reportDrawFinished() { try { - mDrawsNeededToReport = 1; + mDrawsNeededToReport = 0; mWindowSession.finishDrawing(mWindow); } catch (RemoteException e) { // Have fun! @@ -3772,13 +3769,12 @@ public final class ViewRootImpl implements ViewParent, args.recycle(); if (msg.what == MSG_RESIZED_REPORT) { - mReportNextDraw = true; + reportNextDraw(); } if (mView != null && framesChanged) { forceLayout(mView); } - requestLayout(); } break; @@ -7343,6 +7339,14 @@ public final class ViewRootImpl implements ViewParent, return false; } + + private void reportNextDraw() { + if (mReportNextDraw == false) { + drawPending(); + } + mReportNextDraw = true; + } + /** * Force the window to report its next draw. *

@@ -7352,7 +7356,7 @@ public final class ViewRootImpl implements ViewParent, * @hide */ public void setReportNextDraw() { - mReportNextDraw = true; + reportNextDraw(); invalidate(); }