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

Commit 70b54e38 authored by Stefan Andonian's avatar Stefan Andonian
Browse files

Make RecordIssueTile's ServiceConnections not injected, but created. 

This fixes the issue of sometimes having the dagger injection giving the
same instance in the tile and the service, and sometimes not. We can
avoid this by ensuring the service connections are always different.

Also, by Binding to IssueRecordingService, we can avoid "leaking" the
traceurConnection which would otherwise need to be created / destroyed
with every notification action.

Bug: 364824477
Test: Verified locally that System Tracing via the Record Issue Tile
works great in Headless System User Mode.
Flag: EXEMPT bug fix

Change-Id: I076f7a9c058fbc725296a3c2bb34cbc75675d30c
parent c0a488ea

packages/SystemUI/multivalentTests/src/com/android/systemui/recordissue/RecordIssueDialogDelegateTest.kt

+0 −2
Original line number Diff line number Diff line
@@ -78,7 +78,6 @@ class RecordIssueDialogDelegateTest : SysuiTestCase() { 
    @Mock private lateinit var sysuiState: SysUiState

    @Mock private lateinit var systemUIDialogManager: SystemUIDialogManager

    @Mock private lateinit var broadcastDispatcher: BroadcastDispatcher

    @Mock private lateinit var traceurConnection: TraceurConnection

    private val systemClock = FakeSystemClock()

    private val bgExecutor = FakeExecutor(systemClock)

    private val mainExecutor = FakeExecutor(systemClock)
@@ -120,7 +119,6 @@ class RecordIssueDialogDelegateTest : SysuiTestCase() { 
                    mediaProjectionMetricsLogger,

                    screenCaptureDisabledDialogDelegate,

                    state,

                    traceurConnection,

                ) {

                    latch.countDown()

                }

packages/SystemUI/multivalentTests/src/com/android/systemui/recordissue/TraceurConnectionTest.kt

+1 −1
Original line number Diff line number Diff line
@@ -50,7 +50,7 @@ class TraceurConnectionTest : SysuiTestCase() { 
    fun setup() {

        MockitoAnnotations.initMocks(this)

        whenever(userContextProvider.userContext).thenReturn(mContext)

        underTest = TraceurConnection(userContextProvider, Looper.getMainLooper())

        underTest = TraceurConnection.Provider(userContextProvider, Looper.getMainLooper()).create()

    }



    @Test

packages/SystemUI/src/com/android/systemui/qs/tiles/RecordIssueTile.kt

+23 −3
Original line number Diff line number Diff line
@@ -45,6 +45,7 @@ import com.android.systemui.qs.pipeline.domain.interactor.PanelInteractor 
import com.android.systemui.qs.tileimpl.QSTileImpl

import com.android.systemui.recordissue.IssueRecordingService.Companion.getStartIntent

import com.android.systemui.recordissue.IssueRecordingService.Companion.getStopIntent

import com.android.systemui.recordissue.IssueRecordingServiceConnection

import com.android.systemui.recordissue.IssueRecordingState

import com.android.systemui.recordissue.RecordIssueDialogDelegate

import com.android.systemui.recordissue.RecordIssueModule.Companion.TILE_SPEC
@@ -66,7 +67,7 @@ class RecordIssueTile 
constructor(

    host: QSHost,

    uiEventLogger: QsEventLogger,

    @Background backgroundLooper: Looper,

    @Background private val backgroundLooper: Looper,

    @Main mainHandler: Handler,

    falsingManager: FalsingManager,

    metricsLogger: MetricsLogger,
@@ -78,7 +79,8 @@ constructor( 
    private val dialogTransitionAnimator: DialogTransitionAnimator,

    private val panelInteractor: PanelInteractor,

    private val userContextProvider: UserContextProvider,

    private val traceurConnection: TraceurConnection,

    irsConnProvider: IssueRecordingServiceConnection.Provider,

    traceurConnProvider: TraceurConnection.Provider,

    @Background private val bgExecutor: Executor,

    private val issueRecordingState: IssueRecordingState,

    private val delegateFactory: RecordIssueDialogDelegate.Factory,
@@ -98,6 +100,15 @@ constructor( 


    private val onRecordingChangeListener = Runnable { refreshState() }



    private val irsConnection: IssueRecordingServiceConnection = irsConnProvider.create()

    private val traceurConnection =

        traceurConnProvider.create().apply {

            onBound.add {

                getTags(issueRecordingState)

                doUnBind()

            }

        }



    override fun handleSetListening(listening: Boolean) {

        super.handleSetListening(listening)

        if (listening) {
@@ -109,7 +120,7 @@ constructor( 


    override fun handleDestroy() {

        super.handleDestroy()

        bgExecutor.execute { traceurConnection.doUnBind() }

        bgExecutor.execute { irsConnection.doUnBind() }

    }



    override fun getTileLabel(): CharSequence = mContext.getString(R.string.qs_record_issue_label)
@@ -158,6 +169,15 @@ constructor( 
        )



    private fun showPrompt(expandable: Expandable?) {

        bgExecutor.execute {

            // We only want to get the tags once per session, as this is not likely to change, if at

            // all on a month to month basis. Using onBound's size is a way to verify if the tag

            // retrieval has already happened or not.

            if (traceurConnection.onBound.isNotEmpty()) {

                traceurConnection.doBind()

            }

            irsConnection.doBind()

        }

        val dialog: AlertDialog =

            delegateFactory

                .create {

packages/SystemUI/src/com/android/systemui/recordissue/IssueRecordingService.kt

+24 −1
Original line number Diff line number Diff line
@@ -23,9 +23,12 @@ import android.content.Intent 
import android.content.res.Resources

import android.net.Uri

import android.os.Handler

import android.os.IBinder

import android.os.Looper

import android.util.Log

import com.android.internal.logging.UiEventLogger

import com.android.systemui.animation.DialogTransitionAnimator

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

import com.android.systemui.dagger.qualifiers.LongRunning

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

import com.android.systemui.qs.pipeline.domain.interactor.PanelInteractor
@@ -42,6 +45,7 @@ class IssueRecordingService 
@Inject

constructor(

    controller: RecordingController,

    @Background private val bgLooper: Looper,

    @LongRunning private val bgExecutor: Executor,

    @Main handler: Handler,

    uiEventLogger: UiEventLogger,
@@ -50,8 +54,8 @@ constructor( 
    keyguardDismissUtil: KeyguardDismissUtil,

    dialogTransitionAnimator: DialogTransitionAnimator,

    panelInteractor: PanelInteractor,

    traceurConnection: TraceurConnection,

    private val issueRecordingState: IssueRecordingState,

    traceurConnectionProvider: TraceurConnection.Provider,

    iActivityManager: IActivityManager,

) :

    RecordingService(
@@ -64,6 +68,8 @@ constructor( 
        keyguardDismissUtil,

    ) {



    private val traceurConnection: TraceurConnection = traceurConnectionProvider.create()



    private val session =

        IssueRecordingServiceSession(

            bgExecutor,
@@ -76,6 +82,23 @@ constructor( 
            userContextProvider,

        )



    /**

     * It is necessary to bind to IssueRecordingService from the Record Issue Tile because there are

     * instances where this service is not created in the same user profile as the record issue tile

     * aka, headless system user mode. In those instances, the TraceurConnection will be considered

     * a leak in between notification actions unless the tile is bound to this service to keep it

     * alive.

     */

    override fun onBind(intent: Intent): IBinder? {

        traceurConnection.doBind()

        return super.onBind(intent)

    }



    override fun onUnbind(intent: Intent?): Boolean {

        traceurConnection.doUnBind()

        return super.onUnbind(intent)

    }



    override fun getTag(): String = TAG



    override fun getChannelId(): String = CHANNEL_ID

packages/SystemUI/src/com/android/systemui/recordissue/IssueRecordingServiceConnection.kt

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

 * Copyright (C) 2024 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.recordissue



import android.content.Intent

import com.android.systemui.dagger.SysUISingleton

import com.android.systemui.settings.UserContextProvider

import com.android.traceur.MessageConstants.SYSTEM_UI_PACKAGE_NAME

import javax.inject.Inject



/**

 * It is necessary to bind to IssueRecordingService from the Record Issue Tile because there are

 * instances where this service is not created in the same user profile as the record issue tile

 * aka, headless system user mode. In those instances, the TraceurConnection will be considered a

 * leak in between notification actions unless the tile is bound to this service to keep it alive.

 */

class IssueRecordingServiceConnection(userContextProvider: UserContextProvider) :

    UserAwareConnection(

        userContextProvider,

        Intent().setClassName(SYSTEM_UI_PACKAGE_NAME, IssueRecordingService::class.java.name),

    ) {

    @SysUISingleton

    class Provider @Inject constructor(private val userContextProvider: UserContextProvider) {

        fun create() = IssueRecordingServiceConnection(userContextProvider)

    }

}