Update activeConfigsChangedBroadcast
avoid using intentsender in #sendActiveConfigsChangedBroadcast
and #removeActiveConfigsChangedBroadcast.
Bug: 146074295
Test: Ran GTS Tests
Change-Id: I9313299ea0bc89f092b1c62fbfc34e06a127eaa9
diff --git a/apex/statsd/aidl/android/os/IStatsCompanionService.aidl b/apex/statsd/aidl/android/os/IStatsCompanionService.aidl
index ec3a226..32413e2 100644
--- a/apex/statsd/aidl/android/os/IStatsCompanionService.aidl
+++ b/apex/statsd/aidl/android/os/IStatsCompanionService.aidl
@@ -67,12 +67,6 @@
StatsLogEventWrapper[] pullData(int pullCode);
/**
- * Send a broadcast to the specified PendingIntent's as IBinder notifying it that the list
- * of active configs has changed.
- */
- oneway void sendActiveConfigsChangedBroadcast(in IBinder intentSender, in long[] configIds);
-
- /**
* Requests StatsCompanionService to send a broadcast using the given intentSender
* (which should cast to an IIntentSender), along with the other information specified.
*/
diff --git a/apex/statsd/aidl/android/os/IStatsManagerService.aidl b/apex/statsd/aidl/android/os/IStatsManagerService.aidl
index 151cdbd..291140c 100644
--- a/apex/statsd/aidl/android/os/IStatsManagerService.aidl
+++ b/apex/statsd/aidl/android/os/IStatsManagerService.aidl
@@ -53,6 +53,13 @@
long[] setActiveConfigsChangedOperation(in PendingIntent pendingIntent, in String packageName);
/**
+ * Removes the active configs changed operation for the specified package name.
+ *
+ * Requires Manifest.permission.DUMP and Manifest.permission.PACKAGE_USAGE_STATS.
+ */
+ void removeActiveConfigsChangedOperation(in String packageName);
+
+ /**
* Set the PendingIntent to be used when broadcasting subscriber
* information to the given subscriberId within the given config.
*
diff --git a/apex/statsd/aidl/android/os/IStatsd.aidl b/apex/statsd/aidl/android/os/IStatsd.aidl
index 986d483..323d56f 100644
--- a/apex/statsd/aidl/android/os/IStatsd.aidl
+++ b/apex/statsd/aidl/android/os/IStatsd.aidl
@@ -133,14 +133,14 @@
*
* Requires Manifest.permission.DUMP and Manifest.permission.PACKAGE_USAGE_STATS.
*/
- long[] setActiveConfigsChangedOperation(in IBinder intentSender, in String packageName);
+ long[] setActiveConfigsChangedOperation(in IPendingIntentRef pendingIntentRef, int callingUid);
/**
* Removes the active configs changed operation for the specified package name.
*
* Requires Manifest.permission.DUMP and Manifest.permission.PACKAGE_USAGE_STATS.
*/
- void removeActiveConfigsChangedOperation(in String packageName);
+ void removeActiveConfigsChangedOperation(int callingUid);
/**
* Removes the configuration with the matching config key. No-op if this config key does not
diff --git a/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java b/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java
index 6a0bf62..77ccfb6 100644
--- a/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java
+++ b/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java
@@ -17,6 +17,7 @@
package com.android.server.stats;
import android.app.PendingIntent;
+import android.app.StatsManager;
import android.content.Context;
import android.content.Intent;
import android.os.Binder;
@@ -27,6 +28,8 @@
import com.android.server.SystemService;
+import java.util.Arrays;
+
/**
* @hide
*/
@@ -97,6 +100,8 @@
*/
private static final String EXTRA_LAST_REPORT_TIME = "android.app.extra.LAST_REPORT_TIME";
private static final int CODE_DATA_BROADCAST = 1;
+ private static final int CODE_ACTIVE_CONFIGS_BROADCAST = 1;
+
private final PendingIntent mPendingIntent;
private final Context mContext;
@@ -120,7 +125,17 @@
@Override
public void sendActiveConfigsChangedBroadcast(long[] configIds) {
- // no-op
+ enforceStatsCompanionPermission(mContext);
+ Intent intent = new Intent();
+ intent.putExtra(StatsManager.EXTRA_STATS_ACTIVE_CONFIG_KEYS, configIds);
+ try {
+ mPendingIntent.send(mContext, CODE_ACTIVE_CONFIGS_BROADCAST, intent, null, null);
+ if (DEBUG) {
+ Slog.d(TAG, "Sent broadcast with config ids " + Arrays.toString(configIds));
+ }
+ } catch (PendingIntent.CanceledException e) {
+ Slog.w(TAG, "Unable to send active configs changed broadcast using PendingIntent");
+ }
}
@Override
diff --git a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java
index d160f22..befce21 100644
--- a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java
+++ b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java
@@ -203,7 +203,6 @@
private static final int INSTALLER_FIELD_ID = 5;
public static final int CODE_SUBSCRIBER_BROADCAST = 1;
- public static final int CODE_ACTIVE_CONFIGS_BROADCAST = 1;
public static final int DEATH_THRESHOLD = 10;
/**
* Which native processes to snapshot memory for.
@@ -444,22 +443,6 @@
}
@Override
- public void sendActiveConfigsChangedBroadcast(IBinder intentSenderBinder, long[] configIds) {
- StatsCompanion.enforceStatsCompanionPermission(mContext);
- IntentSender intentSender = new IntentSender(intentSenderBinder);
- Intent intent = new Intent();
- intent.putExtra(StatsManager.EXTRA_STATS_ACTIVE_CONFIG_KEYS, configIds);
- try {
- intentSender.sendIntent(mContext, CODE_ACTIVE_CONFIGS_BROADCAST, intent, null, null);
- if (DEBUG) {
- Slog.d(TAG, "Sent broadcast with config ids " + Arrays.toString(configIds));
- }
- } catch (IntentSender.SendIntentException e) {
- Slog.w(TAG, "Unable to send active configs changed broadcast using IntentSender");
- }
- }
-
- @Override
public void sendSubscriberBroadcast(IBinder intentSenderBinder, long configUid, long configKey,
long subscriptionId, long subscriptionRuleId, String[] cookies,
StatsDimensionsValue dimensionsValue) {
diff --git a/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java b/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java
index 46fe597..ef2b761 100644
--- a/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java
+++ b/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java
@@ -37,6 +37,7 @@
/**
* Service for {@link android.app.StatsManager}.
+ *
* @hide
*/
public class StatsManagerService extends IStatsManagerService.Stub {
@@ -57,6 +58,9 @@
@GuardedBy("mLock")
private ArrayMap<ConfigKey, PendingIntentRef> mDataFetchPirMap = new ArrayMap<>();
+ @GuardedBy("mLock")
+ private ArrayMap<Integer, PendingIntentRef> mActiveConfigsPirMap =
+ new ArrayMap<>();
public StatsManagerService(Context context) {
super();
@@ -143,11 +147,45 @@
@Override
public long[] setActiveConfigsChangedOperation(PendingIntent pendingIntent,
String packageName) {
- // no-op
- if (DEBUG) {
- Slog.d(TAG, "setActiveConfigsChangedOperation");
+ enforceDumpAndUsageStatsPermission(packageName);
+ int callingUid = Binder.getCallingUid();
+ final long token = Binder.clearCallingIdentity();
+ PendingIntentRef pir = new PendingIntentRef(pendingIntent, mContext);
+ // We add the PIR to a map so we can reregister if statsd is unavailable.
+ synchronized (mLock) {
+ mActiveConfigsPirMap.put(callingUid, pir);
}
- return new long[]{};
+ try {
+ IStatsd statsd = getStatsdNonblocking();
+ if (statsd != null) {
+ return statsd.setActiveConfigsChangedOperation(pir, callingUid);
+ }
+ } catch (RemoteException e) {
+ Slog.e(TAG, "Failed to setActiveConfigsChangedOperation with statsd");
+ } finally {
+ Binder.restoreCallingIdentity(token);
+ }
+ return new long[] {};
+ }
+
+ @Override
+ public void removeActiveConfigsChangedOperation(String packageName) {
+ enforceDumpAndUsageStatsPermission(packageName);
+ int callingUid = Binder.getCallingUid();
+ final long token = Binder.clearCallingIdentity();
+ synchronized (mLock) {
+ mActiveConfigsPirMap.remove(callingUid);
+ }
+ try {
+ IStatsd statsd = getStatsdNonblocking();
+ if (statsd != null) {
+ statsd.removeActiveConfigsChangedOperation(callingUid);
+ }
+ } catch (RemoteException e) {
+ Slog.e(TAG, "Failed to removeActiveConfigsChangedOperation with statsd");
+ } finally {
+ Binder.restoreCallingIdentity(token);
+ }
}
@Override
@@ -248,8 +286,10 @@
return;
}
ArrayMap<ConfigKey, PendingIntentRef> dataFetchCopy;
+ ArrayMap<Integer, PendingIntentRef> activeConfigsChangedCopy;
synchronized (mLock) {
dataFetchCopy = new ArrayMap<>(mDataFetchPirMap);
+ activeConfigsChangedCopy = new ArrayMap<>(mActiveConfigsPirMap);
}
for (Map.Entry<ConfigKey, PendingIntentRef> entry : dataFetchCopy.entrySet()) {
ConfigKey key = entry.getKey();
@@ -259,5 +299,13 @@
Slog.e(TAG, "Failed to setDataFetchOperation from pirMap");
}
}
+ for (Map.Entry<Integer, PendingIntentRef> entry
+ : activeConfigsChangedCopy.entrySet()) {
+ try {
+ statsd.setActiveConfigsChangedOperation(entry.getValue(), entry.getKey());
+ } catch (RemoteException e) {
+ Slog.e(TAG, "Failed to setActiveConfigsChangedOperation from pirMap");
+ }
+ }
}
}
diff --git a/cmds/statsd/src/StatsService.cpp b/cmds/statsd/src/StatsService.cpp
index 7fecf46..3a29b72 100644
--- a/cmds/statsd/src/StatsService.cpp
+++ b/cmds/statsd/src/StatsService.cpp
@@ -185,18 +185,17 @@
}
},
[this](const int& uid, const vector<int64_t>& activeConfigs) {
- auto receiver = mConfigManager->GetActiveConfigsChangedReceiver(uid);
- sp<IStatsCompanionService> sc = getStatsCompanionService();
- if (sc == nullptr) {
- VLOG("Could not access statsCompanion");
- return false;
- } else if (receiver == nullptr) {
+ sp<IPendingIntentRef> receiver =
+ mConfigManager->GetActiveConfigsChangedReceiver(uid);
+ if (receiver == nullptr) {
VLOG("Could not find receiver for uid %d", uid);
return false;
- } else {
- sc->sendActiveConfigsChangedBroadcast(receiver, activeConfigs);
+ } else if (receiver->sendActiveConfigsChangedBroadcast(activeConfigs).isOk()) {
VLOG("StatsService::active configs broadcast succeeded for uid %d" , uid);
return true;
+ } else {
+ VLOG("StatsService::active configs broadcast failed for uid %d" , uid);
+ return false;
}
});
@@ -630,15 +629,15 @@
}
}
}
- auto receiver = mConfigManager->GetActiveConfigsChangedReceiver(uid);
- sp<IStatsCompanionService> sc = getStatsCompanionService();
- if (sc == nullptr) {
- VLOG("Could not access statsCompanion");
- } else if (receiver == nullptr) {
+ sp<IPendingIntentRef> receiver = mConfigManager->GetActiveConfigsChangedReceiver(uid);
+ if (receiver == nullptr) {
VLOG("Could not find receiver for uid %d", uid);
- } else {
- sc->sendActiveConfigsChangedBroadcast(receiver, configIds);
+ return UNKNOWN_ERROR;
+ } else if (receiver->sendActiveConfigsChangedBroadcast(configIds).isOk()) {
VLOG("StatsService::trigger active configs changed broadcast succeeded for uid %d" , uid);
+ } else {
+ VLOG("StatsService::trigger active configs changed broadcast failed for uid %d", uid);
+ return UNKNOWN_ERROR;
}
return NO_ERROR;
}
@@ -1209,27 +1208,24 @@
return Status::ok();
}
-Status StatsService::setActiveConfigsChangedOperation(const sp<android::IBinder>& intentSender,
- const String16& packageName,
+Status StatsService::setActiveConfigsChangedOperation(const sp<IPendingIntentRef>& pir,
+ const int32_t callingUid,
vector<int64_t>* output) {
- ENFORCE_DUMP_AND_USAGE_STATS(packageName);
+ ENFORCE_UID(AID_SYSTEM);
- IPCThreadState* ipc = IPCThreadState::self();
- int uid = ipc->getCallingUid();
- mConfigManager->SetActiveConfigsChangedReceiver(uid, intentSender);
+ mConfigManager->SetActiveConfigsChangedReceiver(callingUid, pir);
if (output != nullptr) {
- mProcessor->GetActiveConfigs(uid, *output);
+ mProcessor->GetActiveConfigs(callingUid, *output);
} else {
ALOGW("StatsService::setActiveConfigsChanged output was nullptr");
}
return Status::ok();
}
-Status StatsService::removeActiveConfigsChangedOperation(const String16& packageName) {
- ENFORCE_DUMP_AND_USAGE_STATS(packageName);
+Status StatsService::removeActiveConfigsChangedOperation(const int32_t callingUid) {
+ ENFORCE_UID(AID_SYSTEM);
- IPCThreadState* ipc = IPCThreadState::self();
- mConfigManager->RemoveActiveConfigsChangedReceiver(ipc->getCallingUid());
+ mConfigManager->RemoveActiveConfigsChangedReceiver(callingUid);
return Status::ok();
}
diff --git a/cmds/statsd/src/StatsService.h b/cmds/statsd/src/StatsService.h
index 9912d2f..0e89a2b 100644
--- a/cmds/statsd/src/StatsService.h
+++ b/cmds/statsd/src/StatsService.h
@@ -134,14 +134,14 @@
/**
* Binder call to let clients register the active configs changed operation.
*/
- virtual Status setActiveConfigsChangedOperation(const sp<android::IBinder>& intentSender,
- const String16& packageName,
+ virtual Status setActiveConfigsChangedOperation(const sp<IPendingIntentRef>& pir,
+ const int32_t callingUid,
vector<int64_t>* output) override;
/**
* Binder call to remove the active configs changed operation for the specified package..
*/
- virtual Status removeActiveConfigsChangedOperation(const String16& packageName) override;
+ virtual Status removeActiveConfigsChangedOperation(const int32_t callingUid) override;
/**
* Binder call to allow clients to remove the specified configuration.
*/
diff --git a/cmds/statsd/src/config/ConfigManager.cpp b/cmds/statsd/src/config/ConfigManager.cpp
index 7bfb991..55d73c1 100644
--- a/cmds/statsd/src/config/ConfigManager.cpp
+++ b/cmds/statsd/src/config/ConfigManager.cpp
@@ -63,6 +63,24 @@
}
};
+class ActiveConfigChangedReceiverDeathRecipient : public android::IBinder::DeathRecipient {
+ public:
+ ActiveConfigChangedReceiverDeathRecipient(sp<ConfigManager> configManager, const int uid):
+ mConfigManager(configManager),
+ mUid(uid) {}
+ ~ActiveConfigChangedReceiverDeathRecipient() override = default;
+ private:
+ sp<ConfigManager> mConfigManager;
+ int mUid;
+
+ void binderDied(const android::wp<android::IBinder>& who) override {
+ if (IInterface::asBinder(mConfigManager->GetActiveConfigsChangedReceiver(mUid))
+ == who.promote()) {
+ mConfigManager->RemoveActiveConfigsChangedReceiver(mUid);
+ }
+ }
+};
+
ConfigManager::ConfigManager() {
}
@@ -148,9 +166,11 @@
}
void ConfigManager::SetActiveConfigsChangedReceiver(const int uid,
- const sp<IBinder>& intentSender) {
+ const sp<IPendingIntentRef>& pir) {
lock_guard<mutex> lock(mMutex);
- mActiveConfigsChangedReceivers[uid] = intentSender;
+ mActiveConfigsChangedReceivers[uid] = pir;
+ IInterface::asBinder(pir)->linkToDeath(
+ new ActiveConfigChangedReceiverDeathRecipient(this, uid));
}
void ConfigManager::RemoveActiveConfigsChangedReceiver(const int uid) {
@@ -296,7 +316,7 @@
}
}
-const sp<android::IBinder> ConfigManager::GetActiveConfigsChangedReceiver(const int uid) const {
+const sp<IPendingIntentRef> ConfigManager::GetActiveConfigsChangedReceiver(const int uid) const {
lock_guard<mutex> lock(mMutex);
auto it = mActiveConfigsChangedReceivers.find(uid);
diff --git a/cmds/statsd/src/config/ConfigManager.h b/cmds/statsd/src/config/ConfigManager.h
index 1aeb355..88e864a 100644
--- a/cmds/statsd/src/config/ConfigManager.h
+++ b/cmds/statsd/src/config/ConfigManager.h
@@ -16,7 +16,6 @@
#pragma once
-#include "binder/IBinder.h"
#include "config/ConfigKey.h"
#include "config/ConfigListener.h"
@@ -86,13 +85,13 @@
* Sets the broadcast receiver that is notified whenever the list of active configs
* changes for this uid.
*/
- void SetActiveConfigsChangedReceiver(const int uid, const sp<IBinder>& intentSender);
+ void SetActiveConfigsChangedReceiver(const int uid, const sp<IPendingIntentRef>& pir);
/**
* Returns the broadcast receiver for active configs changed for this uid.
*/
- const sp<IBinder> GetActiveConfigsChangedReceiver(const int uid) const;
+ const sp<IPendingIntentRef> GetActiveConfigsChangedReceiver(const int uid) const;
/**
* Erase any active configs changed broadcast receiver associated with this uid.
@@ -148,9 +147,9 @@
/**
* Each uid can be subscribed by up to one receiver to notify that the list of active configs
- * for this uid has changed. The receiver is specified as IBinder from PendingIntent.
+ * for this uid has changed. The receiver is specified as IPendingIntentRef.
*/
- std::map<int, sp<android::IBinder>> mActiveConfigsChangedReceivers;
+ std::map<int, sp<IPendingIntentRef>> mActiveConfigsChangedReceivers;
/**
* The ConfigListeners that will be told about changes.
diff --git a/core/java/android/app/StatsManager.java b/core/java/android/app/StatsManager.java
index a458a55..51b2d40 100644
--- a/core/java/android/app/StatsManager.java
+++ b/core/java/android/app/StatsManager.java
@@ -345,20 +345,18 @@
throws StatsUnavailableException {
synchronized (sLock) {
try {
- IStatsd service = getIStatsdLocked();
+ IStatsManagerService service = getIStatsManagerServiceLocked();
if (pendingIntent == null) {
service.removeActiveConfigsChangedOperation(mContext.getOpPackageName());
return new long[0];
} else {
- // Extracts IIntentSender from the PendingIntent and turns it into an IBinder.
- IBinder intentSender = pendingIntent.getTarget().asBinder();
- return service.setActiveConfigsChangedOperation(intentSender,
+ return service.setActiveConfigsChangedOperation(pendingIntent,
mContext.getOpPackageName());
}
} catch (RemoteException e) {
- Slog.e(TAG,
- "Failed to connect to statsd when registering active configs listener.");
+ Slog.e(TAG, "Failed to connect to statsmanager "
+ + "when registering active configs listener.");
throw new StatsUnavailableException("could not connect", e);
} catch (SecurityException e) {
throw new StatsUnavailableException(e.getMessage(), e);