Guard against a null time when recording query duration

Bug: 1116927
Change-Id: Ie0603c81108b236f7f4356559cef7759219ea00f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359566Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799745}

chrome/android/java/src/org/chromium/chrome/browser/omnibox/voice/VoiceRecognitionHandler.java

@@ -200,8 +200,6 @@ public class VoiceRecognitionHandler {
         // WindowAndroid.IntentCallback implementation:
         @Override
         public void onIntentCompleted(WindowAndroid window, int resultCode, Intent data) {
-            stopTrackingAndRecordQueryDuration();
-
             if (resultCode != Activity.RESULT_OK || data.getExtras() == null) {
                 if (resultCode == Activity.RESULT_CANCELED) {
                     recordVoiceSearchDismissedEventSource(mSource);
@@ -211,6 +209,8 @@ public class VoiceRecognitionHandler {
                 return;
             }
 
+            // Only record query duration on success.
+            stopTrackingAndRecordQueryDuration();
             AutocompleteCoordinator autocompleteCoordinator =
                     mDelegate.getAutocompleteCoordinator();
             assert autocompleteCoordinator != null;
@@ -423,13 +423,15 @@ public class VoiceRecognitionHandler {
 
     /** Start tracking query duration by capturing when it started */
     private void startTrackingQueryDuration() {
-        assert mQueryStartTimeMs == null;
         mQueryStartTimeMs = SystemClock.elapsedRealtime();
     }
 
     /** Calculate the query duration and report it and cleanup afterwards. */
-    private void stopTrackingAndRecordQueryDuration() {
-        assert mQueryStartTimeMs != null;
+    @VisibleForTesting
+    void stopTrackingAndRecordQueryDuration() {
+        // Defensive check to guard against onIntentResult being called more than once. This only
+        // happens with assistant experiments. See crbug.com/1116927 for details.
+        if (mQueryStartTimeMs == null) return;
 
         long elapsedTimeMs = SystemClock.elapsedRealtime() - mQueryStartTimeMs;
         recordVoiceSearchOpenDuration(elapsedTimeMs);
@@ -519,4 +521,9 @@ public class VoiceRecognitionHandler {
     protected boolean isRecognitionIntentPresent(boolean useCachedValue) {
         return VoiceRecognitionUtil.isRecognitionIntentPresent(useCachedValue);
     }
+
+    /** Sets the start time for testing. */
+    void setQueryStartTimeForTesting(Long queryStartTimeMs) {
+        mQueryStartTimeMs = queryStartTimeMs;
+    }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/voice/VoiceRecognitionHandlerTest.java

@@ -614,7 +614,7 @@ public class VoiceRecognitionHandlerTest {
         Assert.assertEquals(null, mHandler.getVoiceSearchResult());
         Assert.assertEquals(
                 VoiceInteractionSource.NTP, mHandler.getVoiceSearchFailureEventSource());
-        Assert.assertEquals(1,
+        Assert.assertEquals(0,
                 RecordHistogram.getHistogramTotalCountForTesting(
                         "VoiceInteraction.QueryDuration.Android"));
     }
@@ -628,7 +628,7 @@ public class VoiceRecognitionHandlerTest {
         Assert.assertEquals(null, mHandler.getVoiceSearchResult());
         Assert.assertEquals(
                 VoiceInteractionSource.NTP, mHandler.getVoiceSearchDismissedEventSource());
-        Assert.assertEquals(1,
+        Assert.assertEquals(0,
                 RecordHistogram.getHistogramTotalCountForTesting(
                         "VoiceInteraction.QueryDuration.Android"));
     }
@@ -752,6 +752,26 @@ public class VoiceRecognitionHandlerTest {
         });
     }
 
+    @Test
+    @SmallTest
+    public void teststopTrackingAndRecordQueryDuration() {
+        mHandler.setQueryStartTimeForTesting(100L);
+        mHandler.stopTrackingAndRecordQueryDuration();
+        Assert.assertEquals(1,
+                RecordHistogram.getHistogramTotalCountForTesting(
+                        "VoiceInteraction.QueryDuration.Android"));
+    }
+
+    @Test
+    @SmallTest
+    public void teststopTrackingAndRecordQueryDuration_calledWithNull() {
+        mHandler.setQueryStartTimeForTesting(null);
+        mHandler.stopTrackingAndRecordQueryDuration();
+        Assert.assertEquals(0,
+                RecordHistogram.getHistogramTotalCountForTesting(
+                        "VoiceInteraction.QueryDuration.Android"));
+    }
+
     private static Bundle createDummyBundle(String text, float confidence) {
         return createDummyBundle(new String[] {text}, new float[] {confidence});
     }