From 4d705e558d25a8fd810cb551c43c5a517cd1c2b9 Mon Sep 17 00:00:00 2001 From: wangqi Date: Thu, 28 Sep 2017 12:23:35 -0700 Subject: Fix crash on hanging up simulator video call. The problem is that InCallActivityCommon#onStop will try to commit any fragment which will crash. This change also fix a bug that simulatorRemoteVideo is not stopped after hanging up. Bug: 67045513 Test: manual PiperOrigin-RevId: 170380589 Change-Id: I25e0f43141eaa573189f4d9c0109fd2689c10374 --- java/com/android/dialer/logging/video_tech.proto | 19 ++++++++++ .../simulator/impl/SimulatorVideoProvider.java | 4 +++ java/com/android/incallui/InCallActivity.java | 2 +- java/com/android/incallui/call/DialerCall.java | 42 ++++++++-------------- java/com/android/incallui/videotech/VideoTech.java | 2 ++ .../incallui/videotech/empty/EmptyVideoTech.java | 5 +++ .../incallui/videotech/ims/ImsVideoTech.java | 5 +++ .../videotech/lightbringer/LightbringerTech.java | 5 +++ 8 files changed, 56 insertions(+), 28 deletions(-) create mode 100644 java/com/android/dialer/logging/video_tech.proto diff --git a/java/com/android/dialer/logging/video_tech.proto b/java/com/android/dialer/logging/video_tech.proto new file mode 100644 index 000000000..b5cd35145 --- /dev/null +++ b/java/com/android/dialer/logging/video_tech.proto @@ -0,0 +1,19 @@ +syntax = "proto2"; + +package com.android.dialer.logging; +option java_package = "com.android.dialer.logging"; +option java_multiple_files = true; +option optimize_for = LITE_RUNTIME; + + + + +message VideoTech { + enum Type { + NONE = 0; + + IMS_VIDEO_TECH = 1; + LIGHTBRINGER_VIDEO_TECH = 2; + RCS_VIDEO_SHARE = 3; + } +} \ No newline at end of file diff --git a/java/com/android/dialer/simulator/impl/SimulatorVideoProvider.java b/java/com/android/dialer/simulator/impl/SimulatorVideoProvider.java index a596728a8..0ed81da28 100644 --- a/java/com/android/dialer/simulator/impl/SimulatorVideoProvider.java +++ b/java/com/android/dialer/simulator/impl/SimulatorVideoProvider.java @@ -52,6 +52,10 @@ final class SimulatorVideoProvider extends Connection.VideoProvider { simulatorPreviewCamera.stopCamera(); simulatorPreviewCamera = null; } + if (previewCameraId == null && simulatorRemoteVideo != null) { + simulatorRemoteVideo.stopVideo(); + simulatorRemoteVideo = null; + } } @Override diff --git a/java/com/android/incallui/InCallActivity.java b/java/com/android/incallui/InCallActivity.java index 178a404ba..b82b6c994 100644 --- a/java/com/android/incallui/InCallActivity.java +++ b/java/com/android/incallui/InCallActivity.java @@ -200,9 +200,9 @@ public class InCallActivity extends TransactionSafeFragmentActivity protected void onStop() { Trace.beginSection("InCallActivity.onStop"); LogUtil.i("InCallActivity.onStop", ""); + isVisible = false; super.onStop(); common.onStop(); - isVisible = false; Trace.endSection(); } diff --git a/java/com/android/incallui/call/DialerCall.java b/java/com/android/incallui/call/DialerCall.java index 4f31dadf4..82a29ee84 100644 --- a/java/com/android/incallui/call/DialerCall.java +++ b/java/com/android/incallui/call/DialerCall.java @@ -153,9 +153,10 @@ public class DialerCall implements VideoTechListener, StateChangedListener, Capa // Times when a second call is received but AnswerAndRelease button is not shown // since it's not supported. private int secondCallWithoutAnswerAndReleasedButtonTimes = 0; - private VideoTech videoTech; + private VideoTech videoTech = new EmptyVideoTech(); - private boolean isImsReachable; + private com.android.dialer.logging.VideoTech.Type selectedAvailableVideoTechType = + com.android.dialer.logging.VideoTech.Type.NONE; public static String getNumberFromHandle(Uri handle) { return handle == null ? "" : handle.getSchemeSpecificPart(); @@ -451,10 +452,15 @@ public class DialerCall implements VideoTechListener, StateChangedListener, Capa private void update() { Trace.beginSection("DialerCall.update"); int oldState = getState(); - // Clear any cache here that could potentially change on update. - videoTech = null; // We want to potentially register a video call callback here. updateFromTelecomCall(); + // Only store the first video tech type found to be available during the life of the call. + if (selectedAvailableVideoTechType == com.android.dialer.logging.VideoTech.Type.NONE) { + // Update the video tech. + videoTech = mVideoTechManager.findBestAvailableVideoTech(); + videoTech.becomePrimary(); + selectedAvailableVideoTechType = videoTech.getVideoTechType(); + } if (oldState != getState() && getState() == DialerCall.State.DISCONNECTED) { for (DialerCallListener listener : mListeners) { listener.onDialerCallDisconnect(); @@ -1156,9 +1162,6 @@ public class DialerCall implements VideoTechListener, StateChangedListener, Capa } public VideoTech getVideoTech() { - if (videoTech == null) { - videoTech = mVideoTechManager.getVideoTech(); - } return videoTech; } @@ -1333,12 +1336,8 @@ public class DialerCall implements VideoTechListener, StateChangedListener, Capa mVideoTechManager.dispatchRemovedFromCallList(); } - public boolean isImsReachable() { - return isImsReachable; - } - - private void setImsReachable(boolean imsReachable) { - isImsReachable = imsReachable; + public com.android.dialer.logging.VideoTech.Type getSelectedAvailableVideoTechType() { + return selectedAvailableVideoTechType; } /** @@ -1535,7 +1534,6 @@ public class DialerCall implements VideoTechListener, StateChangedListener, Capa private final Context context; private final EmptyVideoTech emptyVideoTech = new EmptyVideoTech(); private final List videoTechs; - private VideoTech savedTech; VideoTechManager(DialerCall call) { this.context = call.mContext; @@ -1547,10 +1545,7 @@ public class DialerCall implements VideoTechListener, StateChangedListener, Capa // Insert order here determines the priority of that video tech option videoTechs = new ArrayList<>(); - ImsVideoTech imsVideoTech = - new ImsVideoTech(Logger.get(call.mContext), call, call.mTelecomCall); - call.setImsReachable(imsVideoTech.isAvailable(context)); - videoTechs.add(imsVideoTech); + videoTechs.add(new ImsVideoTech(Logger.get(call.mContext), call, call.mTelecomCall)); VideoTech rcsVideoTech = EnrichedCallComponent.get(call.mContext) @@ -1571,17 +1566,10 @@ public class DialerCall implements VideoTechListener, StateChangedListener, Capa phoneNumber)); } - VideoTech getVideoTech() { - if (savedTech != null) { - return savedTech; - } - + VideoTech findBestAvailableVideoTech() { for (VideoTech tech : videoTechs) { if (tech.isAvailable(context)) { - // Remember the first VideoTech that becomes available and always use it - savedTech = tech; - savedTech.becomePrimary(); - return savedTech; + return tech; } } diff --git a/java/com/android/incallui/videotech/VideoTech.java b/java/com/android/incallui/videotech/VideoTech.java index 410c35762..71087cb2c 100644 --- a/java/com/android/incallui/videotech/VideoTech.java +++ b/java/com/android/incallui/videotech/VideoTech.java @@ -85,6 +85,8 @@ public interface VideoTech { */ void becomePrimary(); + com.android.dialer.logging.VideoTech.Type getVideoTechType(); + /** Listener for video call events. */ interface VideoTechListener { diff --git a/java/com/android/incallui/videotech/empty/EmptyVideoTech.java b/java/com/android/incallui/videotech/empty/EmptyVideoTech.java index f156a14ad..2e34a6a8a 100644 --- a/java/com/android/incallui/videotech/empty/EmptyVideoTech.java +++ b/java/com/android/incallui/videotech/empty/EmptyVideoTech.java @@ -107,4 +107,9 @@ public class EmptyVideoTech implements VideoTech { @Override public void becomePrimary() {} + + @Override + public com.android.dialer.logging.VideoTech.Type getVideoTechType() { + return com.android.dialer.logging.VideoTech.Type.NONE; + } } diff --git a/java/com/android/incallui/videotech/ims/ImsVideoTech.java b/java/com/android/incallui/videotech/ims/ImsVideoTech.java index 0ef07d667..0310a90ac 100644 --- a/java/com/android/incallui/videotech/ims/ImsVideoTech.java +++ b/java/com/android/incallui/videotech/ims/ImsVideoTech.java @@ -327,6 +327,11 @@ public class ImsVideoTech implements VideoTech { DialerImpression.Type.UPGRADE_TO_VIDEO_CALL_BUTTON_SHOWN_FOR_IMS); } + @Override + public com.android.dialer.logging.VideoTech.Type getVideoTechType() { + return com.android.dialer.logging.VideoTech.Type.IMS_VIDEO_TECH; + } + private boolean canPause() { return call.getDetails().can(Details.CAPABILITY_CAN_PAUSE_VIDEO); } diff --git a/java/com/android/incallui/videotech/lightbringer/LightbringerTech.java b/java/com/android/incallui/videotech/lightbringer/LightbringerTech.java index 1ddf3716e..9adc53b0d 100644 --- a/java/com/android/incallui/videotech/lightbringer/LightbringerTech.java +++ b/java/com/android/incallui/videotech/lightbringer/LightbringerTech.java @@ -195,4 +195,9 @@ public class LightbringerTech implements VideoTech, LightbringerListener { public void onLightbringerStateChanged() { listener.onVideoTechStateChanged(); } + + @Override + public com.android.dialer.logging.VideoTech.Type getVideoTechType() { + return com.android.dialer.logging.VideoTech.Type.LIGHTBRINGER_VIDEO_TECH; + } } -- cgit v1.2.3 From 90b570e4f427f14fd17d53561c209d5eb1ac1936 Mon Sep 17 00:00:00 2001 From: yueg Date: Thu, 28 Sep 2017 15:06:58 -0700 Subject: Fix audio route selector. 1. Use onCancel() instead of onDismiss(). onCancel() is not called when changing orientation, so it won't crash on getParentUnsafe(). onCancel() is also not called when pressing home button, but it will be handled by AudioRouteSelectorActivity.onPause(). b/67013452 will happen after the fix. 2. Use FLAG_ACTIVITY_NEW_TASK and FLAG_ACTIVITY_MULTIPLE_TASK when starting AudioRouteSelectorActivity. This prevents showing Dialer with AudioRouteSelectorActivity. The downside is that the activity is no longer excluded from recent screen. The two fixes can affect each other, so they are in one CL. Video: https://drive.google.com/open?id=0Bz1rQbdSCWSKYVJkd3R1SkI4c3c Test: manual PiperOrigin-RevId: 170404203 Change-Id: Ifa8ebcd566670115d3865b0d67c311c296fbbd51 --- .../android/incallui/AudioRouteSelectorActivity.java | 6 ++++-- .../android/incallui/ReturnToCallActionReceiver.java | 8 +++++++- .../audioroute/AudioRouteSelectorDialogFragment.java | 20 +++++++++----------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/java/com/android/incallui/AudioRouteSelectorActivity.java b/java/com/android/incallui/AudioRouteSelectorActivity.java index f0ae79bc2..2fdc89aa8 100644 --- a/java/com/android/incallui/AudioRouteSelectorActivity.java +++ b/java/com/android/incallui/AudioRouteSelectorActivity.java @@ -57,7 +57,9 @@ public class AudioRouteSelectorActivity extends FragmentActivity if (audioRouteSelectorDialogFragment != null) { audioRouteSelectorDialogFragment.dismiss(); } - // We don't expect the activity to resume - finish(); + // We don't expect the activity to resume, except for orientation change. + if (!isChangingConfigurations()) { + finish(); + } } } diff --git a/java/com/android/incallui/ReturnToCallActionReceiver.java b/java/com/android/incallui/ReturnToCallActionReceiver.java index b645c155c..c37b0b816 100644 --- a/java/com/android/incallui/ReturnToCallActionReceiver.java +++ b/java/com/android/incallui/ReturnToCallActionReceiver.java @@ -20,6 +20,7 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.telecom.CallAudioState; +import com.android.dialer.common.Assert; import com.android.dialer.common.LogUtil; import com.android.dialer.logging.DialerImpression; import com.android.dialer.logging.Logger; @@ -51,6 +52,9 @@ public class ReturnToCallActionReceiver extends BroadcastReceiver { case ACTION_END_CALL: endCall(context); break; + default: + throw Assert.createIllegalStateFailException( + "Invalid intent action: " + intent.getAction()); } } @@ -87,7 +91,9 @@ public class ReturnToCallActionReceiver extends BroadcastReceiver { } public void showAudioRouteSelector(Context context) { - context.startActivity(new Intent(context, AudioRouteSelectorActivity.class)); + Intent intent = new Intent(context, AudioRouteSelectorActivity.class); + intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK | Intent.FLAG_ACTIVITY_MULTIPLE_TASK); + context.startActivity(intent); } private void toggleMute(Context context) { diff --git a/java/com/android/incallui/audioroute/AudioRouteSelectorDialogFragment.java b/java/com/android/incallui/audioroute/AudioRouteSelectorDialogFragment.java index 860d2d282..9fd3aed5b 100644 --- a/java/com/android/incallui/audioroute/AudioRouteSelectorDialogFragment.java +++ b/java/com/android/incallui/audioroute/AudioRouteSelectorDialogFragment.java @@ -27,7 +27,6 @@ import android.support.design.widget.BottomSheetDialogFragment; import android.telecom.CallAudioState; import android.view.LayoutInflater; import android.view.View; -import android.view.View.OnClickListener; import android.view.ViewGroup; import android.view.WindowManager; import android.widget.TextView; @@ -92,12 +91,14 @@ public class AudioRouteSelectorDialogFragment extends BottomSheetDialogFragment (TextView) view.findViewById(R.id.audioroute_earpiece), CallAudioState.ROUTE_EARPIECE, audioState); + + // TODO(b/67013452): set peak height correctly to fully expand it in landscape mode. return view; } @Override - public void onDismiss(DialogInterface dialogInterface) { - super.onDismiss(dialogInterface); + public void onCancel(DialogInterface dialogInterface) { + super.onCancel(dialogInterface); FragmentUtils.getParentUnsafe( AudioRouteSelectorDialogFragment.this, AudioRouteSelectorPresenter.class) .onAudioRouteSelectorDismiss(); @@ -113,14 +114,11 @@ public class AudioRouteSelectorDialogFragment extends BottomSheetDialogFragment item.setCompoundDrawableTintMode(Mode.SRC_ATOP); } item.setOnClickListener( - new OnClickListener() { - @Override - public void onClick(View v) { - dismiss(); - FragmentUtils.getParentUnsafe( - AudioRouteSelectorDialogFragment.this, AudioRouteSelectorPresenter.class) - .onAudioRouteSelected(itemRoute); - } + (v) -> { + dismiss(); + FragmentUtils.getParentUnsafe( + AudioRouteSelectorDialogFragment.this, AudioRouteSelectorPresenter.class) + .onAudioRouteSelected(itemRoute); }); } } -- cgit v1.2.3