Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 47c59ce6 authored by Samer Naoura's avatar Samer Naoura Committed by Mehmet Murat Sevim
Browse files

BugreportManager takes screenshot even though screenshotFd is null 

Bug: 385709501
Fix: 385709501
Flag: android.os.bugreport_deferred_consent_screenshot_fix
Test: tested on a device
(cherry picked from https://partner-android-review.googlesource.com/q/commit:1de4dc9d0db76cf6f914e38af0a7af91cda58cdc)
Merged-In: Ib1b90701312096ecdacad91b4d9e44ca184a12db
Change-Id: Ib1b90701312096ecdacad91b4d9e44ca184a12db
parent f88743fc

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
@@ -411,4 +411,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,