From bd78991bc6a1319c97172a53fd1c0bddb3200332 Mon Sep 17 00:00:00 2001
From: David Chen <dwchen@google.com>
Date: Fri, 16 Mar 2018 17:19:55 -0700
Subject: [PATCH] Moves the settings changed logging for statsd.

Previously, we wrote a log entry regardless of permission checks, so
the logging could be misleading. Now we only send the log to statsd
after verifying that this setting mutation is valid.

Test: Flashed onto marlin-eng and verified stats-log as expected.
Bug: 73493944
Change-Id: I2a8b052aa8c380ffc5d15caec089fffcdc5823f4
---
 cmds/statsd/src/atoms.proto                            | 10 ++++++++--
 core/java/android/provider/Settings.java               |  5 -----
 .../com/android/providers/settings/SettingsState.java  |  8 ++++++++
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/cmds/statsd/src/atoms.proto b/cmds/statsd/src/atoms.proto
index 31ca13a1585c7..8d3693633631f 100644
--- a/cmds/statsd/src/atoms.proto
+++ b/cmds/statsd/src/atoms.proto
@@ -1098,7 +1098,7 @@ message PhoneSignalStrengthChanged {
 /**
  * Logs that a setting was updated.
  * Logged from:
- *   frameworks/base/core/java/android/provider/Settings.java
+ *   frameworks/base/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
  * The tag and is_default allow resetting of settings to default values based on the specified
  * tag. See Settings#putString(ContentResolver, String, String, String, boolean) for more details.
  */
@@ -1124,8 +1124,14 @@ message SettingChanged {
     // True if this setting with tag should be resettable.
     optional bool is_default = 6;
 
-    // The user ID associated. Defined in android/os/UserHandle.java
+    // The associated user (for multi-user feature). Defined in android/os/UserHandle.java
     optional int32 user = 7;
+
+    enum ChangeReason {
+        UPDATED = 1; // Updated can be an insertion or an update.
+        DELETED = 2;
+    }
+    optional ChangeReason reason = 8;
 }
 
 /**
diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java
index 70615c8c19159..df80e45436cc4 100644
--- a/core/java/android/provider/Settings.java
+++ b/core/java/android/provider/Settings.java
@@ -83,7 +83,6 @@ import android.util.ArrayMap;
 import android.util.ArraySet;
 import android.util.Log;
 import android.util.MemoryIntArray;
-import android.util.StatsLog;
 import android.view.textservice.TextServicesManager;
 
 import com.android.internal.annotations.GuardedBy;
@@ -1914,11 +1913,7 @@ public final class Settings {
                     arg.putBoolean(CALL_METHOD_MAKE_DEFAULT_KEY, true);
                 }
                 IContentProvider cp = mProviderHolder.getProvider(cr);
-                String prevValue = getStringForUser(cr, name, userHandle);
                 cp.call(cr.getPackageName(), mCallSetCommand, name, arg);
-                String newValue = getStringForUser(cr, name, userHandle);
-                StatsLog.write(StatsLog.SETTING_CHANGED, name, value, newValue, prevValue, tag,
-                        makeDefault, userHandle);
             } catch (RemoteException e) {
                 Log.w(TAG, "Can't set key " + name + " in " + mUri, e);
                 return false;
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
index 6d341c5607e25..449946d7ab156 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
@@ -42,6 +42,7 @@ import android.util.AtomicFile;
 import android.util.Base64;
 import android.util.Slog;
 import android.util.SparseIntArray;
+import android.util.StatsLog;
 import android.util.TimeUtils;
 import android.util.Xml;
 import android.util.proto.ProtoOutputStream;
@@ -386,6 +387,9 @@ final class SettingsState {
             mSettings.put(name, newState);
         }
 
+        StatsLog.write(StatsLog.SETTING_CHANGED, name, value, newState.value, oldValue, tag,
+            makeDefault, getUserIdFromKey(mKey), StatsLog.SETTING_CHANGED__REASON__UPDATED);
+
         addHistoricalOperationLocked(HISTORICAL_OPERATION_UPDATE, newState);
 
         updateMemoryUsagePerPackageLocked(packageName, oldValue, value,
@@ -410,6 +414,10 @@ final class SettingsState {
 
         Setting oldState = mSettings.remove(name);
 
+        StatsLog.write(StatsLog.SETTING_CHANGED, name, /* value= */ "", /* newValue= */ "",
+            oldState.value, /* tag */ "", false, getUserIdFromKey(mKey),
+            StatsLog.SETTING_CHANGED__REASON__DELETED);
+
         updateMemoryUsagePerPackageLocked(oldState.packageName, oldState.value,
                 null, oldState.defaultValue, null);