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

Commit 3fc44b9a authored by Felipe Leme's avatar Felipe Leme
Browse files

Changed logic when dumpstate's max progress increases. 

When dumpstate starts, it estimates its maximum duration and sends it
through an extra on BUGREPORT_STARTED; as it progress, it sets a system
property with its current progress and if the progress value overflows
the estimated max, it increases the max as well.

Shell uses the max/progress to display the progress % in the
system notification, and need to handle the scenario where the max
changes. The initial implementation would recalculate the progress, with
makes it swing back and forth as dumpstate increases the max.

This CL changes the Shell logic so the progress never go back, just
forward. The drawback of this approach is that if dumpstate
underestimate the maximum, the progress might get stuck in a high
value (99%) early on, but such issue will be addressed in the long
term by tuning the estimated max value.

BUG: 26354314
Change-Id: I3a5416acaffaaa43fd28d2f1f8ec8ea12aa0d91e
parent 1981e602

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

+63 −21
Original line number Diff line number Diff line
@@ -149,6 +149,10 @@ public class BugreportProgressService extends Service { 
    // Passed to Message.obtain() when msg.arg2 is not used.

    private static final int UNUSED_ARG2 = -2;



    // Maximum progress displayed (like 99.00%).

    private static final int CAPPED_PROGRESS = 9900;

    private static final int CAPPED_MAX = 10000;



    /**

     * Delay before a screenshot is taken.

     * <p>
@@ -427,7 +431,7 @@ public class BugreportProgressService extends Service { 
        final NumberFormat nf = NumberFormat.getPercentInstance();

        nf.setMinimumFractionDigits(2);

        nf.setMaximumFractionDigits(2);

        final String percentText = nf.format((double) info.progress / info.max);

        final String percentageText = nf.format((double) info.progress / info.max);

        final Action cancelAction = new Action.Builder(null, mContext.getString(

                com.android.internal.R.string.cancel), newCancelIntent(mContext, info)).build();

        final Intent infoIntent = new Intent(mContext, BugreportProgressService.class);
@@ -458,7 +462,7 @@ public class BugreportProgressService extends Service { 
                .setContentTitle(title)

                .setTicker(title)

                .setContentText(name)

                .setContentInfo(percentText)

                .setContentInfo(percentageText)

                .setProgress(info.max, info.progress, false)

                .setOngoing(true)

                .setContentIntent(infoPendingIntent)
@@ -472,7 +476,7 @@ public class BugreportProgressService extends Service { 
        }

        if (DEBUG) {

            Log.d(TAG, "Sending 'Progress' notification for id " + info.id + "(pid " + info.pid

                    + "): " + percentText);

                    + "): " + percentageText);

        }

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

    }
@@ -545,25 +549,47 @@ public class BugreportProgressService extends Service { 
            }

            activeProcesses++;

            final String progressKey = DUMPSTATE_PREFIX + pid + PROGRESS_SUFFIX;

            final int progress = SystemProperties.getInt(progressKey, 0);

            if (progress == 0) {

            info.realProgress = SystemProperties.getInt(progressKey, 0);

            if (info.realProgress == 0) {

                Log.v(TAG, "System property " + progressKey + " is not set yet");

            }

            final int max = SystemProperties.getInt(DUMPSTATE_PREFIX + pid + MAX_SUFFIX, 0);

            final boolean maxChanged = max > 0 && max != info.max;

            final boolean progressChanged = progress > 0 && progress != info.progress;

            final String maxKey = DUMPSTATE_PREFIX + pid + MAX_SUFFIX;

            info.realMax = SystemProperties.getInt(maxKey, info.max);

            if (info.realMax <= 0 ) {

                Log.w(TAG, "Property " + maxKey + " is not positive: " + info.max);

                continue;

            }

            /*

             * Checks whether the progress changed in a way that should be displayed to the user:

             * - info.progress / info.max represents the displayed progress

             * - info.realProgress / info.realMax represents the real progress

             * - since the real progress can decrease, the displayed progress is only updated if it

             *   increases

             * - the displayed progress is capped at a maximum (like 99%)

             */

            final int oldPercentage = (CAPPED_MAX * info.progress) / info.max;

            int newPercentage = (CAPPED_MAX * info.realProgress) / info.realMax;

            int max = info.realMax;

            int progress = info.realProgress;



            if (newPercentage > CAPPED_PROGRESS) {

                progress = newPercentage = CAPPED_PROGRESS;

                max = CAPPED_MAX;

            }



            if (progressChanged || maxChanged) {

                if (progressChanged) {

                    if (DEBUG) Log.v(TAG, "Updating progress for PID " + pid + "(id: " + id

                            + ") from " + info.progress + " to " + progress);

                    info.progress = progress;

            if (newPercentage > oldPercentage) {

                if (DEBUG) {

                    if (progress != info.progress) {

                        Log.v(TAG, "Updating progress for PID " + pid + "(id: " + id + ") from "

                                + info.progress + " to " + progress);

                    }

                if (maxChanged) {

                    Log.i(TAG, "Updating max progress for PID " + pid + "(id: " + id

                            + ") from " + info.max + " to " + max);

                    info.max = max;

                    if (max != info.max) {

                        Log.v(TAG, "Updating max progress for PID " + pid + "(id: " + id + ") from "

                                + info.max + " to " + max);

                    }

                }

                info.progress = progress;

                info.max = max;

                info.lastUpdate = System.currentTimeMillis();

                updateProgress(info);

            } else {
@@ -1450,15 +1476,25 @@ public class BugreportProgressService extends Service { 
        String description;



        /**

         * Maximum progress of the bugreport generation.

         * Maximum progress of the bugreport generation as displayed by the UI.

         */

        int max;



        /**

         * Current progress of the bugreport generation.

         * Current progress of the bugreport generation as displayed by the UI.

         */

        int progress;



        /**

         * Maximum progress of the bugreport generation as reported by dumpstate.

         */

        int realMax;



        /**

         * Current progress of the bugreport generation as reported by dumpstate.

         */

        int realProgress;



        /**

         * Time of the last progress update.

         */
@@ -1568,10 +1604,12 @@ public class BugreportProgressService extends Service { 
        @Override

        public String toString() {

            final float percent = ((float) progress * 100 / max);

            final float realPercent = ((float) realProgress * 100 / realMax);

            return "id: " + id + ", pid: " + pid + ", name: " + name + ", finished: " + finished

                    + "\n\ttitle: " + title + "\n\tdescription: " + description

                    + "\n\tfile: " + bugreportFile + "\n\tscreenshots: " + screenshotFiles

                    + "\n\tprogress: " + progress + "/" + max + " (" + percent + ")"

                    + "\n\treal progress: " + realProgress + "/" + realMax + " (" + realPercent + ")"

                    + "\n\tlast_update: " + getFormattedLastUpdate()

                    + "\naddingDetailsToZip: " + addingDetailsToZip

                    + " addedDetailsToZip: " + addedDetailsToZip;
@@ -1587,6 +1625,8 @@ public class BugreportProgressService extends Service { 
            description = in.readString();

            max = in.readInt();

            progress = in.readInt();

            realMax = in.readInt();

            realProgress = in.readInt();

            lastUpdate = in.readLong();

            formattedLastUpdate = in.readString();

            bugreportFile = readFile(in);
@@ -1609,6 +1649,8 @@ public class BugreportProgressService extends Service { 
            dest.writeString(description);

            dest.writeInt(max);

            dest.writeInt(progress);

            dest.writeInt(realMax);

            dest.writeInt(realProgress);

            dest.writeLong(lastUpdate);

            dest.writeString(getFormattedLastUpdate());

            writeFile(dest, bugreportFile);

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

+25 −2
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ import static com.android.shell.BugreportProgressService.EXTRA_PID; 
import static com.android.shell.BugreportProgressService.EXTRA_SCREENSHOT;

import static com.android.shell.BugreportProgressService.INTENT_BUGREPORT_FINISHED;

import static com.android.shell.BugreportProgressService.INTENT_BUGREPORT_STARTED;

import static com.android.shell.BugreportProgressService.POLLING_FREQUENCY;

import static com.android.shell.BugreportProgressService.SCREENSHOT_DELAY_SECONDS;



import java.io.BufferedOutputStream;
@@ -92,7 +93,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
    private static final String TAG = "BugreportReceiverTest";



    // Timeout for UI operations, in milliseconds.

    private static final int TIMEOUT = (int) BugreportProgressService.POLLING_FREQUENCY * 4;

    private static final int TIMEOUT = (int) POLLING_FREQUENCY * 4;



    // Timeout for when waiting for a screenshot to finish.

    private static final int SAFE_SCREENSHOT_DELAY = SCREENSHOT_DELAY_SECONDS + 10;
@@ -190,8 +191,30 @@ public class BugreportReceiverTest extends InstrumentationTestCase { 
        SystemProperties.set(PROGRESS_PROPERTY, "500");

        assertProgressNotification(NAME, nf.format(0.50));



        SystemProperties.set(PROGRESS_PROPERTY, "950");

        assertProgressNotification(NAME, nf.format(0.95));



        // Make sure progress never goes back...

        SystemProperties.set(MAX_PROPERTY, "2000");

        assertProgressNotification(NAME, nf.format(0.25));

        Thread.sleep(POLLING_FREQUENCY + DateUtils.SECOND_IN_MILLIS);

        assertProgressNotification(NAME, nf.format(0.95));



        SystemProperties.set(PROGRESS_PROPERTY, "1000");

        assertProgressNotification(NAME, nf.format(0.95));



        // ...only forward...

        SystemProperties.set(PROGRESS_PROPERTY, "1902");

        assertProgressNotification(NAME, nf.format(0.9510));



        SystemProperties.set(PROGRESS_PROPERTY, "1960");

        assertProgressNotification(NAME, nf.format(0.98));



        // ...but never more than the capped value.

        SystemProperties.set(PROGRESS_PROPERTY, "2000");

        assertProgressNotification(NAME, nf.format(0.99));



        SystemProperties.set(PROGRESS_PROPERTY, "3000");

        assertProgressNotification(NAME, nf.format(0.99));



        Bundle extras =

                sendBugreportFinishedAndGetSharedIntent(ID, mPlainTextPath, mScreenshotPath);

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

+3 −3
Original line number Diff line number Diff line
@@ -32,7 +32,7 @@ import static junit.framework.Assert.assertTrue; 
final class UiBot {



    private static final String TAG = "UiBot";

    private static final String SYSTEMUI_PACKAGED = "com.android.systemui";

    private static final String SYSTEMUI_PACKAGE = "com.android.systemui";



    private final UiDevice mDevice;

    private final int mTimeout;
@@ -51,8 +51,8 @@ final class UiBot { 
    public UiObject getNotification(String text) {

        boolean opened = mDevice.openNotification();

        Log.v(TAG, "openNotification(): " + opened);

        boolean gotIt = mDevice.wait(Until.hasObject(By.pkg(SYSTEMUI_PACKAGED)), mTimeout);

        assertTrue("could not get system ui (" + SYSTEMUI_PACKAGED + ")", gotIt);

        boolean gotIt = mDevice.wait(Until.hasObject(By.pkg(SYSTEMUI_PACKAGE)), mTimeout);

        assertTrue("could not get system ui (" + SYSTEMUI_PACKAGE + ")", gotIt);



        return getObject(text);

    }