Dialog denies access automatically when disappear (new test is added)
Originally, dialog does not deny access on “return home” and “lock screen” until timeout passes because nothing triggers a decline in log access action in onStop(). Therefore, onStop() is overridden in this change, which calls cancel() in Dialog. As a result, every time the dialog disappears, onDismiss() is guaranteed to be called and will trigger declineLogAccess() immediately. This also fixes the case where users lock the screen without clicking any button on the dialog.
Additionally, a new test for LogAccessDialogActivity is created and added to SystemUI/tests. LogAccessDialogActivityTest.java is an instrumented test which verifies the behavior of the activity when:
1. the state moves to onStop (whenever dialog disappears)
2. onClick() is called and view.getId() == R.id.log_access_dialog_allow_button
3. onClick() is called and view.getId() == R.id.log_access_dialog_deny_button
Some other small changes for improvement:
1. Original onClick() calls finish() at the end of every case, which is unnecessary. It’s better to not call finish() manually since it will be called whenever a dialog is in onDismiss() and the original onClick() will result in finish() being called unnecessarily repeatedly. Thus, mAlert.dismiss() is added into a “FINALLY” block in onClick().
2. Add “mAlert = null” in setOnDismissListener and delete some redundant settings of “mAlert = null” in other places.
3. onDestroy() is not needed anymore because it has mostly the same logic as onStop(). In addition, because mAlert is set to NULL whenever it enters into onDismiss(), it’s almost impossible to enter into the if statement block in the original onDestroy()
Test: atest SystemUITests:LogAccessDialogTest
Bug: 244005521
Change-Id: I62b7f0e5ebeb29d5a4f09b4cf42b1a7e0a235a62
diff --git a/packages/SystemUI/src/com/android/systemui/logcat/LogAccessDialogActivity.java b/packages/SystemUI/src/com/android/systemui/logcat/LogAccessDialogActivity.java
index 81de607..2cfe5a7 100644
--- a/packages/SystemUI/src/com/android/systemui/logcat/LogAccessDialogActivity.java
+++ b/packages/SystemUI/src/com/android/systemui/logcat/LogAccessDialogActivity.java
@@ -41,6 +41,7 @@
import android.widget.Button;
import android.widget.TextView;
+import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.app.ILogAccessDialogCallback;
import com.android.systemui.R;
@@ -69,7 +70,8 @@
private AlertDialog.Builder mAlertDialog;
private AlertDialog mAlert;
- private View mAlertView;
+ @VisibleForTesting
+ protected View mAlertView;
@Override
protected void onCreate(Bundle savedInstanceState) {
@@ -115,7 +117,10 @@
mAlertDialog = new AlertDialog.Builder(this, themeId);
mAlertDialog.setView(mAlertView);
mAlertDialog.setOnCancelListener(dialog -> declineLogAccess());
- mAlertDialog.setOnDismissListener(dialog -> finish());
+ mAlertDialog.setOnDismissListener(dialog -> {
+ mAlert = null;
+ finish();
+ });
// show Alert
mAlert = mAlertDialog.create();
@@ -127,12 +132,11 @@
}
@Override
- protected void onDestroy() {
- super.onDestroy();
- if (!isChangingConfigurations() && mAlert != null && mAlert.isShowing()) {
- mAlert.dismiss();
+ protected void onStop() {
+ super.onStop();
+ if (!isChangingConfigurations() && mAlert != null) {
+ mAlert.cancel();
}
- mAlert = null;
}
private boolean readIntentInfo(Intent intent) {
@@ -170,7 +174,6 @@
case MSG_DISMISS_DIALOG:
if (mAlert != null) {
mAlert.dismiss();
- mAlert = null;
declineLogAccess();
}
break;
@@ -181,7 +184,7 @@
}
};
- private String getTitleString(Context context, String callingPackage, int uid)
+ protected String getTitleString(Context context, String callingPackage, int uid)
throws NameNotFoundException {
PackageManager pm = context.getPackageManager();
@@ -238,13 +241,13 @@
try {
if (view.getId() == R.id.log_access_dialog_allow_button) {
mCallback.approveAccessForClient(mUid, mPackageName);
- finish();
} else if (view.getId() == R.id.log_access_dialog_deny_button) {
declineLogAccess();
- finish();
}
- } catch (RemoteException e) {
- finish();
+ } catch (RemoteException ignored) {
+ // Do nothing.
+ } finally {
+ mAlert.dismiss();
}
}
diff --git a/packages/SystemUI/tests/AndroidManifest.xml b/packages/SystemUI/tests/AndroidManifest.xml
index 12da17f..03fde9f 100644
--- a/packages/SystemUI/tests/AndroidManifest.xml
+++ b/packages/SystemUI/tests/AndroidManifest.xml
@@ -192,6 +192,12 @@
android:excludeFromRecents="true" />
<activity
+ android:name="com.android.systemui.logcat.LogAccessDialogActivityTest$DialogTestable"
+ android:exported="false"
+ android:permission="com.android.systemui.permission.SELF"
+ android:excludeFromRecents="true" />
+
+ <activity
android:name="com.android.systemui.notetask.LaunchNotesRoleSettingsTrampolineActivity"
android:exported="false"
android:permission="com.android.systemui.permission.SELF"
diff --git a/packages/SystemUI/tests/src/com/android/systemui/logcat/LogAccessDialogActivityTest.java b/packages/SystemUI/tests/src/com/android/systemui/logcat/LogAccessDialogActivityTest.java
new file mode 100644
index 0000000..1d7d5df
--- /dev/null
+++ b/packages/SystemUI/tests/src/com/android/systemui/logcat/LogAccessDialogActivityTest.java
@@ -0,0 +1,143 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.logcat;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import android.app.Activity;
+import android.content.Context;
+import android.content.Intent;
+import android.view.View;
+
+import androidx.test.core.app.ActivityScenario;
+import androidx.test.ext.junit.rules.ActivityScenarioRule;
+import androidx.test.filters.LargeTest;
+import androidx.test.platform.app.InstrumentationRegistry;
+
+import com.android.internal.app.ILogAccessDialogCallback;
+import com.android.systemui.R;
+import com.android.systemui.SysuiTestCase;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+@LargeTest
+public class LogAccessDialogActivityTest extends SysuiTestCase {
+
+ public static final String EXTRA_CALLBACK = "EXTRA_CALLBACK";
+ private final DialogCallbackTestable mDialogCallback = new DialogCallbackTestable();
+ @Rule
+ public ActivityScenarioRule<DialogTestable> mActivityScenarioRule =
+ new ActivityScenarioRule<>(getIntent());
+
+ static final class DialogCallbackTestable extends ILogAccessDialogCallback.Stub {
+
+ int mNumOfApprove = 0;
+ int mNumOfDecline = 0;
+ CountDownLatch mCountDownLatch = new CountDownLatch(1);
+
+ @Override
+ public void approveAccessForClient(int i, String s) {
+ mNumOfApprove++;
+ mCountDownLatch.countDown();
+ }
+
+ @Override
+ public void declineAccessForClient(int i, String s) {
+ mNumOfDecline++;
+ mCountDownLatch.countDown();
+ }
+ }
+
+ @Before
+ public void setUp() {
+ mDialogCallback.mNumOfDecline = 0;
+ mDialogCallback.mNumOfApprove = 0;
+ mDialogCallback.mCountDownLatch = new CountDownLatch(1);
+ }
+
+ private Intent getIntent() {
+ Context context = InstrumentationRegistry.getInstrumentation().getContext();
+ final Intent intent = new Intent(context, DialogTestable.class);
+
+ intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK | Intent.FLAG_ACTIVITY_CLEAR_TASK);
+ intent.putExtra(Intent.EXTRA_PACKAGE_NAME, "packageName");
+ intent.putExtra(Intent.EXTRA_UID, 1);
+
+ intent.putExtra(EXTRA_CALLBACK, mDialogCallback.asBinder());
+ intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
+
+ return intent;
+ }
+
+ @Test
+ public void test_dialogDisappear_withoutClick_autoDeclined() throws InterruptedException {
+ ActivityScenario<DialogTestable> activityScenario = mActivityScenarioRule.getScenario();
+ activityScenario.onActivity(Activity::finish);
+
+ assertTrue(mDialogCallback.mCountDownLatch.await(2, TimeUnit.SECONDS));
+ assertEquals(mDialogCallback.mNumOfDecline, 1);
+ assertEquals(mDialogCallback.mNumOfApprove, 0);
+
+ }
+
+ @Test
+ public void test_clickAllow() throws InterruptedException {
+ ActivityScenario<DialogTestable> activityScenario = mActivityScenarioRule.getScenario();
+
+ activityScenario.onActivity(activity -> {
+ View allowButton =
+ activity.mAlertView.findViewById(R.id.log_access_dialog_allow_button);
+ assertNotNull(allowButton);
+ allowButton.performClick();
+ });
+
+ assertTrue(mDialogCallback.mCountDownLatch.await(10, TimeUnit.SECONDS));
+ assertEquals(mDialogCallback.mNumOfDecline, 0);
+ assertEquals(mDialogCallback.mNumOfApprove, 1);
+ }
+
+ @Test
+ public void test_clickDeny() throws InterruptedException {
+ ActivityScenario<DialogTestable> activityScenario = mActivityScenarioRule.getScenario();
+
+ activityScenario.onActivity(activity -> {
+ View denyButton =
+ activity.mAlertView.findViewById(R.id.log_access_dialog_deny_button);
+ assertNotNull(denyButton);
+ denyButton.performClick();
+ });
+
+ assertTrue(mDialogCallback.mCountDownLatch.await(10, TimeUnit.SECONDS));
+ assertEquals(mDialogCallback.mNumOfDecline, 1);
+ assertEquals(mDialogCallback.mNumOfApprove, 0);
+ }
+
+ public static class DialogTestable extends LogAccessDialogActivity {
+
+ @Override
+ protected String getTitleString(Context context, String callingPackage, int uid) {
+ return "DialogTitle";
+ }
+ }
+}
