Merge "BugreportManager takes screenshot even though screenshotFd is null" into main (57cb264e) · Commits · e / os / android_frameworks_base

core/java/android/os/BugreportManager.java

+6 −2

Original line number	Diff line number	Diff line
		@@ -160,7 +160,6 @@ public final class BugreportManager {
		*
		* @param bugreportFile the absolute path of the generated bugreport file.
		* @hide

		*/
		@SystemApi
		public void onFinished(@NonNull String bugreportFile) {}
		@@ -234,7 +233,12 @@ public final class BugreportManager {

		boolean deferConsent =
		(params.getFlags() & BugreportParams.BUGREPORT_FLAG_DEFER_CONSENT) != 0;
		boolean isScreenshotRequested = screenshotFd != null \|\| deferConsent;
		boolean isScreenshotRequested;
		if (Flags.bugreportDeferredConsentScreenshotFix()) {
		isScreenshotRequested = screenshotFd != null;
		} else {
		isScreenshotRequested = screenshotFd != null \|\| deferConsent;
		}
		if (screenshotFd == null) {
		// Binder needs a valid File Descriptor to be passed
		screenshotFd =

core/java/android/os/flags.aconfig

+7 −0

Original line number	Diff line number	Diff line
		@@ -425,4 +425,11 @@ flag {
		bug: "324241334"
		}

		flag {
		namespace: "wear_services"
		name: "bugreport_deferred_consent_screenshot_fix"
		description: "Stops always taking a screenshot when consent is deferred."
		bug: "385709501"
		}

		# keep-sorted end

core/tests/bugreports/Android.bp

+2 −0

Original line number	Diff line number	Diff line
		@@ -33,6 +33,8 @@ android_test {
		"android.tracing.flags-aconfig-java",
		"androidx.test.rules",
		"androidx.test.uiautomator_uiautomator",
		"android.os.flags-aconfig-java",
		"flag-junit",
		"truth",
		],
		test_suites: ["general-tests"],

core/tests/bugreports/src/com/android/os/bugreports/tests/BugreportManagerTest.java

+70 −1

Original line number	Diff line number	Diff line
		@@ -38,6 +38,8 @@ import android.os.HandlerThread;
		import android.os.ParcelFileDescriptor;
		import android.os.Process;
		import android.os.StrictMode;
		import android.platform.test.annotations.DisableFlags;
		import android.platform.test.annotations.EnableFlags;
		import android.util.Log;

		import androidx.annotation.NonNull;
		@@ -295,6 +297,58 @@ public class BugreportManagerTest {
		assertThatBugreportContainsFiles(mUiTracesPreDumped);
		}

		@Test
		@LargeTest
		@EnableFlags(android.os.Flags.FLAG_BUGREPORT_DEFERRED_CONSENT_SCREENSHOT_FIX)
		public void deferredConsentFlow_screenshotFixEnabled() throws Exception {
		// Uses a placeholder file descriptor for the startBugreport call. This file is unused as
		// startBugreport does not copy the bugreport content if the consent is deferred.
		File placeholderFile = createTempFile("placeholder_file", ".zip");
		ParcelFileDescriptor placeholderFd = parcelFd(placeholderFile);
		BugreportCallbackImpl callback = new BugreportCallbackImpl();
		mBrm.startBugreport(placeholderFd, null /screenshotFd = null/, fullWithDeferredConsent(),
		mExecutor, callback);

		waitTillDoneOrTimeout(callback);
		assertThat(callback.isDone()).isTrue();
		String bugreportFile = callback.mBugreportFile;
		callback = new BugreportCallbackImpl();

		mBrm.retrieveBugreport(bugreportFile, mBugreportFd, mExecutor, callback);
		shareConsentDialog(ConsentReply.ALLOW);
		waitTillDoneOrTimeout(callback);

		assertThat(placeholderFile.length()).isEqualTo(0L);
		assertThat(mBugreportFile.length()).isGreaterThan(0L);
		assertFdsAreClosed(placeholderFd, mBugreportFd);
		}

		@Test
		@LargeTest
		@DisableFlags(android.os.Flags.FLAG_BUGREPORT_DEFERRED_CONSENT_SCREENSHOT_FIX)
		public void deferredConsentFlow_screenshotFixDisabled() throws Exception {
		// Uses a placeholder file descriptor for the startBugreport call. This file is unused as
		// startBugreport does not copy the bugreport content if the consent is deferred.
		File placeholderFile = createTempFile("placeholder_file", ".zip");
		ParcelFileDescriptor placeholderFd = parcelFd(placeholderFile);
		BugreportCallbackImpl callback = new BugreportCallbackImpl();
		mBrm.startBugreport(placeholderFd, null /screenshotFd = null/, fullWithDeferredConsent(),
		mExecutor, callback);

		waitTillDoneOrTimeout(callback);
		assertThat(callback.isDone()).isTrue();
		String bugreportFile = callback.mBugreportFile;

		callback = new BugreportCallbackImpl();
		mBrm.retrieveBugreport(bugreportFile, mBugreportFd, mExecutor, callback);
		shareConsentDialog(ConsentReply.ALLOW);
		waitTillDoneOrTimeout(callback);

		assertThat(placeholderFile.length()).isEqualTo(0L);
		assertThat(mBugreportFile.length()).isGreaterThan(0L);
		assertFdsAreClosed(placeholderFd, mBugreportFd);
		}

		@Test
		public void simultaneousBugreportsNotAllowed() throws Exception {
		// Start bugreport #1
		@@ -435,6 +489,7 @@ public class BugreportManagerTest {
		private boolean mReceivedProgress = false;
		private boolean mEarlyReportFinished = false;
		private final Object mLock = new Object();
		private String mBugreportFile;

		@Override
		public void onProgress(float progress) {
		@@ -459,6 +514,15 @@ public class BugreportManagerTest {
		}
		}

		@Override
		public void onFinished(@NonNull String bugreportFile) {
		synchronized (mLock) {
		Log.d(TAG, "consent defer bugreport finished bugreportFile=" + bugreportFile);
		mSuccess = true;
		mBugreportFile = bugreportFile;
		}
		}

		@Override
		public void onEarlyReportFinished() {
		synchronized (mLock) {
		@@ -804,6 +868,11 @@ public class BugreportManagerTest {
		BugreportParams.BUGREPORT_FLAG_USE_PREDUMPED_UI_DATA);
		}

		private static BugreportParams fullWithDeferredConsent() {
		return new BugreportParams(BugreportParams.BUGREPORT_MODE_FULL,
		BugreportParams.BUGREPORT_FLAG_DEFER_CONSENT);
		}

		/* Allow/deny the consent dialog to sharing bugreport data or check existence only. */
		private enum ConsentReply {
		ALLOW,