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

Commit 2288129d authored by Felipe Leme's avatar Felipe Leme
Browse files

Fixed corner-case scenario where a screenshot is finished after the share 

notification is sent.

Prior to this change, if a screenshot finished after the share
notification was sent, it would replace the share notification with a
progress notification, and the share notification would never be sent
again.

Also improved the test cases that automatically generate a screenshot
but don't use it to wait for the screenshot to finish before proceeding,
otherwise it could cause a future test to fail (if the screenshot is
finished after the initial test is completed).

Change-Id: I6e2a6549ebb48e5bebf5aa78d1bda94404c1812b
parent 4bd4f40f

packages/Shell/src/com/android/shell/BugreportProgressService.java

+11 −1
Original line number Original line Diff line number Diff line
@@ -449,6 +449,11 @@ public class BugreportProgressService extends Service { 
                .addAction(cancelAction)

                .addAction(cancelAction)

                .build();

                .build();





        if (info.finished) {

            Log.w(TAG, "Not sending progress notification because bugreport has finished already ("

                    + info + ")");

            return;

        }

        NotificationManager.from(mContext).notify(TAG, info.pid, notification);

        NotificationManager.from(mContext).notify(TAG, info.pid, notification);

    }

    }
@@ -628,7 +633,12 @@ public class BugreportProgressService extends Service { 
        synchronized (BugreportProgressService.this) {

        synchronized (BugreportProgressService.this) {

            mTakingScreenshot = flag;

            mTakingScreenshot = flag;

            for (int i = 0; i < mProcesses.size(); i++) {

            for (int i = 0; i < mProcesses.size(); i++) {

                updateProgress(mProcesses.valueAt(i));

                final BugreportInfo info = mProcesses.valueAt(i);

                if (info.finished) {

                    Log.d(TAG, "Not updating progress because share notification was already sent");

                    continue;

                }

                updateProgress(info);

            }

            }

        }

        }

    }

    }

packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java

+31 −9
Original line number Original line Diff line number Diff line
@@ -142,6 +142,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
    public void testProgress() throws Exception {

    public void testProgress() throws Exception {

        resetProperties();

        resetProperties();

        sendBugreportStarted(1000);

        sendBugreportStarted(1000);

        waitForScreenshotButtonEnabled(true);





        assertProgressNotification(NAME, "0.00%");

        assertProgressNotification(NAME, "0.00%");
@@ -157,7 +158,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
        Bundle extras =

        Bundle extras =

                sendBugreportFinishedAndGetSharedIntent(PID, mPlainTextPath, mScreenshotPath);

                sendBugreportFinishedAndGetSharedIntent(PID, mPlainTextPath, mScreenshotPath);

        assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, NAME, ZIP_FILE,

        assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, NAME, ZIP_FILE,

                null, 1);

                null, 1, true);





        assertServiceNotRunning();

        assertServiceNotRunning();

    }

    }
@@ -174,7 +175,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
        Bundle extras =

        Bundle extras =

                sendBugreportFinishedAndGetSharedIntent(PID, mPlainTextPath, mScreenshotPath);

                sendBugreportFinishedAndGetSharedIntent(PID, mPlainTextPath, mScreenshotPath);

        assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, NAME, ZIP_FILE,

        assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, NAME, ZIP_FILE,

                null, 2);

                null, 2, true);





        assertServiceNotRunning();

        assertServiceNotRunning();

    }

    }
@@ -182,6 +183,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
    public void testProgress_changeDetails() throws Exception {

    public void testProgress_changeDetails() throws Exception {

        resetProperties();

        resetProperties();

        sendBugreportStarted(1000);

        sendBugreportStarted(1000);

        waitForScreenshotButtonEnabled(true);





        DetailsUi detailsUi = new DetailsUi(mUiBot);

        DetailsUi detailsUi = new DetailsUi(mUiBot);
@@ -218,14 +220,33 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
        Bundle extras = sendBugreportFinishedAndGetSharedIntent(PID, mPlainTextPath,

        Bundle extras = sendBugreportFinishedAndGetSharedIntent(PID, mPlainTextPath,

                mScreenshotPath);

                mScreenshotPath);

        assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, NEW_NAME, TITLE,

        assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, NEW_NAME, TITLE,

                mDescription, 1);

                mDescription, 1, true);





        assertServiceNotRunning();

        assertServiceNotRunning();

    }

    }





    /**

     * Tests the scenario where the initial screenshot and dumpstate are finished while the user

     * is changing the info in the details screen.

     */

    public void testProgress_bugreportAndScreenshotFinishedWhileChangingDetails() throws Exception {

        bugreportFinishedWhileChangingDetailsTest(false);

    }



    /**

     * Tests the scenario where dumpstate is finished while the user is changing the info in the

     * details screen, but the initial screenshot finishes afterwards.

     */

    public void testProgress_bugreportFinishedWhileChangingDetails() throws Exception {

    public void testProgress_bugreportFinishedWhileChangingDetails() throws Exception {

        bugreportFinishedWhileChangingDetailsTest(true);

    }



    private void bugreportFinishedWhileChangingDetailsTest(boolean waitScreenshot) throws Exception {

        resetProperties();

        resetProperties();

        sendBugreportStarted(1000);

        sendBugreportStarted(1000);

        if (waitScreenshot) {

            waitForScreenshotButtonEnabled(true);

        }





        DetailsUi detailsUi = new DetailsUi(mUiBot);

        DetailsUi detailsUi = new DetailsUi(mUiBot);
@@ -233,7 +254,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
        detailsUi.nameField.setText(NEW_NAME);

        detailsUi.nameField.setText(NEW_NAME);

        sendBugreportFinished(PID, mPlainTextPath, mScreenshotPath);

        sendBugreportFinished(PID, mPlainTextPath, mScreenshotPath);





        // Wait until the share notifcation is received...

        // Wait until the share notification is received...

        mUiBot.getNotification(mContext.getString(R.string.bugreport_finished_title));

        mUiBot.getNotification(mContext.getString(R.string.bugreport_finished_title));

        // ...then close notification bar.

        // ...then close notification bar.

        mContext.sendBroadcast(new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS));

        mContext.sendBroadcast(new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS));
@@ -250,7 +271,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
        // Finally, share bugreport.

        // Finally, share bugreport.

        Bundle extras = acceptBugreportAndGetSharedIntent();

        Bundle extras = acceptBugreportAndGetSharedIntent();

        assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, NAME, TITLE,

        assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, NAME, TITLE,

                mDescription, 1);

                mDescription, 1, waitScreenshot);





        assertServiceNotRunning();

        assertServiceNotRunning();

    }

    }
@@ -406,7 +427,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
    private void assertActionSendMultiple(Bundle extras, String bugreportContent,

    private void assertActionSendMultiple(Bundle extras, String bugreportContent,

            String screenshotContent) throws IOException {

            String screenshotContent) throws IOException {

        assertActionSendMultiple(extras, bugreportContent, screenshotContent, PID, null, ZIP_FILE,

        assertActionSendMultiple(extras, bugreportContent, screenshotContent, PID, null, ZIP_FILE,

                null, 0);

                null, 0, false);

    }

    }





    /**

    /**
@@ -420,10 +441,11 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
     * @param title bugreport name as provided by the user (or received by dumpstate)

     * @param title bugreport name as provided by the user (or received by dumpstate)

     * @param description bugreport description as provided by the user

     * @param description bugreport description as provided by the user

     * @param numberScreenshots expected number of screenshots taken by Shell.

     * @param numberScreenshots expected number of screenshots taken by Shell.

     * @param renamedScreenshots whether the screenshots are expected to be renamed

     */

     */

    private void assertActionSendMultiple(Bundle extras, String bugreportContent,

    private void assertActionSendMultiple(Bundle extras, String bugreportContent,

            String screenshotContent, int pid, String name, String title, String description,

            String screenshotContent, int pid, String name, String title, String description,

            int numberScreenshots) throws IOException {

            int numberScreenshots, boolean renamedScreenshots) throws IOException {

        String body = extras.getString(Intent.EXTRA_TEXT);

        String body = extras.getString(Intent.EXTRA_TEXT);

        assertContainsRegex("missing build info",

        assertContainsRegex("missing build info",

                SystemProperties.get("ro.build.description"), body);

                SystemProperties.get("ro.build.description"), body);
@@ -480,7 +502,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
        // Check internal screenshots.

        // Check internal screenshots.

        SortedSet<String> expectedNames = new TreeSet<>();

        SortedSet<String> expectedNames = new TreeSet<>();

        for (int i = 1 ; i <= numberScreenshots; i++) {

        for (int i = 1 ; i <= numberScreenshots; i++) {

            String prefix = name != null ? name : Integer.toString(pid);

            String prefix = renamedScreenshots  ? name : Integer.toString(pid);

            String expectedName = "screenshot-" + prefix + "-" + i + ".png";

            String expectedName = "screenshot-" + prefix + "-" + i + ".png";

            expectedNames.add(expectedName);

            expectedNames.add(expectedName);

        }

        }
@@ -592,7 +614,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 




    private UiObject waitForScreenshotButtonEnabled(boolean expectedEnabled) throws Exception {

    private UiObject waitForScreenshotButtonEnabled(boolean expectedEnabled) throws Exception {

        UiObject screenshotButton = getScreenshotButton();

        UiObject screenshotButton = getScreenshotButton();

        int maxAttempts = SCREENSHOT_DELAY_SECONDS + 2;

        int maxAttempts = SCREENSHOT_DELAY_SECONDS + 5;

        int i = 0;

        int i = 0;

        do {

        do {

            boolean enabled = screenshotButton.isEnabled();

            boolean enabled = screenshotButton.isEnabled();