From 9effa5c8840b462f58a1844693776a3b7bfccb1b Mon Sep 17 00:00:00 2001 From: twyen Date: Tue, 5 Sep 2017 11:05:06 -0700 Subject: Ignore read missed calls and voicemails for notification When the call log is restored by the system, all items are marked as new even though they could already been read. This causes a torrent of "missed" calls after restoring call log with the setup wizard. This CL ignore read entities in the query. Bug: 62871863 Test: CallLogNotificationQueryHelperTest PiperOrigin-RevId: 167602820 Change-Id: Iba5609ace895a309685bfcd61eae85e080562ec5 --- .../dialer/app/calllog/CallLogNotificationsQueryHelper.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/java/com/android/dialer/app/calllog/CallLogNotificationsQueryHelper.java b/java/com/android/dialer/app/calllog/CallLogNotificationsQueryHelper.java index 43e03e9fd..c749b65ba 100644 --- a/java/com/android/dialer/app/calllog/CallLogNotificationsQueryHelper.java +++ b/java/com/android/dialer/app/calllog/CallLogNotificationsQueryHelper.java @@ -298,7 +298,13 @@ public class CallLogNotificationsQueryHelper { "no READ_CALL_LOG permission, returning null for calls lookup."); return null; } - final String selection = String.format("%s = 1 AND %s = ?", Calls.NEW, Calls.TYPE); + // A call is "new" when: + // NEW is 1. usually set when a new row is inserted + // TYPE matches the query type. + // IS_READ is not 1. A call might be backed up and restored, so it will be "new" to the + // call log, but the user has already read it on another device. + final String selection = + String.format("%s = 1 AND %s = ? AND %s IS NOT 1", Calls.NEW, Calls.TYPE, Calls.IS_READ); final String[] selectionArgs = new String[] {Integer.toString(type)}; try (Cursor cursor = mContentResolver.query( -- cgit v1.2.3 From 6a80c8282d5dcb3749573baa2f3769204859307a Mon Sep 17 00:00:00 2001 From: yueg Date: Tue, 5 Sep 2017 11:31:47 -0700 Subject: Fix bubble crash when dismiss audio route selector dialog. When AudioRouteSelectorDialogFragment tries to call onAudioRouteSelectorDismiss() on its parent AudioRouteSelectorActivity, the parent might already finish, which causes NPE. We should make sure the fragment is dismissed and removed before the activity finished. We do it when activity onPause because we don't expect it to resume. Test: AudioRouteSelectorDialogFragmentTest PiperOrigin-RevId: 167607068 Change-Id: Ifd2efcc92eb45262da2c6441bfac8119799d78f2 --- .../android/incallui/AudioRouteSelectorActivity.java | 18 +++++++++++++++++- .../audioroute/AudioRouteSelectorDialogFragment.java | 1 + 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/java/com/android/incallui/AudioRouteSelectorActivity.java b/java/com/android/incallui/AudioRouteSelectorActivity.java index dfd4d1abf..f0ae79bc2 100644 --- a/java/com/android/incallui/AudioRouteSelectorActivity.java +++ b/java/com/android/incallui/AudioRouteSelectorActivity.java @@ -32,7 +32,7 @@ public class AudioRouteSelectorActivity extends FragmentActivity protected void onCreate(@Nullable Bundle bundle) { super.onCreate(bundle); AudioRouteSelectorDialogFragment.newInstance(AudioModeProvider.getInstance().getAudioState()) - .show(getSupportFragmentManager(), null); + .show(getSupportFragmentManager(), AudioRouteSelectorDialogFragment.TAG); } @Override @@ -44,4 +44,20 @@ public class AudioRouteSelectorActivity extends FragmentActivity public void onAudioRouteSelectorDismiss() { finish(); } + + @Override + protected void onPause() { + super.onPause(); + AudioRouteSelectorDialogFragment audioRouteSelectorDialogFragment = + (AudioRouteSelectorDialogFragment) + getSupportFragmentManager().findFragmentByTag(AudioRouteSelectorDialogFragment.TAG); + // If Android back button is pressed, the fragment is dismissed and removed. If home button is + // pressed, we have to manually dismiss the fragment here. The fragment is also removed when + // dismissed. + if (audioRouteSelectorDialogFragment != null) { + audioRouteSelectorDialogFragment.dismiss(); + } + // We don't expect the activity to resume + finish(); + } } diff --git a/java/com/android/incallui/audioroute/AudioRouteSelectorDialogFragment.java b/java/com/android/incallui/audioroute/AudioRouteSelectorDialogFragment.java index c7a9d6332..860d2d282 100644 --- a/java/com/android/incallui/audioroute/AudioRouteSelectorDialogFragment.java +++ b/java/com/android/incallui/audioroute/AudioRouteSelectorDialogFragment.java @@ -37,6 +37,7 @@ import com.android.dialer.common.LogUtil; /** Shows picker for audio routes */ public class AudioRouteSelectorDialogFragment extends BottomSheetDialogFragment { + public static final String TAG = "AudioRouteSelectorDialogFragment"; private static final String ARG_AUDIO_STATE = "audio_state"; /** Called when an audio route is picked */ -- cgit v1.2.3 From 1d6cd6dd54251ee100e0f105bbcd299fbb609712 Mon Sep 17 00:00:00 2001 From: twyen Date: Tue, 5 Sep 2017 12:22:56 -0700 Subject: Load VVM config override before loading anything else The override config is used by carriers to test out different configs using a prod dialer. Previously the type was loaded before the override config, causing carriers that is not currently supported never able to force enable VVM. This CL also fixes getCarrierPackageNames() and getDisabledCapabilities() not loading the override. Bug: 65370543 Test: revived OmtpVvmCarrierConfigHelperTest PiperOrigin-RevId: 167614182 Change-Id: Ibafdaf5fdc3e948bc65656f94c4bdc7d6e97046c --- .../voicemail/impl/OmtpVvmCarrierConfigHelper.java | 29 +++++++++++++++------- .../impl/configui/ConfigOverrideFragment.java | 3 ++- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/java/com/android/voicemail/impl/OmtpVvmCarrierConfigHelper.java b/java/com/android/voicemail/impl/OmtpVvmCarrierConfigHelper.java index 700e1cbca..90303f59f 100644 --- a/java/com/android/voicemail/impl/OmtpVvmCarrierConfigHelper.java +++ b/java/com/android/voicemail/impl/OmtpVvmCarrierConfigHelper.java @@ -50,6 +50,8 @@ import java.util.Set; * that may clutter CarrierConfigManager too much. * *

The current hidden configs are: {@link #getSslPort()} {@link #getDisabledCapabilities()} + * + *

TODO(twyen): refactor this to an interface. */ @TargetApi(VERSION_CODES.O) public class OmtpVvmCarrierConfigHelper { @@ -112,19 +114,19 @@ public class OmtpVvmCarrierConfigHelper { return; } - mCarrierConfig = getCarrierConfig(telephonyManager); - mTelephonyConfig = - new TelephonyVvmConfigManager(context).getConfig(telephonyManager.getSimOperator()); - - mVvmType = getVvmType(); - mProtocol = VisualVoicemailProtocolFactory.create(mContext.getResources(), mVvmType); - if (ConfigOverrideFragment.isOverridden(context)) { mOverrideConfig = ConfigOverrideFragment.getConfig(context); VvmLog.w(TAG, "Config override is activated: " + mOverrideConfig); } else { mOverrideConfig = null; } + + mCarrierConfig = getCarrierConfig(telephonyManager); + mTelephonyConfig = + new TelephonyVvmConfigManager(context).getConfig(telephonyManager.getSimOperator()); + + mVvmType = getVvmType(); + mProtocol = VisualVoicemailProtocolFactory.create(mContext.getResources(), mVvmType); } @VisibleForTesting @@ -187,7 +189,11 @@ public class OmtpVvmCarrierConfigHelper { @Nullable public Set getCarrierVvmPackageNames() { Assert.checkArgument(isValid()); - Set names = getCarrierVvmPackageNames(mCarrierConfig); + Set names = getCarrierVvmPackageNames(mOverrideConfig); + if (names != null) { + return names; + } + names = getCarrierVvmPackageNames(mCarrierConfig); if (names != null) { return names; } @@ -278,7 +284,12 @@ public class OmtpVvmCarrierConfigHelper { @Nullable public Set getDisabledCapabilities() { Assert.checkArgument(isValid()); - Set disabledCapabilities = getDisabledCapabilities(mCarrierConfig); + Set disabledCapabilities; + disabledCapabilities = getDisabledCapabilities(mOverrideConfig); + if (disabledCapabilities != null) { + return disabledCapabilities; + } + disabledCapabilities = getDisabledCapabilities(mCarrierConfig); if (disabledCapabilities != null) { return disabledCapabilities; } diff --git a/java/com/android/voicemail/impl/configui/ConfigOverrideFragment.java b/java/com/android/voicemail/impl/configui/ConfigOverrideFragment.java index 1624ce579..caf33df13 100644 --- a/java/com/android/voicemail/impl/configui/ConfigOverrideFragment.java +++ b/java/com/android/voicemail/impl/configui/ConfigOverrideFragment.java @@ -49,7 +49,8 @@ public class ConfigOverrideFragment extends PreferenceFragment * Any preference with key that starts with this prefix will be written to the dialer carrier * config. */ - @VisibleForTesting static final String CONFIG_OVERRIDE_KEY_PREFIX = "vvm_config_override_key_"; + @VisibleForTesting + public static final String CONFIG_OVERRIDE_KEY_PREFIX = "vvm_config_override_key_"; @Override public void onCreate(@Nullable Bundle savedInstanceState) { -- cgit v1.2.3 From 2928203107c971ff8129f57addddb6b8fa19e065 Mon Sep 17 00:00:00 2001 From: wangqi Date: Tue, 5 Sep 2017 12:24:21 -0700 Subject: Strip trace info for release build. Each trace info will add several milliseconds overhead and it's not used by end user. Bug: 64542087 Test: none. PiperOrigin-RevId: 167614347 Change-Id: I70c032fde162d6c8aa46be72c9ece3b7c3249fe8 --- java/com/android/dialer/proguard/proguard_release.flags | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/java/com/android/dialer/proguard/proguard_release.flags b/java/com/android/dialer/proguard/proguard_release.flags index c6bdd490e..1429740f4 100644 --- a/java/com/android/dialer/proguard/proguard_release.flags +++ b/java/com/android/dialer/proguard/proguard_release.flags @@ -22,3 +22,9 @@ static *** v(...); static *** isLoggable(...); } + +# This allows proguard to strip Trace code from release builds. +-assumenosideeffects class android.os.Trace { + static *** beginSection(...); + static *** endSection(...); +} -- cgit v1.2.3 From 9abbf7ff9b31dd3420fd2b304b63703b87d843f6 Mon Sep 17 00:00:00 2001 From: mdooley Date: Tue, 5 Sep 2017 12:48:03 -0700 Subject: Fixing transcription crashes caused by job stoppage Apparently, scheduling a new job when one is already running (even using the enqueue api) causes the running job to be stopped. We weren't handling that case correctly. This cl makes sure no more work is attempted after a job is stopped by cancelling any active transcription task. We request that stopped task be rescheduled by the job scheduler, so it will get run eventually. I was able to verify this fix by sending a new voicemail while backfill old transcription tasks were running. Bug: 64908823,63524274,65129734,63803709 Test: manual and unit tests PiperOrigin-RevId: 167617191 Change-Id: Icc92997c2687e61bef9b3b7f9ff572da2cb4ed2e --- .../android/dialer/logging/dialer_impression.proto | 4 +++ .../impl/transcribe/TranscriptionService.java | 30 +++++++++++++++++----- .../impl/transcribe/TranscriptionTask.java | 28 +++++++++++++++++--- .../impl/transcribe/TranscriptionTaskAsync.java | 14 ++++++++-- 4 files changed, 65 insertions(+), 11 deletions(-) diff --git a/java/com/android/dialer/logging/dialer_impression.proto b/java/com/android/dialer/logging/dialer_impression.proto index ef249c262..94af6c3fd 100644 --- a/java/com/android/dialer/logging/dialer_impression.proto +++ b/java/com/android/dialer/logging/dialer_impression.proto @@ -530,5 +530,9 @@ message DialerImpression { IN_CALL_DIALPAD_NUMBER_BUTTON_PRESSED = 1265; IN_CALL_DIALPAD_HANG_UP_BUTTON_PRESSED = 1266; IN_CALL_DIALPAD_CLOSE_BUTTON_PRESSED = 1267; + + // More voicemail transcription impressions + VVM_TRANSCRIPTION_JOB_STOPPED = 1268; + VVM_TRANSCRIPTION_TASK_CANCELLED = 1269; } } diff --git a/java/com/android/voicemail/impl/transcribe/TranscriptionService.java b/java/com/android/voicemail/impl/transcribe/TranscriptionService.java index 2ca16fbf2..b733928d7 100644 --- a/java/com/android/voicemail/impl/transcribe/TranscriptionService.java +++ b/java/com/android/voicemail/impl/transcribe/TranscriptionService.java @@ -49,6 +49,8 @@ public class TranscriptionService extends JobService { private JobParameters jobParameters; private TranscriptionClientFactory clientFactory; private TranscriptionConfigProvider configProvider; + private TranscriptionTask activeTask; + private boolean stopped; /** Callback used by a task to indicate it has finished processing its work item */ interface JobCallback { @@ -134,8 +136,14 @@ public class TranscriptionService extends JobService { @MainThread public boolean onStopJob(JobParameters params) { Assert.isMainThread(); - LogUtil.enterBlock("TranscriptionService.onStopJob"); - cleanup(); + LogUtil.i("TranscriptionService.onStopJob", "params: " + params); + stopped = true; + Logger.get(this).logImpression(DialerImpression.Type.VVM_TRANSCRIPTION_JOB_STOPPED); + if (activeTask != null) { + LogUtil.i("TranscriptionService.onStopJob", "cancelling active task"); + activeTask.cancel(); + Logger.get(this).logImpression(DialerImpression.Type.VVM_TRANSCRIPTION_TASK_CANCELLED); + } return true; } @@ -161,15 +169,20 @@ public class TranscriptionService extends JobService { @MainThread private boolean checkForWork() { Assert.isMainThread(); + if (stopped) { + LogUtil.i("TranscriptionService.checkForWork", "stopped"); + return false; + } JobWorkItem workItem = jobParameters.dequeueWork(); if (workItem != null) { - TranscriptionTask task = + Assert.checkState(activeTask == null); + activeTask = configProvider.shouldUseSyncApi() ? new TranscriptionTaskSync( this, new Callback(), workItem, getClientFactory(), configProvider) : new TranscriptionTaskAsync( this, new Callback(), workItem, getClientFactory(), configProvider); - getExecutorService().execute(task); + getExecutorService().execute(activeTask); return true; } else { return false; @@ -196,8 +209,13 @@ public class TranscriptionService extends JobService { public void onWorkCompleted(JobWorkItem completedWorkItem) { Assert.isMainThread(); LogUtil.i("TranscriptionService.Callback.onWorkCompleted", completedWorkItem.toString()); - jobParameters.completeWork(completedWorkItem); - checkForWork(); + activeTask = null; + if (stopped) { + LogUtil.i("TranscriptionService.Callback.onWorkCompleted", "stopped"); + } else { + jobParameters.completeWork(completedWorkItem); + checkForWork(); + } } } diff --git a/java/com/android/voicemail/impl/transcribe/TranscriptionTask.java b/java/com/android/voicemail/impl/transcribe/TranscriptionTask.java index fbab07655..60b97dad5 100644 --- a/java/com/android/voicemail/impl/transcribe/TranscriptionTask.java +++ b/java/com/android/voicemail/impl/transcribe/TranscriptionTask.java @@ -19,8 +19,10 @@ import android.annotation.TargetApi; import android.app.job.JobWorkItem; import android.content.Context; import android.net.Uri; +import android.support.annotation.MainThread; import android.text.TextUtils; import android.util.Pair; +import com.android.dialer.common.Assert; import com.android.dialer.common.concurrent.ThreadUtil; import com.android.dialer.compat.android.provider.VoicemailCompat; import com.android.dialer.logging.DialerImpression; @@ -64,6 +66,7 @@ public abstract class TranscriptionTask implements Runnable { protected final TranscriptionConfigProvider configProvider; protected ByteString audioData; protected AudioFormat encoding; + protected volatile boolean cancelled; static final String AMR_PREFIX = "#!AMR\n"; @@ -87,6 +90,13 @@ public abstract class TranscriptionTask implements Runnable { databaseHelper = new TranscriptionDbHelper(context, voicemailUri); } + @MainThread + void cancel() { + Assert.isMainThread(); + VvmLog.i(TAG, "cancel"); + cancelled = true; + } + @Override public void run() { VvmLog.i(TAG, "run"); @@ -144,7 +154,11 @@ public abstract class TranscriptionTask implements Runnable { .logImpression(DialerImpression.Type.VVM_TRANSCRIPTION_RESPONSE_EXPIRED); break; default: - updateTranscriptionAndState(transcript, VoicemailCompat.TRANSCRIPTION_FAILED); + updateTranscriptionAndState( + transcript, + cancelled + ? VoicemailCompat.TRANSCRIPTION_NOT_STARTED + : VoicemailCompat.TRANSCRIPTION_FAILED); Logger.get(context).logImpression(DialerImpression.Type.VVM_TRANSCRIPTION_RESPONSE_EMPTY); break; } @@ -155,6 +169,11 @@ public abstract class TranscriptionTask implements Runnable { VvmLog.i(TAG, "sendRequest"); TranscriptionClient client = clientFactory.getClient(); for (int i = 0; i < configProvider.getMaxTranscriptionRetries(); i++) { + if (cancelled) { + VvmLog.i(TAG, "sendRequest, cancelled"); + return null; + } + VvmLog.i(TAG, "sendRequest, try: " + (i + 1)); if (i == 0) { Logger.get(context).logImpression(getRequestSentImpression()); @@ -163,7 +182,10 @@ public abstract class TranscriptionTask implements Runnable { } TranscriptionResponse response = request.getResponse(client); - if (response.hasRecoverableError()) { + if (cancelled) { + VvmLog.i(TAG, "sendRequest, cancelled"); + return null; + } else if (response.hasRecoverableError()) { Logger.get(context) .logImpression(DialerImpression.Type.VVM_TRANSCRIPTION_RESPONSE_RECOVERABLE_ERROR); backoff(i); @@ -187,7 +209,7 @@ public abstract class TranscriptionTask implements Runnable { try { Thread.sleep(millis); } catch (InterruptedException e) { - VvmLog.w(TAG, "interrupted"); + VvmLog.e(TAG, "interrupted", e); Thread.currentThread().interrupt(); } } diff --git a/java/com/android/voicemail/impl/transcribe/TranscriptionTaskAsync.java b/java/com/android/voicemail/impl/transcribe/TranscriptionTaskAsync.java index 3c41aef89..930d7f113 100644 --- a/java/com/android/voicemail/impl/transcribe/TranscriptionTaskAsync.java +++ b/java/com/android/voicemail/impl/transcribe/TranscriptionTaskAsync.java @@ -62,7 +62,10 @@ public class TranscriptionTaskAsync extends TranscriptionTask { (TranscriptionResponseAsync) sendRequest((client) -> client.sendUploadRequest(getUploadRequest())); - if (uploadResponse == null) { + if (cancelled) { + VvmLog.i(TAG, "getTranscription, cancelled."); + return new Pair<>(null, TranscriptionStatus.FAILED_NO_RETRY); + } else if (uploadResponse == null) { VvmLog.i(TAG, "getTranscription, failed to upload voicemail."); return new Pair<>(null, TranscriptionStatus.FAILED_NO_RETRY); } else { @@ -87,10 +90,17 @@ public class TranscriptionTaskAsync extends TranscriptionTask { VvmLog.i(TAG, "pollForTranscription"); GetTranscriptRequest request = getGetTranscriptRequest(uploadResponse); for (int i = 0; i < configProvider.getMaxGetTranscriptPolls(); i++) { + if (cancelled) { + VvmLog.i(TAG, "pollForTranscription, cancelled."); + return new Pair<>(null, TranscriptionStatus.FAILED_NO_RETRY); + } GetTranscriptResponseAsync response = (GetTranscriptResponseAsync) sendRequest((client) -> client.sendGetTranscriptRequest(request)); - if (response == null) { + if (cancelled) { + VvmLog.i(TAG, "pollForTranscription, cancelled."); + return new Pair<>(null, TranscriptionStatus.FAILED_NO_RETRY); + } else if (response == null) { VvmLog.i(TAG, "pollForTranscription, no transcription result."); } else if (response.isTranscribing()) { VvmLog.i(TAG, "pollForTranscription, poll count: " + (i + 1)); -- cgit v1.2.3