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

Commit 3aaed291 authored by Makoto Onuki's avatar Makoto Onuki
Browse files

Follow-up for Ife38c2cd94ac9902911b005dbbca8b0d0a62e6d7

Address review comments on the previous CL.
(Plus a couple bug fixes.)

Test: atest BatterySaverPolicyTest
Test: manual test
Bug: 68769804
Change-Id: If2de9148d1b8175a9f0d66bc3a7ecd02ce7a620b
parent a786f00f
Loading
Loading
Loading
Loading
+8 −0
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;

/**
@@ -133,6 +134,13 @@ public class ArrayUtils {
        return array == null || array.isEmpty();
    }

    /**
     * Checks if given map is null or has zero elements.
     */
    public static boolean isEmpty(@Nullable Map<?, ?> map) {
        return map == null || map.isEmpty();
    }

    /**
     * Checks if given array is null or has zero elements.
     */
+21 −21
Original line number Diff line number Diff line
@@ -35,10 +35,15 @@ import com.android.internal.R;

import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.List;

/**
 * Class to decide whether to turn on battery saver mode for specific service
 *
 * TODO: We should probably make {@link #mFilesForInteractive} and {@link #mFilesForNoninteractive}
 * less flexible and just take a list of "CPU number - frequency" pairs. Being able to write
 * anything under /sys/ and /proc/ is too loose.
 *
 * Test: atest BatterySaverPolicyTest
 */
public class BatterySaverPolicy extends ContentObserver {
@@ -66,8 +71,8 @@ public class BatterySaverPolicy extends ContentObserver {
    private static final String KEY_FORCE_ALL_APPS_STANDBY_ALARMS = "force_all_apps_standby_alarms";
    private static final String KEY_OPTIONAL_SENSORS_DISABLED = "optional_sensors_disabled";

    private static final String KEY_SCREEN_ON_FILE_PREFIX = "file-on:";
    private static final String KEY_SCREEN_OFF_FILE_PREFIX = "file-off:";
    private static final String KEY_FILE_FOR_INTERACTIVE_PREFIX = "file-on:";
    private static final String KEY_FILE_FOR_NONINTERACTIVE_PREFIX = "file-off:";

    private static String mSettings;
    private static String mDeviceSpecificSettings;
@@ -179,25 +184,25 @@ public class BatterySaverPolicy extends ContentObserver {
    private ContentResolver mContentResolver;

    @GuardedBy("mLock")
    private final ArrayList<BatterySaverPolicyListener> mListeners = new ArrayList<>();
    private final List<BatterySaverPolicyListener> mListeners = new ArrayList<>();

    /**
     * List of [Filename -> content] that should be written when battery saver is activated
     * and the screen is on.
     * and the device is interactive.
     *
     * We use this to change the max CPU frequencies.
     */
    @GuardedBy("mLock")
    private ArrayMap<String, String> mScreenOnFiles;
    private ArrayMap<String, String> mFilesForInteractive;

    /**
     * List of [Filename -> content] that should be written when battery saver is activated
     * and the screen is off.
     * and the device is non-interactive.
     *
     * We use this to change the max CPU frequencies.
     */
    @GuardedBy("mLock")
    private ArrayMap<String, String> mScreenOffFiles;
    private ArrayMap<String, String> mFilesForNoninteractive;

    public interface BatterySaverPolicyListener {
        void onBatterySaverPolicyChanged(BatterySaverPolicy policy);
@@ -236,11 +241,6 @@ public class BatterySaverPolicy extends ContentObserver {
        return R.string.config_batterySaverDeviceSpecificConfig;
    }

    @VisibleForTesting
    void onChangeForTest() {
        onChange(true, null);
    }

    @Override
    public void onChange(boolean selfChange, Uri uri) {
        final BatterySaverPolicyListener[] listeners;
@@ -315,8 +315,8 @@ public class BatterySaverPolicy extends ContentObserver {
                    + deviceSpecificSetting);
        }

        mScreenOnFiles = collectParams(parser, KEY_SCREEN_ON_FILE_PREFIX);
        mScreenOffFiles = collectParams(parser, KEY_SCREEN_OFF_FILE_PREFIX);
        mFilesForInteractive = collectParams(parser, KEY_FILE_FOR_INTERACTIVE_PREFIX);
        mFilesForNoninteractive = collectParams(parser, KEY_FILE_FOR_NONINTERACTIVE_PREFIX);
    }

    private static ArrayMap<String, String> collectParams(
@@ -330,7 +330,7 @@ public class BatterySaverPolicy extends ContentObserver {
            }
            final String path = key.substring(prefix.length());

            if (!(path.startsWith("/sys/") || path.startsWith("/proc"))) {
            if (!(path.startsWith("/sys/") || path.startsWith("/proc/"))) {
                Slog.wtf(TAG, "Invalid path: " + path);
                continue;
            }
@@ -403,9 +403,9 @@ public class BatterySaverPolicy extends ContentObserver {
        }
    }

    public ArrayMap<String, String> getFileValues(boolean screenOn) {
    public ArrayMap<String, String> getFileValues(boolean interactive) {
        synchronized (mLock) {
            return screenOn ? mScreenOnFiles : mScreenOffFiles;
            return interactive ? mFilesForInteractive : mFilesForNoninteractive;
        }
    }

@@ -433,12 +433,12 @@ public class BatterySaverPolicy extends ContentObserver {
            pw.println("  " + KEY_OPTIONAL_SENSORS_DISABLED + "=" + mOptionalSensorsDisabled);
            pw.println();

            pw.print("  Screen On Files:\n");
            dumpMap(pw, "    ", mScreenOnFiles);
            pw.print("  Interactive File values:\n");
            dumpMap(pw, "    ", mFilesForInteractive);
            pw.println();

            pw.print("  Screen Off Files:\n");
            dumpMap(pw, "    ", mScreenOffFiles);
            pw.print("  Noninteractive File values:\n");
            dumpMap(pw, "    ", mFilesForNoninteractive);
            pw.println();
        }
    }
+1 −1
Original line number Diff line number Diff line
@@ -3105,7 +3105,7 @@ public final class PowerManagerService extends SystemService
        mIsVrModeEnabled = enabled;
    }

    public static void powerHintInternal(int hintId, int data) {
    private static void powerHintInternal(int hintId, int data) {
        nativeSendPowerHint(hintId, data);
    }

+46 −28
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import android.os.Handler;
import android.os.Looper;
import android.os.Message;
import android.os.PowerManager;
import android.os.PowerManagerInternal;
import android.os.PowerManagerInternal.LowPowerModeListener;
import android.os.PowerSaveState;
import android.os.UserHandle;
@@ -33,7 +34,9 @@ import android.util.Slog;
import android.widget.Toast;

import com.android.internal.annotations.GuardedBy;
import com.android.internal.util.ArrayUtils;
import com.android.internal.util.Preconditions;
import com.android.server.LocalServices;
import com.android.server.power.BatterySaverPolicy;
import com.android.server.power.BatterySaverPolicy.BatterySaverPolicyListener;
import com.android.server.power.PowerManagerService;
@@ -63,20 +66,17 @@ public class BatterySaverController implements BatterySaverPolicyListener {
    @GuardedBy("mLock")
    private boolean mEnabled;

    /**
     * Keep track of the previous enabled state, which we use to decide when to send broadcasts,
     * which we don't want to send only when the screen state changes.
     */
    @GuardedBy("mLock")
    private boolean mWasEnabled;

    private final BroadcastReceiver mReceiver = new BroadcastReceiver() {
        @Override
        public void onReceive(Context context, Intent intent) {
            switch (intent.getAction()) {
                case Intent.ACTION_SCREEN_ON:
                case Intent.ACTION_SCREEN_OFF:
                    mHandler.postStateChanged();
                    if (!isEnabled()) {
                        return; // No need to send it if not enabled.
                    }
                    // Don't send the broadcast, because we never did so in this case.
                    mHandler.postStateChanged(/*sendBroadcast=*/ false);
                    break;
            }
        }
@@ -121,25 +121,32 @@ public class BatterySaverController implements BatterySaverPolicyListener {

    @Override
    public void onBatterySaverPolicyChanged(BatterySaverPolicy policy) {
        mHandler.postStateChanged();
        if (!isEnabled()) {
            return; // No need to send it if not enabled.
        }
        mHandler.postStateChanged(/*sendBroadcast=*/ true);
    }

    private class MyHandler extends Handler {
        private final int MSG_STATE_CHANGED = 1;
        private static final int MSG_STATE_CHANGED = 1;

        private static final int ARG_DONT_SEND_BROADCAST = 0;
        private static final int ARG_SEND_BROADCAST = 1;

        public MyHandler(Looper looper) {
            super(looper);
        }

        public void postStateChanged() {
            obtainMessage(MSG_STATE_CHANGED).sendToTarget();
        public void postStateChanged(boolean sendBroadcast) {
            obtainMessage(MSG_STATE_CHANGED, sendBroadcast ?
                    ARG_SEND_BROADCAST : ARG_DONT_SEND_BROADCAST, 0).sendToTarget();
        }

        @Override
        public void dispatchMessage(Message msg) {
            switch (msg.what) {
                case MSG_STATE_CHANGED:
                    handleBatterySaverStateChanged();
                    handleBatterySaverStateChanged(msg.arg1 == ARG_SEND_BROADCAST);
                    break;
            }
        }
@@ -155,38 +162,53 @@ public class BatterySaverController implements BatterySaverPolicyListener {
            }
            mEnabled = enable;

            mHandler.postStateChanged();
            mHandler.postStateChanged(/*sendBroadcast=*/ true);
        }
    }

    /** @return whether battery saver is enabled or not. */
    boolean isEnabled() {
        synchronized (mLock) {
            return mEnabled;
        }
    }

    /**
     * Dispatch power save events to the listeners.
     *
     * This is always called on the handler thread.
     * This method is always called on the handler thread.
     *
     * This method is called only in the following cases:
     * - When battery saver becomes activated.
     * - When battery saver becomes deactivated.
     * - When battery saver is on the interactive state changes.
     * - When battery saver is on the battery saver policy changes.
     */
    void handleBatterySaverStateChanged() {
    void handleBatterySaverStateChanged(boolean sendBroadcast) {
        final LowPowerModeListener[] listeners;

        final boolean wasEnabled;
        final boolean enabled;
        final boolean isScreenOn = getPowerManager().isInteractive();
        final boolean isInteractive = getPowerManager().isInteractive();
        final ArrayMap<String, String> fileValues;

        synchronized (mLock) {
            Slog.i(TAG, "Battery saver enabled: screen on=" + isScreenOn);
            Slog.i(TAG, "Battery saver " + (mEnabled ? "enabled" : "disabled")
                    + ": isInteractive=" + isInteractive);

            listeners = mListeners.toArray(new LowPowerModeListener[mListeners.size()]);
            wasEnabled = mWasEnabled;
            enabled = mEnabled;

            if (enabled) {
                fileValues = mBatterySaverPolicy.getFileValues(isScreenOn);
                fileValues = mBatterySaverPolicy.getFileValues(isInteractive);
            } else {
                fileValues = null;
            }
        }

        PowerManagerService.powerHintInternal(PowerHint.LOW_POWER, enabled ? 1 : 0);
        final PowerManagerInternal pmi = LocalServices.getService(PowerManagerInternal.class);
        if (pmi != null) {
            pmi.powerHint(PowerHint.LOW_POWER, enabled ? 1 : 0);
        }

        if (enabled) {
            // STOPSHIP Remove the toast.
@@ -195,13 +217,13 @@ public class BatterySaverController implements BatterySaverPolicyListener {
                    Toast.LENGTH_LONG).show();
        }

        if (fileValues == null || fileValues.size() == 0) {
        if (ArrayUtils.isEmpty(fileValues)) {
            mFileUpdater.restoreDefault();
        } else {
            mFileUpdater.writeFiles(fileValues);
        }

        if (enabled != wasEnabled) {
        if (sendBroadcast) {
            if (DEBUG) {
                Slog.i(TAG, "Sending broadcasts for mode: " + enabled);
            }
@@ -231,9 +253,5 @@ public class BatterySaverController implements BatterySaverPolicyListener {
                listener.onLowPowerModeChanged(result);
            }
        }

        synchronized (mLock) {
            mWasEnabled = enabled;
        }
    }
}
+11 −5
Original line number Diff line number Diff line
@@ -18,13 +18,13 @@ package com.android.server.power;
import android.os.PowerManager.ServiceType;
import android.os.PowerSaveState;
import android.os.Handler;
import android.provider.Settings;
import android.provider.Settings.Global;
import android.test.AndroidTestCase;
import android.test.suitebuilder.annotation.SmallTest;
import android.util.ArrayMap;

import com.android.frameworks.servicestests.R;
import com.android.internal.annotations.VisibleForTesting;

import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
@@ -68,6 +68,12 @@ public class BatterySaverPolicyTest extends AndroidTestCase {
        int getDeviceSpecificConfigResId() {
            return mDeviceSpecificConfigResId;
        }


        @VisibleForTesting
        void onChange() {
            onChange(true, null);
        }
    }

    @Mock
@@ -221,14 +227,14 @@ public class BatterySaverPolicyTest extends AndroidTestCase {
        mMockGlobalSettings.put(Global.BATTERY_SAVER_CONSTANTS, "");
        mMockGlobalSettings.put(Global.BATTERY_SAVER_DEVICE_SPECIFIC_CONSTANTS, "");

        mBatterySaverPolicy.onChangeForTest();
        mBatterySaverPolicy.onChange();
        assertThat(mBatterySaverPolicy.getFileValues(true).toString()).isEqualTo("{}");
        assertThat(mBatterySaverPolicy.getFileValues(false).toString()).isEqualTo("{}");


        mDeviceSpecificConfigResId = R.string.config_batterySaverDeviceSpecificConfig_2;

        mBatterySaverPolicy.onChangeForTest();
        mBatterySaverPolicy.onChange();
        assertThat(mBatterySaverPolicy.getFileValues(true).toString()).isEqualTo("{}");
        assertThat(mBatterySaverPolicy.getFileValues(false).toString())
                .isEqualTo("{/sys/a=1, /sys/b=2}");
@@ -236,7 +242,7 @@ public class BatterySaverPolicyTest extends AndroidTestCase {

        mDeviceSpecificConfigResId = R.string.config_batterySaverDeviceSpecificConfig_3;

        mBatterySaverPolicy.onChangeForTest();
        mBatterySaverPolicy.onChange();
        assertThat(mBatterySaverPolicy.getFileValues(true).toString()).isEqualTo("{/proc/c=4}");
        assertThat(mBatterySaverPolicy.getFileValues(false).toString()).isEqualTo("{/sys/a=3}");

@@ -244,7 +250,7 @@ public class BatterySaverPolicyTest extends AndroidTestCase {
        mMockGlobalSettings.put(Global.BATTERY_SAVER_DEVICE_SPECIFIC_CONSTANTS,
                "file-on:/proc/z=4");

        mBatterySaverPolicy.onChangeForTest();
        mBatterySaverPolicy.onChange();
        assertThat(mBatterySaverPolicy.getFileValues(true).toString()).isEqualTo("{/proc/z=4}");
        assertThat(mBatterySaverPolicy.getFileValues(false).toString()).isEqualTo("{}");
    }