Merge "HeadsUpViewBinder: Don't WTF if notif stops alerting after cleanup." into tm-qpr-dev
This commit is contained in:
committed by
Android (Google) Code Review
commit
cc94b4c9f9
@@ -114,7 +114,18 @@ public class HeadsUpViewBinder {
|
||||
*/
|
||||
public void unbindHeadsUpView(NotificationEntry entry) {
|
||||
abortBindCallback(entry);
|
||||
mStage.getStageParams(entry).markContentViewsFreeable(FLAG_CONTENT_VIEW_HEADS_UP);
|
||||
|
||||
// params may be null if the notification was already removed from the collection but we let
|
||||
// it stick around during a launch animation. In this case, the heads up view has already
|
||||
// been unbound, so we don't need to unbind it.
|
||||
// TODO(b/253081345): Change this back to getStageParams and remove null check.
|
||||
RowContentBindParams params = mStage.tryGetStageParams(entry);
|
||||
if (params == null) {
|
||||
mLogger.entryBindStageParamsNullOnUnbind(entry);
|
||||
return;
|
||||
}
|
||||
|
||||
params.markContentViewsFreeable(FLAG_CONTENT_VIEW_HEADS_UP);
|
||||
mLogger.entryContentViewMarkedFreeable(entry);
|
||||
mStage.requestRebind(entry, e -> mLogger.entryUnbound(e));
|
||||
}
|
||||
|
||||
@@ -47,6 +47,14 @@ class HeadsUpViewBinderLogger @Inject constructor(@NotificationHeadsUpLog val bu
|
||||
"start unbinding heads up entry $str1 "
|
||||
})
|
||||
}
|
||||
|
||||
fun entryBindStageParamsNullOnUnbind(entry: NotificationEntry) {
|
||||
buffer.log(TAG, INFO, {
|
||||
str1 = entry.logKey
|
||||
}, {
|
||||
"heads up entry bind stage params null on unbind $str1 "
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
private const val TAG = "HeadsUpViewBinder"
|
||||
|
||||
@@ -21,6 +21,7 @@ import android.util.ArrayMap;
|
||||
import android.util.Log;
|
||||
|
||||
import androidx.annotation.NonNull;
|
||||
import androidx.annotation.Nullable;
|
||||
|
||||
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
|
||||
|
||||
@@ -64,7 +65,7 @@ public abstract class BindStage<Params> extends BindRequester {
|
||||
* Get the stage parameters for the entry. Clients should use this to modify how the stage
|
||||
* handles the notification content.
|
||||
*/
|
||||
public final Params getStageParams(@NonNull NotificationEntry entry) {
|
||||
public final @NonNull Params getStageParams(@NonNull NotificationEntry entry) {
|
||||
Params params = mContentParams.get(entry);
|
||||
if (params == null) {
|
||||
// TODO: This should throw an exception but there are some cases of re-entrant calls
|
||||
@@ -79,6 +80,17 @@ public abstract class BindStage<Params> extends BindRequester {
|
||||
return params;
|
||||
}
|
||||
|
||||
// TODO(b/253081345): Remove this method.
|
||||
/**
|
||||
* Get the stage parameters for the entry, or null if there are no stage parameters for the
|
||||
* entry.
|
||||
*
|
||||
* @see #getStageParams(NotificationEntry)
|
||||
*/
|
||||
public final @Nullable Params tryGetStageParams(@NonNull NotificationEntry entry) {
|
||||
return mContentParams.get(entry);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a params entry for the notification for this stage.
|
||||
*/
|
||||
|
||||
@@ -91,6 +91,8 @@ public class HeadsUpViewBinderTest extends SysuiTestCase {
|
||||
verifyNoMoreInteractions(mLogger);
|
||||
clearInvocations(mLogger);
|
||||
|
||||
when(mBindStage.tryGetStageParams(eq(mEntry))).thenReturn(new RowContentBindParams());
|
||||
|
||||
mViewBinder.unbindHeadsUpView(mEntry);
|
||||
verify(mLogger).entryContentViewMarkedFreeable(eq(mEntry));
|
||||
verifyNoMoreInteractions(mLogger);
|
||||
@@ -139,6 +141,8 @@ public class HeadsUpViewBinderTest extends SysuiTestCase {
|
||||
verifyNoMoreInteractions(mLogger);
|
||||
clearInvocations(mLogger);
|
||||
|
||||
when(mBindStage.tryGetStageParams(eq(mEntry))).thenReturn(new RowContentBindParams());
|
||||
|
||||
mViewBinder.unbindHeadsUpView(mEntry);
|
||||
verify(mLogger).currentOngoingBindingAborted(eq(mEntry));
|
||||
verify(mLogger).entryContentViewMarkedFreeable(eq(mEntry));
|
||||
@@ -150,4 +154,30 @@ public class HeadsUpViewBinderTest extends SysuiTestCase {
|
||||
verifyNoMoreInteractions(mLogger);
|
||||
clearInvocations(mLogger);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testLoggingForLateUnbindFlow() {
|
||||
AtomicReference<NotifBindPipeline.BindCallback> callback = new AtomicReference<>();
|
||||
when(mBindStage.requestRebind(any(), any())).then(i -> {
|
||||
callback.set(i.getArgument(1));
|
||||
return new CancellationSignal();
|
||||
});
|
||||
|
||||
mViewBinder.bindHeadsUpView(mEntry, null);
|
||||
verify(mLogger).startBindingHun(eq(mEntry));
|
||||
verifyNoMoreInteractions(mLogger);
|
||||
clearInvocations(mLogger);
|
||||
|
||||
callback.get().onBindFinished(mEntry);
|
||||
verify(mLogger).entryBoundSuccessfully(eq(mEntry));
|
||||
verifyNoMoreInteractions(mLogger);
|
||||
clearInvocations(mLogger);
|
||||
|
||||
when(mBindStage.tryGetStageParams(eq(mEntry))).thenReturn(null);
|
||||
|
||||
mViewBinder.unbindHeadsUpView(mEntry);
|
||||
verify(mLogger).entryBindStageParamsNullOnUnbind(eq(mEntry));
|
||||
verifyNoMoreInteractions(mLogger);
|
||||
clearInvocations(mLogger);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -21,6 +21,10 @@ import static com.android.systemui.statusbar.notification.row.NotificationRowCon
|
||||
import static com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_EXPANDED;
|
||||
import static com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_HEADS_UP;
|
||||
|
||||
import static junit.framework.Assert.assertEquals;
|
||||
import static junit.framework.Assert.assertNotNull;
|
||||
import static junit.framework.Assert.assertNotSame;
|
||||
import static junit.framework.Assert.assertNull;
|
||||
import static junit.framework.Assert.assertTrue;
|
||||
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
@@ -31,6 +35,7 @@ import static org.mockito.Mockito.verify;
|
||||
|
||||
import android.testing.AndroidTestingRunner;
|
||||
import android.testing.TestableLooper;
|
||||
import android.util.Log;
|
||||
|
||||
import androidx.test.filters.SmallTest;
|
||||
|
||||
@@ -100,6 +105,67 @@ public class RowContentBindStageTest extends SysuiTestCase {
|
||||
verify(mBinder).unbindContent(eq(mEntry), any(), eq(flags));
|
||||
}
|
||||
|
||||
class CountingWtfHandler implements Log.TerribleFailureHandler {
|
||||
private Log.TerribleFailureHandler mOldHandler = null;
|
||||
private int mWtfCount = 0;
|
||||
|
||||
public void register() {
|
||||
mOldHandler = Log.setWtfHandler(this);
|
||||
}
|
||||
|
||||
public void unregister() {
|
||||
Log.setWtfHandler(mOldHandler);
|
||||
mOldHandler = null;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onTerribleFailure(String tag, Log.TerribleFailure what, boolean system) {
|
||||
mWtfCount++;
|
||||
}
|
||||
|
||||
public int getWtfCount() {
|
||||
return mWtfCount;
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testGetStageParamsAfterCleanUp() {
|
||||
// GIVEN an entry whose params have already been deleted.
|
||||
RowContentBindParams originalParams = mRowContentBindStage.getStageParams(mEntry);
|
||||
mRowContentBindStage.deleteStageParams(mEntry);
|
||||
|
||||
// WHEN a caller calls getStageParams.
|
||||
CountingWtfHandler countingWtfHandler = new CountingWtfHandler();
|
||||
countingWtfHandler.register();
|
||||
|
||||
RowContentBindParams blankParams = mRowContentBindStage.getStageParams(mEntry);
|
||||
|
||||
countingWtfHandler.unregister();
|
||||
|
||||
// THEN getStageParams logs a WTF and returns blank params created to avoid a crash.
|
||||
assertEquals(1, countingWtfHandler.getWtfCount());
|
||||
assertNotNull(blankParams);
|
||||
assertNotSame(originalParams, blankParams);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testTryGetStageParamsAfterCleanUp() {
|
||||
// GIVEN an entry whose params have already been deleted.
|
||||
mRowContentBindStage.deleteStageParams(mEntry);
|
||||
|
||||
// WHEN a caller calls getStageParams.
|
||||
CountingWtfHandler countingWtfHandler = new CountingWtfHandler();
|
||||
countingWtfHandler.register();
|
||||
|
||||
RowContentBindParams nullParams = mRowContentBindStage.tryGetStageParams(mEntry);
|
||||
|
||||
countingWtfHandler.unregister();
|
||||
|
||||
// THEN getStageParams does NOT log a WTF and returns null to indicate missing params.
|
||||
assertEquals(0, countingWtfHandler.getWtfCount());
|
||||
assertNull(nullParams);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testRebindAllContentViews() {
|
||||
// GIVEN a view with content bound.
|
||||
|
||||
Reference in New Issue
Block a user