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

Commit 964426e2 authored by Julia Reynolds's avatar Julia Reynolds
Browse files

Do less on the main thread 

With a settings ContentObserver. However, unlike the
last time only the binder-related work is happening on a
background thread, and everything else is on the main thread.

Test: android.platform.test.scenario.sysui.bubble.CreateBubbleFromNotificationTest#testCreateBubbleFromNotification
Test: ExpandableNotificationRowControllerTest
Test: NotificationSettingsControllerTest
Test: NotificationContentViewTest
Fixes: 271435682
Change-Id: I2d1f5c52107409a6c1081fb3000035046202cad3
parent f86743bf

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowController.java

+29 −0
Original line number Diff line number Diff line
@@ -21,12 +21,16 @@ import static com.android.systemui.statusbar.NotificationRemoteInputManager.ENAB 
import static com.android.systemui.statusbar.StatusBarState.KEYGUARD;

import static com.android.systemui.statusbar.notification.NotificationUtils.logKey;



import android.net.Uri;

import android.os.UserHandle;

import android.provider.Settings;

import android.util.Log;

import android.view.View;

import android.view.ViewGroup;



import androidx.annotation.NonNull;

import androidx.annotation.Nullable;

import androidx.annotation.VisibleForTesting;



import com.android.internal.logging.MetricsLogger;

import com.android.internal.statusbar.IStatusBarService;
@@ -71,6 +75,10 @@ import javax.inject.Named; 
@NotificationRowScope

public class ExpandableNotificationRowController implements NotifViewController {

    private static final String TAG = "NotifRowController";



    static final Uri BUBBLES_SETTING_URI =

            Settings.Secure.getUriFor(Settings.Secure.NOTIFICATION_BUBBLES);

    private static final String BUBBLES_SETTING_ENABLED_VALUE = "1";

    private final ExpandableNotificationRow mView;

    private final NotificationListContainer mListContainer;

    private final RemoteInputViewSubcomponent.Factory mRemoteInputViewSubcomponentFactory;
@@ -104,6 +112,23 @@ public class ExpandableNotificationRowController implements NotifViewController 
    private final ExpandableNotificationRowDragController mDragController;

    private final NotificationDismissibilityProvider mDismissibilityProvider;

    private final IStatusBarService mStatusBarService;



    private final NotificationSettingsController mSettingsController;



    @VisibleForTesting

    final NotificationSettingsController.Listener mSettingsListener =

            new NotificationSettingsController.Listener() {

                @Override

                public void onSettingChanged(Uri setting, int userId, String value) {

                    if (BUBBLES_SETTING_URI.equals(setting)) {

                        final int viewUserId = mView.getEntry().getSbn().getUserId();

                        if (viewUserId == UserHandle.USER_ALL || viewUserId == userId) {

                            mView.getPrivateLayout().setBubblesEnabledForUser(

                                    BUBBLES_SETTING_ENABLED_VALUE.equals(value));

                        }

                    }

                }

            };

    private final ExpandableNotificationRow.ExpandableNotificationRowLogger mLoggerCallback =

            new ExpandableNotificationRow.ExpandableNotificationRowLogger() {

                @Override
@@ -201,6 +226,7 @@ public class ExpandableNotificationRowController implements NotifViewController 
            FeatureFlags featureFlags,

            PeopleNotificationIdentifier peopleNotificationIdentifier,

            Optional<BubblesManager> bubblesManagerOptional,

            NotificationSettingsController settingsController,

            ExpandableNotificationRowDragController dragController,

            NotificationDismissibilityProvider dismissibilityProvider,

            IStatusBarService statusBarService) {
@@ -229,6 +255,7 @@ public class ExpandableNotificationRowController implements NotifViewController 
        mFeatureFlags = featureFlags;

        mPeopleNotificationIdentifier = peopleNotificationIdentifier;

        mBubblesManagerOptional = bubblesManagerOptional;

        mSettingsController = settingsController;

        mDragController = dragController;

        mMetricsLogger = metricsLogger;

        mChildrenContainerLogger = childrenContainerLogger;
@@ -298,12 +325,14 @@ public class ExpandableNotificationRowController implements NotifViewController 
                        NotificationMenuRowPlugin.class, false /* Allow multiple */);

                mView.setOnKeyguard(mStatusBarStateController.getState() == KEYGUARD);

                mStatusBarStateController.addCallback(mStatusBarStateListener);

                mSettingsController.addCallback(BUBBLES_SETTING_URI, mSettingsListener);

            }



            @Override

            public void onViewDetachedFromWindow(View v) {

                mPluginManager.removePluginListener(mView);

                mStatusBarStateController.removeCallback(mStatusBarStateListener);

                mSettingsController.removeCallback(BUBBLES_SETTING_URI, mSettingsListener);

            }

        });

    }

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java

+13 −2
Original line number Diff line number Diff line
@@ -41,6 +41,8 @@ import android.widget.FrameLayout; 
import android.widget.ImageView;

import android.widget.LinearLayout;



import androidx.annotation.MainThread;



import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.statusbar.IStatusBarService;

import com.android.systemui.R;
@@ -65,7 +67,6 @@ import com.android.systemui.statusbar.policy.SmartReplyStateInflaterKt; 
import com.android.systemui.statusbar.policy.SmartReplyView;

import com.android.systemui.statusbar.policy.dagger.RemoteInputViewSubcomponent;

import com.android.systemui.util.Compile;

import com.android.systemui.wmshell.BubblesManager;



import java.io.PrintWriter;

import java.util.ArrayList;
@@ -134,6 +135,7 @@ public class NotificationContentView extends FrameLayout implements Notification 
    private PeopleNotificationIdentifier mPeopleIdentifier;

    private RemoteInputViewSubcomponent.Factory mRemoteInputSubcomponentFactory;

    private IStatusBarService mStatusBarService;

    private boolean mBubblesEnabledForUser;



    /**

     * List of listeners for when content views become inactive (i.e. not the showing view).
@@ -1440,12 +1442,20 @@ public class NotificationContentView extends FrameLayout implements Notification 
        }

    }



    @MainThread

    public void setBubblesEnabledForUser(boolean enabled) {

        mBubblesEnabledForUser = enabled;



        applyBubbleAction(mExpandedChild, mNotificationEntry);

        applyBubbleAction(mHeadsUpChild, mNotificationEntry);

    }



    @VisibleForTesting

    boolean shouldShowBubbleButton(NotificationEntry entry) {

        boolean isPersonWithShortcut =

                mPeopleIdentifier.getPeopleNotificationType(entry)

                        >= PeopleNotificationIdentifier.TYPE_FULL_PERSON;

        return BubblesManager.areBubblesEnabled(mContext, entry.getSbn().getUser())

        return mBubblesEnabledForUser

                && isPersonWithShortcut

                && entry.getBubbleMetadata() != null;

    }
@@ -2079,6 +2089,7 @@ public class NotificationContentView extends FrameLayout implements Notification 
            pw.print("null");

        }

        pw.println();

        pw.println("mBubblesEnabledForUser: " + mBubblesEnabledForUser);



        pw.print("RemoteInputViews { ");

        pw.print(" visibleType: " + mVisibleType);

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationSettingsController.java

0 → 100644
+180 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2023 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.systemui.statusbar.notification.row;



import android.content.Context;

import android.database.ContentObserver;

import android.net.Uri;

import android.os.Handler;

import android.os.HandlerExecutor;



import androidx.annotation.MainThread;

import androidx.annotation.NonNull;

import androidx.annotation.Nullable;



import com.android.systemui.Dumpable;

import com.android.systemui.dagger.SysUISingleton;

import com.android.systemui.dagger.qualifiers.Background;

import com.android.systemui.dagger.qualifiers.Main;

import com.android.systemui.dump.DumpManager;

import com.android.systemui.settings.UserTracker;

import com.android.systemui.util.settings.SecureSettings;



import java.io.PrintWriter;

import java.util.ArrayList;

import java.util.HashMap;



import javax.inject.Inject;



/**

 * Centralized controller for listening to Secure Settings changes and informing in-process

 * listeners, on a background thread.

 */

@SysUISingleton

public class NotificationSettingsController implements Dumpable {



    private final static String TAG = "NotificationSettingsController";

    private final UserTracker mUserTracker;

    private final UserTracker.Callback mCurrentUserTrackerCallback;

    private final Handler mMainHandler;

    private final Handler mBackgroundHandler;

    private final ContentObserver mContentObserver;

    private final SecureSettings mSecureSettings;

    private final HashMap<Uri, ArrayList<Listener>> mListeners = new HashMap<>();



    @Inject

    public NotificationSettingsController(UserTracker userTracker,

            @Main Handler mainHandler,

            @Background Handler backgroundHandler,

            SecureSettings secureSettings,

            DumpManager dumpManager) {

        mUserTracker = userTracker;

        mMainHandler = mainHandler;

        mBackgroundHandler = backgroundHandler;

        mSecureSettings = secureSettings;

        mContentObserver = new ContentObserver(mBackgroundHandler) {

            @Override

            public void onChange(boolean selfChange, Uri uri) {

                super.onChange(selfChange, uri);

                synchronized (mListeners) {

                    if (mListeners.containsKey(uri)) {

                        int userId = mUserTracker.getUserId();

                        String value = getCurrentSettingValue(uri, userId);

                        for (Listener listener : mListeners.get(uri)) {

                            mMainHandler.post(() -> listener.onSettingChanged(uri, userId, value));

                        }

                    }

                }

            }

        };



        mCurrentUserTrackerCallback = new UserTracker.Callback() {

            @Override

            public void onUserChanged(int newUser, Context userContext) {

                synchronized (mListeners) {

                    if (mListeners.size() > 0) {

                        mSecureSettings.unregisterContentObserver(mContentObserver);

                        for (Uri uri : mListeners.keySet()) {

                            mSecureSettings.registerContentObserverForUser(

                                    uri, false, mContentObserver, newUser);

                        }

                    }

                }

            }

        };

        mUserTracker.addCallback(

                mCurrentUserTrackerCallback, new HandlerExecutor(mBackgroundHandler));



        dumpManager.registerNormalDumpable(TAG, this);

    }



    /**

     * Register a callback whenever the given secure settings changes.

     *

     * On registration, will trigger the listener on the main thread with the current value of

     * the setting.

     */

    @Main

    public void addCallback(@NonNull Uri uri, @NonNull Listener listener) {

        if (uri == null || listener == null) {

            return;

        }

        synchronized (mListeners) {

            ArrayList<Listener> currentListeners = mListeners.get(uri);

            if (currentListeners == null) {

                currentListeners = new ArrayList<>();

            }

            if (!currentListeners.contains(listener)) {

                currentListeners.add(listener);

            }

            mListeners.put(uri, currentListeners);

            if (currentListeners.size() == 1) {

                mSecureSettings.registerContentObserverForUser(

                        uri, false, mContentObserver, mUserTracker.getUserId());

            }

        }

        mBackgroundHandler.post(() -> {

            int userId = mUserTracker.getUserId();

            String value = getCurrentSettingValue(uri, userId);

            mMainHandler.post(() -> listener.onSettingChanged(uri, userId, value));

        });



    }



    public void removeCallback(Uri uri, Listener listener) {

        synchronized (mListeners) {

            ArrayList<Listener> currentListeners = mListeners.get(uri);



            if (currentListeners != null) {

                currentListeners.remove(listener);

            }

            if (currentListeners == null || currentListeners.size() == 0) {

                mListeners.remove(uri);

            }



            if (mListeners.size() == 0) {

                mSecureSettings.unregisterContentObserver(mContentObserver);

            }

        }

    }



    @Override

    public void dump(@NonNull PrintWriter pw, @NonNull String[] args) {

        synchronized (mListeners) {

            pw.println("Settings Uri Listener List:");

            for (Uri uri : mListeners.keySet()) {

                pw.println("   Uri=" + uri);

                for (Listener listener : mListeners.get(uri)) {

                    pw.println("      Listener=" + listener.getClass().getName());

                }

            }

        }

    }



    private String getCurrentSettingValue(Uri uri, int userId) {

        final String setting = uri == null ? null : uri.getLastPathSegment();

        return mSecureSettings.getStringForUser(setting, userId);

    }



    /**

     * Listener invoked whenever settings are changed.

     */

    public interface Listener {

        @MainThread

        void onSettingChanged(@NonNull Uri setting, int userId, @Nullable String value);

    }

}
 
 No newline at end of file

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowControllerTest.kt

+87 −2
Original line number Diff line number Diff line
@@ -17,6 +17,10 @@ 


package com.android.systemui.statusbar.notification.row



import android.app.Notification

import android.net.Uri

import android.os.UserHandle

import android.os.UserHandle.USER_ALL

import android.testing.AndroidTestingRunner

import android.testing.TestableLooper

import androidx.test.filters.SmallTest
@@ -29,13 +33,17 @@ import com.android.systemui.flags.FeatureFlags 
import com.android.systemui.plugins.FalsingManager

import com.android.systemui.plugins.PluginManager

import com.android.systemui.plugins.statusbar.StatusBarStateController

import com.android.systemui.statusbar.SbnBuilder

import com.android.systemui.statusbar.SmartReplyController

import com.android.systemui.statusbar.notification.collection.NotificationEntry

import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder

import com.android.systemui.statusbar.notification.collection.provider.NotificationDismissibilityProvider

import com.android.systemui.statusbar.notification.collection.render.FakeNodeController

import com.android.systemui.statusbar.notification.collection.render.GroupExpansionManager

import com.android.systemui.statusbar.notification.collection.render.GroupMembershipManager

import com.android.systemui.statusbar.notification.logging.NotificationLogger

import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier

import com.android.systemui.statusbar.notification.row.ExpandableNotificationRowController.BUBBLES_SETTING_URI

import com.android.systemui.statusbar.notification.stack.NotificationChildrenContainer

import com.android.systemui.statusbar.notification.stack.NotificationChildrenContainerLogger

import com.android.systemui.statusbar.notification.stack.NotificationListContainer
@@ -46,9 +54,9 @@ import com.android.systemui.statusbar.policy.dagger.RemoteInputViewSubcomponent 
import com.android.systemui.util.mockito.any

import com.android.systemui.util.mockito.eq

import com.android.systemui.util.mockito.mock

import com.android.systemui.util.mockito.withArgCaptor

import com.android.systemui.util.time.SystemClock

import com.android.systemui.wmshell.BubblesManager

import java.util.Optional

import junit.framework.Assert

import org.junit.After

import org.junit.Before
@@ -56,9 +64,11 @@ import org.junit.Test 
import org.junit.runner.RunWith

import org.mockito.Mockito

import org.mockito.Mockito.anyBoolean

import org.mockito.Mockito.mock

import org.mockito.Mockito.never

import org.mockito.Mockito.verify

import org.mockito.Mockito.`when` as whenever

import java.util.Optional



@SmallTest

@RunWith(AndroidTestingRunner::class)
@@ -94,10 +104,10 @@ class ExpandableNotificationRowControllerTest : SysuiTestCase() { 
    private val featureFlags: FeatureFlags = mock()

    private val peopleNotificationIdentifier: PeopleNotificationIdentifier = mock()

    private val bubblesManager: BubblesManager = mock()

    private val settingsController: NotificationSettingsController = mock()

    private val dragController: ExpandableNotificationRowDragController = mock()

    private val dismissibilityProvider: NotificationDismissibilityProvider = mock()

    private val statusBarService: IStatusBarService = mock()



    private lateinit var controller: ExpandableNotificationRowController



    @Before
@@ -134,11 +144,16 @@ class ExpandableNotificationRowControllerTest : SysuiTestCase() { 
                featureFlags,

                peopleNotificationIdentifier,

                Optional.of(bubblesManager),

                settingsController,

                dragController,

                dismissibilityProvider,

                statusBarService

            )

        whenever(view.childrenContainer).thenReturn(childrenContainer)



        val notification = Notification.Builder(mContext).build()

        val sbn = SbnBuilder().setNotification(notification).build()

        whenever(view.entry).thenReturn(NotificationEntryBuilder().setSbn(sbn).build())

    }



    @After
@@ -206,4 +221,74 @@ class ExpandableNotificationRowControllerTest : SysuiTestCase() { 
        verify(view).removeChildNotification(eq(childView))

        verify(listContainer).notifyGroupChildRemoved(eq(childView), eq(childrenContainer))

    }



    @Test

    fun registerSettingsListener_forBubbles() {

        controller.init(mock(NotificationEntry::class.java))

        val viewStateObserver = withArgCaptor {

            verify(view).addOnAttachStateChangeListener(capture());

        }

        viewStateObserver.onViewAttachedToWindow(view);

        verify(settingsController).addCallback(any(), any());

    }



    @Test

    fun unregisterSettingsListener_forBubbles() {

        controller.init(mock(NotificationEntry::class.java))

        val viewStateObserver = withArgCaptor {

            verify(view).addOnAttachStateChangeListener(capture());

        }

        viewStateObserver.onViewDetachedFromWindow(view);

        verify(settingsController).removeCallback(any(), any());

    }



    @Test

    fun settingsListener_invalidUri() {

        controller.mSettingsListener.onSettingChanged(Uri.EMPTY, view.entry.sbn.userId, "1")



        verify(view, never()).getPrivateLayout()

    }



    @Test

    fun settingsListener_invalidUserId() {

        controller.mSettingsListener.onSettingChanged(BUBBLES_SETTING_URI, -1000, "1")

        controller.mSettingsListener.onSettingChanged(BUBBLES_SETTING_URI, -1000, null)



        verify(view, never()).getPrivateLayout()

    }



    @Test

    fun settingsListener_validUserId() {

        val childView: NotificationContentView = mock()

        whenever(view.privateLayout).thenReturn(childView)



        controller.mSettingsListener.onSettingChanged(

                BUBBLES_SETTING_URI, view.entry.sbn.userId, "1")

        verify(childView).setBubblesEnabledForUser(true)



        controller.mSettingsListener.onSettingChanged(

                BUBBLES_SETTING_URI, view.entry.sbn.userId, "9")

        verify(childView).setBubblesEnabledForUser(false)

    }



    @Test

    fun settingsListener_userAll() {

        val childView: NotificationContentView = mock()

        whenever(view.privateLayout).thenReturn(childView)



        val notification = Notification.Builder(mContext).build()

        val sbn = SbnBuilder().setNotification(notification)

                .setUser(UserHandle.of(USER_ALL))

                .build()

        whenever(view.entry).thenReturn(NotificationEntryBuilder()

                .setSbn(sbn)

                .setUser(UserHandle.of(USER_ALL))

                .build())



        controller.mSettingsListener.onSettingChanged(BUBBLES_SETTING_URI, 9, "1")

        verify(childView).setBubblesEnabledForUser(true)



        controller.mSettingsListener.onSettingChanged(BUBBLES_SETTING_URI, 1, "0")

        verify(childView).setBubblesEnabledForUser(false)

    }

}

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationContentViewTest.kt

+9 −1
Original line number Diff line number Diff line
@@ -250,6 +250,9 @@ class NotificationContentViewTest : SysuiTestCase() { 
            .thenReturn(actionListMarginTarget)

        view.setContainingNotification(mockContainingNotification)



        // Given: controller says bubbles are enabled for the user

        view.setBubblesEnabledForUser(true);



        // When: call NotificationContentView.setExpandedChild() to set the expandedChild

        view.expandedChild = mockExpandedChild
@@ -305,6 +308,12 @@ class NotificationContentViewTest : SysuiTestCase() { 
        // NotificationEntry, which should show bubble button

        view.onNotificationUpdated(createMockNotificationEntry(true))



        // Then: no bubble yet

        assertEquals(notificationContentMargin, getMarginBottom(actionListMarginTarget))



        // Given: controller says bubbles are enabled for the user

        view.setBubblesEnabledForUser(true);



        // Then: bottom margin of actionListMarginTarget should not change, still be 20

        assertEquals(0, getMarginBottom(actionListMarginTarget))

    }
@@ -405,7 +414,6 @@ class NotificationContentViewTest : SysuiTestCase() { 
            val userMock: UserHandle = mock()

            whenever(this.sbn).thenReturn(sbnMock)

            whenever(sbnMock.user).thenReturn(userMock)

            doReturn(showButton).whenever(view).shouldShowBubbleButton(this)

        }



    private fun createLinearLayoutWithBottomMargin(bottomMargin: Int): LinearLayout {