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

Commit 079f8961 authored by Felipe Leme's avatar Felipe Leme
Browse files

Remove initial screenshot on interactive bugreport. 

One of the changes in the 'interactive bugreport' bugreport workflow
introduced on N is that the initial screenshot was taken right away (by
Shell, not dumpstate).

Unfortunately, such initial screenshot is often delayed when the system
is overload. Also, if the user is not interested in a screenshot, it
would be adding more load on the system unnecessarily.

Given these issues, and the fact that the user can still easily take an
initial screenhsot by selecting the proper notification action, the
initial screenshot is being removed.

BUG: 28167977

Change-Id: I2cf6616ce3124102b62ec9a36dc5a0ce6455a909
parent 067f821e

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

+14 −28
Original line number Diff line number Diff line
@@ -336,7 +336,7 @@ public class BugreportProgressService extends Service { 
                    launchBugreportInfoDialog(id);

                    break;

                case INTENT_BUGREPORT_SCREENSHOT:

                    takeScreenshot(id, true);

                    takeScreenshot(id);

                    break;

                case INTENT_BUGREPORT_SHARE:

                    shareBugreport(id, (BugreportInfo) intent.getParcelableExtra(EXTRA_INFO));
@@ -417,8 +417,6 @@ public class BugreportProgressService extends Service { 
            return true;

        }

        mProcesses.put(info.id, info);

        // Take initial screenshot.

        takeScreenshot(id, false);

        updateProgress(info);

        return true;

    }
@@ -635,19 +633,11 @@ public class BugreportProgressService extends Service { 
    /**

     * Starting point for taking a screenshot.

     * <p>

     * If {@code delayed} is set, it first display a toast message and waits

     * {@link #SCREENSHOT_DELAY_SECONDS} seconds before taking it, otherwise it takes the screenshot

     * right away.

     * <p>

     * Typical usage is delaying when taken from the notification action, and taking it right away

     * upon receiving a {@link #INTENT_BUGREPORT_STARTED}.

     * It first display a toast message and waits {@link #SCREENSHOT_DELAY_SECONDS} seconds before

     * taking the screenshot.

     */

    private void takeScreenshot(int id, boolean delayed) {

        if (delayed) {

            // Only logs screenshots requested from the notification action.

            MetricsLogger.action(this,

                    MetricsEvent.ACTION_BUGREPORT_NOTIFICATION_ACTION_SCREENSHOT);

        }

    private void takeScreenshot(int id) {

        MetricsLogger.action(this, MetricsEvent.ACTION_BUGREPORT_NOTIFICATION_ACTION_SCREENSHOT);

        if (getInfo(id) == null) {

            // Most likely am killed Shell before user tapped the notification. Since system might

            // be too busy anwyays, it's better to ignore the notification and switch back to the
@@ -659,7 +649,6 @@ public class BugreportProgressService extends Service { 
            return;

        }

        setTakingScreenshot(true);

        if (delayed) {

        collapseNotificationBar();

        final String msg = mContext.getResources()

                .getQuantityString(com.android.internal.R.plurals.bugreport_countdown,
@@ -669,9 +658,6 @@ public class BugreportProgressService extends Service { 
        Toast.makeText(mContext, msg, Toast.LENGTH_SHORT).show();



        takeScreenshot(id, SCREENSHOT_DELAY_SECONDS);

        } else {

            takeScreenshot(id, 0);

        }

    }



    /**

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

+9 −7
Original line number Diff line number Diff line
@@ -216,7 +216,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
        Bundle extras =

                sendBugreportFinishedAndGetSharedIntent(ID, mPlainTextPath, mScreenshotPath);

        assertActionSendMultiple(extras, BUGREPORT_CONTENT, SCREENSHOT_CONTENT, ID, PID, ZIP_FILE,

                NAME, NO_TITLE, NO_DESCRIPTION, 1, RENAMED_SCREENSHOTS);

                NAME, NO_TITLE, NO_DESCRIPTION, 0, RENAMED_SCREENSHOTS);



        assertServiceNotRunning();

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


        Bundle extras = acceptBugreportAndGetSharedIntent(ID);

        assertActionSendMultiple(extras, BUGREPORT_CONTENT, SCREENSHOT_CONTENT, ID, PID, ZIP_FILE,

                NAME, NO_TITLE, NO_DESCRIPTION, 2, RENAMED_SCREENSHOTS);

                NAME, NO_TITLE, NO_DESCRIPTION, 1, RENAMED_SCREENSHOTS);



        assertServiceNotRunning();

    }
@@ -283,6 +283,8 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
        resetProperties();



        sendBugreportStarted(1000);

        waitForScreenshotButtonEnabled(true);

        takeScreenshot();

        sendBugreportFinished(ID, mPlainTextPath, NO_SCREENSHOT);

        waitShareNotification(ID);
@@ -340,7 +342,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
        Bundle extras = sendBugreportFinishedAndGetSharedIntent(ID, mPlainTextPath,

                mScreenshotPath);

        assertActionSendMultiple(extras, BUGREPORT_CONTENT, SCREENSHOT_CONTENT, ID, PID, TITLE,

                NEW_NAME, TITLE, mDescription, 1, RENAMED_SCREENSHOTS);

                NEW_NAME, TITLE, mDescription, 0, RENAMED_SCREENSHOTS);



        assertServiceNotRunning();

    }
@@ -377,7 +379,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
        Bundle extras = sendBugreportFinishedAndGetSharedIntent(ID,

                plainText? mPlainTextPath : mZipPath, mScreenshotPath);

        assertActionSendMultiple(extras, BUGREPORT_CONTENT, SCREENSHOT_CONTENT, ID, PID, TITLE,

                NEW_NAME, TITLE, mDescription, 1, RENAMED_SCREENSHOTS);

                NEW_NAME, TITLE, mDescription, 0, RENAMED_SCREENSHOTS);



        assertServiceNotRunning();

    }
@@ -404,7 +406,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 


        Bundle extras = sendBugreportFinishedAndGetSharedIntent(ID, mZipPath, mScreenshotPath);

        assertActionSendMultiple(extras, BUGREPORT_CONTENT, SCREENSHOT_CONTENT, ID, PID, ZIP_FILE,

                NO_NAME, NO_TITLE, mDescription, 1, DIDNT_RENAME_SCREENSHOTS);

                NO_NAME, NO_TITLE, mDescription, 0, DIDNT_RENAME_SCREENSHOTS);



        assertServiceNotRunning();

    }
@@ -449,7 +451,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
        // title.txt and description.txt entries.

        extras = sendBugreportFinishedAndGetSharedIntent(ID2, mZipPath2, NO_SCREENSHOT);

        assertActionSendMultiple(extras, BUGREPORT_CONTENT, NO_SCREENSHOT, ID2, PID2, TITLE2,

                NEW_NAME2, TITLE2, DESCRIPTION2, 1, RENAMED_SCREENSHOTS);

                NEW_NAME2, TITLE2, DESCRIPTION2, 0, RENAMED_SCREENSHOTS);



        assertServiceNotRunning();

    }
@@ -500,7 +502,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
        // Finally, share bugreport.

        Bundle extras = acceptBugreportAndGetSharedIntent(ID);

        assertActionSendMultiple(extras, BUGREPORT_CONTENT, SCREENSHOT_CONTENT, ID, PID, TITLE,

                NAME, TITLE, mDescription, 1, RENAMED_SCREENSHOTS);

                NAME, TITLE, mDescription, 0, RENAMED_SCREENSHOTS);



        assertServiceNotRunning();

    }