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

Commit a64e9ac0 authored by Tyler Gunn's avatar Tyler Gunn
Browse files

Fix holding behavior across self-manged ConnectionServices. 

- CallsManager#holdActiveCallForNewCall - make logs more concise.
- CallsManager#makeRoomForOutgoingCall - there is behavior which assumes
that if a new call is added by the SAME connection service that we will
let the ConnectionService worry about hold/unhold on its own.  That is a
legacy behavior from Telephony and really doesn't make sense for VOIP
apps.  For VOIP apps we SHOULD auto hold the ongoing call when a new
outgoing call is added.  This was not noticed in the past because the focus
tracker was doing this in some cases so it wasn't obvious.  However, even
then the signalling was not optimal as you could have an ACTIVE and
DIALING call, when realistically as soon as the new dialing call is added
it should hold the old active call.
- CSFocusManager - add local log of focus call to dumpsys to make tracking
focus issues easier in dumpsys.
- Updated the test app to include another connection service which was
useful for reproing this issue.

Test: Manual tests with test app; make calls across different cs in same
app.
Test: Added new unit test coverage for these cases.
Fixes: 279669173

Change-Id: If7d8e9f7280ce1d78b16a0d3b2f0b50b1063a4e6
parent 2ab0678e

src/com/android/server/telecom/CallsManager.java

+28 −7
Original line number Diff line number Diff line
@@ -3412,10 +3412,11 @@ public class CallsManager extends Call.ListenerBase 
     */

    boolean holdActiveCallForNewCall(Call call) {

        Call activeCall = (Call) mConnectionSvrFocusMgr.getCurrentFocusCall();

        Log.i(this, "holdActiveCallForNewCall, newCall: %s, activeCall: %s", call, activeCall);

        Log.i(this, "holdActiveCallForNewCall, newCall: %s, activeCall: %s", call.getId(),

                (activeCall == null ? "<none>" : activeCall.getId()));

        if (activeCall != null && activeCall != call) {

            if (canHold(activeCall)) {

                activeCall.hold();

                activeCall.hold("swap to " + call.getId());

                return true;

            } else if (supportsHold(activeCall)

                    && areFromSameSource(activeCall, call)) {
@@ -4900,12 +4901,25 @@ public class CallsManager extends Call.ListenerBase 
                    liveCallPhoneAccount);

        }



        // First thing, if we are trying to make a call with the same phone account as the live

        // call, then allow it so that the connection service can make its own decision about

        // how to handle the new call relative to the current one.

        // First thing, for managed calls, if we are trying to make a call with the same phone

        // account as the live call, then allow it so that the connection service can make its own

        // decision about how to handle the new call relative to the current one.

        // Note: This behavior is primarily in place because Telephony historically manages the

        // state of the calls it tracks by itself, holding and unholding as needed.  Self-managed

        // calls, even though from the same package are normally held/unheld automatically by

        // Telecom.  Calls within a single ConnectionService get held/unheld automatically during

        // "swap" operations by CallsManager#holdActiveCallForNewCall.  There is, however, a quirk

        // in that if an app declares TWO different ConnectionServices, holdActiveCallForNewCall

        // would not work correctly because focus switches between ConnectionServices, yet we

        // tended to assume that if the calls are from the same package that the hold/unhold should

        // be done by the app.  That was a bad assumption as it meant that we could have two active

        // calls.

        // TODO(b/280826075): We need to come back and revisit all this logic in a holistic manner.

        if (PhoneAccountHandle.areFromSamePackage(liveCallPhoneAccount,

                call.getTargetPhoneAccount())) {

            Log.i(this, "makeRoomForOutgoingCall: phoneAccount matches.");

                call.getTargetPhoneAccount())

                && !call.isSelfManaged()

                && !liveCall.isSelfManaged()) {

            Log.i(this, "makeRoomForOutgoingCall: managed phoneAccount matches");

            call.getAnalytics().setCallIsAdditional(true);

            liveCall.getAnalytics().setCallIsInterrupted(true);

            return true;
@@ -5485,6 +5499,13 @@ public class CallsManager extends Call.ListenerBase 
            impl.dump(pw);

            pw.decreaseIndent();

        }



        if (mConnectionSvrFocusMgr != null) {

            pw.println("mConnectionSvrFocusMgr:");

            pw.increaseIndent();

            mConnectionSvrFocusMgr.dump(pw);

            pw.decreaseIndent();

        }

    }



    /**

src/com/android/server/telecom/ConnectionServiceFocusManager.java

+21 −2
Original line number Diff line number Diff line
@@ -25,8 +25,10 @@ import android.os.Message; 
import android.telecom.Log;

import android.telecom.Logging.Session;

import android.text.TextUtils;

import android.util.LocalLog;



import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.util.IndentingPrintWriter;



import java.util.ArrayList;

import java.util.List;
@@ -41,6 +43,7 @@ import java.util.stream.Collectors; 
public class ConnectionServiceFocusManager {

    private static final String TAG = "ConnectionSvrFocusMgr";

    private static final int GET_CURRENT_FOCUS_TIMEOUT_MILLIS = 1000;

    private final LocalLog mLocalLog = new LocalLog(20);



    /** Factory interface used to create the {@link ConnectionServiceFocusManager} instance. */

    public interface ConnectionServiceFocusManagerFactory {
@@ -124,6 +127,11 @@ public class ConnectionServiceFocusManager { 
         * @return {@code True} if this call can receive focus, {@code false} otherwise.

         */

        boolean isFocusable();



        /**

         * @return the ID of the focusable for debug purposes.

         */

        String getId();

    }



    /** Interface define a call back for focus request event. */
@@ -361,10 +369,11 @@ public class ConnectionServiceFocusManager { 
    }



    private void updateCurrentFocusCall() {

        CallFocus previousFocus = mCurrentFocusCall;

        mCurrentFocusCall = null;



        if (mCurrentFocus == null) {

            Log.d(this, "updateCurrentFocusCall: mCurrentFocus is null");

            Log.i(this, "updateCurrentFocusCall: mCurrentFocus is null");

            return;

        }
@@ -377,11 +386,16 @@ public class ConnectionServiceFocusManager { 
        for (CallFocus call : calls) {

            if (PRIORITY_FOCUS_CALL_STATE.contains(call.getState())) {

                mCurrentFocusCall = call;

                if (previousFocus != call) {

                    mLocalLog.log(call.getId());

                }

                Log.i(this, "updateCurrentFocusCall %s", mCurrentFocusCall);

                return;

            }

        }



        if (previousFocus != null) {

            mLocalLog.log("<none>");

        }

        Log.i(this, "updateCurrentFocusCall = null");

    }
@@ -477,6 +491,11 @@ public class ConnectionServiceFocusManager { 
        }

    }



    public void dump(IndentingPrintWriter pw) {

        pw.println("Call Focus History:");

        mLocalLog.dump(pw);

    }



    private final class FocusManagerHandler extends Handler {

        FocusManagerHandler(Looper looper) {

            super(looper);

testapps/AndroidManifest.xml

+9 −0
Original line number Diff line number Diff line
@@ -259,6 +259,15 @@ 
          </intent-filter>

        </service>



        <service android:name="com.android.server.telecom.testapps.OtherSelfManagedConnectionService"

                 android:permission="android.permission.BIND_TELECOM_CONNECTION_SERVICE"

                 android:process="com.android.server.telecom.testapps.SelfMangingCallingApp"

                 android:exported="true">

            <intent-filter>

                <action android:name="android.telecom.ConnectionService"/>

            </intent-filter>

        </service>



        <receiver android:exported="false"

             android:process="com.android.server.telecom.testapps.SelfMangingCallingApp"

             android:name="com.android.server.telecom.testapps.SelfManagedCallNotificationReceiver"/>

testapps/res/layout/self_managed_sample_main.xml

+6 −0
Original line number Diff line number Diff line
@@ -55,6 +55,12 @@ 
                android:layout_height="wrap_content"

                android:background="@color/test_call_b_color"

                android:text="2"/>

            <RadioButton

                android:id="@+id/useAcct3Button"

                android:layout_width="wrap_content"

                android:layout_height="wrap_content"

                android:background="@color/test_call_c_color"

                android:text="3"/>

        </RadioGroup>

        <TextView

            android:id="@+id/hasFocus"

testapps/res/values/colors.xml

+1 −0
Original line number Diff line number Diff line
@@ -17,4 +17,5 @@ 
<resources>

    <color name="test_call_a_color">#f2eebf</color>

    <color name="test_call_b_color">#afc5e6</color>

    <color name="test_call_c_color">#c5afe6</color>

</resources>