Merge "Remove some duplication of validation logic"

services/core/java/com/android/server/timedetector/TimeDetectorStrategyImpl.java

@@ -114,16 +114,10 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
     }
 
     @Override
-    public synchronized void suggestManualTime(ManualTimeSuggestion suggestion) {
+    public synchronized void suggestManualTime(@NonNull ManualTimeSuggestion suggestion) {
         final TimestampedValue<Long> newUtcTime = suggestion.getUtcTime();
 
-        // We can validate the suggestion against the reference time clock.
+        if (!validateSuggestionTime(newUtcTime, suggestion)) {
-        long elapsedRealtimeMillis = mCallback.elapsedRealtimeMillis();
-        if (elapsedRealtimeMillis < newUtcTime.getReferenceTimeMillis()) {
-            // elapsedRealtime clock went backwards?
-            Slog.w(LOG_TAG, "New reference time is in the future? Ignoring."
-                    + " elapsedRealtimeMillis=" + elapsedRealtimeMillis
-                    + ", timeSuggestion=" + suggestion);
             return;
         }
 
@@ -202,62 +196,76 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
     }
 
     @GuardedBy("this")
-    private boolean validateAndStorePhoneSuggestion(@NonNull PhoneTimeSuggestion timeSuggestion) {
+    private boolean validateAndStorePhoneSuggestion(@NonNull PhoneTimeSuggestion suggestion) {
-        if (timeSuggestion.getUtcTime().getValue() == null) {
+        TimestampedValue<Long> newUtcTime = suggestion.getUtcTime();
-            Slog.w(LOG_TAG, "Suggestion utcTime contains null value"
+        if (!validateSuggestionTime(newUtcTime, suggestion)) {
-                    + " timeSuggestion=" + timeSuggestion);
+            // There's probably nothing useful we can do: elsewhere we assume that reference
+            // times are in the past so just stop here.
             return false;
         }
 
-        int phoneId = timeSuggestion.getPhoneId();
+        int phoneId = suggestion.getPhoneId();
         LinkedList<PhoneTimeSuggestion> phoneSuggestions = mSuggestionByPhoneId.get(phoneId);
         if (phoneSuggestions == null) {
             // The first time we've seen this phoneId.
             phoneSuggestions = new LinkedList<>();
             mSuggestionByPhoneId.put(phoneId, phoneSuggestions);
         } else if (phoneSuggestions.isEmpty()) {
-            Slog.w(LOG_TAG, "Suggestions unexpectedly empty when adding"
+            Slog.w(LOG_TAG, "Suggestions unexpectedly empty when adding suggestion=" + suggestion);
-                    + " timeSuggestion=" + timeSuggestion);
         }
 
         if (!phoneSuggestions.isEmpty()) {
-            PhoneTimeSuggestion previousSuggestion = phoneSuggestions.getFirst();
-
             // We can log / discard suggestions with obvious issues with the reference time clock.
-            long elapsedRealtimeMillis = mCallback.elapsedRealtimeMillis();
+            PhoneTimeSuggestion previousSuggestion = phoneSuggestions.getFirst();
-            TimestampedValue<Long> newTime = timeSuggestion.getUtcTime();
+            if (previousSuggestion == null
-            if (elapsedRealtimeMillis < newTime.getReferenceTimeMillis()) {
+                    || previousSuggestion.getUtcTime() == null
-                // elapsedRealtime clock went backwards?
+                    || previousSuggestion.getUtcTime().getValue() == null) {
-                Slog.w(LOG_TAG, "New reference time is in the future?"
+                // This should be impossible given we only store validated suggestions.
-                        + " elapsedRealtimeMillis=" + elapsedRealtimeMillis
+                Slog.w(LOG_TAG, "Previous suggestion is null or has a null time."
-                        + ", timeSuggestion=" + timeSuggestion);
+                        + " previousSuggestion=" + previousSuggestion
-                // There's probably nothing useful we can do: elsewhere we assume that reference
+                        + ", suggestion=" + suggestion);
-                // times are in the past so just stop here.
                 return false;
             }
 
-            if (previousSuggestion.getUtcTime() != null) {
+            long referenceTimeDifference = TimestampedValue.referenceTimeDifference(
-                long referenceTimeDifference = TimestampedValue.referenceTimeDifference(
+                    newUtcTime, previousSuggestion.getUtcTime());
-                        timeSuggestion.getUtcTime(), previousSuggestion.getUtcTime());
+            if (referenceTimeDifference < 0) {
-                if (referenceTimeDifference < 0) {
+                // The reference time is before the previously received suggestion. Ignore it.
-                    // The reference time is before the previously received suggestion. Ignore it.
+                Slog.w(LOG_TAG, "Out of order phone suggestion received."
-                    Slog.w(LOG_TAG, "Out of order phone suggestion received."
+                        + " referenceTimeDifference=" + referenceTimeDifference
-                            + " referenceTimeDifference=" + referenceTimeDifference
+                        + " previousSuggestion=" + previousSuggestion
-                            + " lastSuggestion=" + previousSuggestion
+                        + " suggestion=" + suggestion);
-                            + " newSuggestion=" + timeSuggestion);
+                return false;
-                    return false;
-                }
             }
         }
 
         // Store the latest suggestion.
-        phoneSuggestions.addFirst(timeSuggestion);
+        phoneSuggestions.addFirst(suggestion);
         if (phoneSuggestions.size() > KEEP_SUGGESTION_HISTORY_SIZE) {
             phoneSuggestions.removeLast();
         }
         return true;
     }
 
+    private boolean validateSuggestionTime(
+            @NonNull TimestampedValue<Long> newUtcTime, @NonNull Object suggestion) {
+        if (newUtcTime.getValue() == null) {
+            Slog.w(LOG_TAG, "Suggested time value is null. suggestion=" + suggestion);
+            return false;
+        }
+
+        // We can validate the suggestion against the reference time clock.
+        long elapsedRealtimeMillis = mCallback.elapsedRealtimeMillis();
+        if (elapsedRealtimeMillis < newUtcTime.getReferenceTimeMillis()) {
+            // elapsedRealtime clock went backwards?
+            Slog.w(LOG_TAG, "New reference time is in the future? Ignoring."
+                    + " elapsedRealtimeMillis=" + elapsedRealtimeMillis
+                    + ", suggestion=" + suggestion);
+            return false;
+        }
+        return true;
+    }
+
     @GuardedBy("this")
     private void doAutoTimeDetection(@NonNull String detectionReason) {
         if (!mCallback.isAutoTimeDetectionEnabled()) {