From 14888548149542435e486ae40b09e903202ecc6d Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 19 May 2015 14:42:38 -0700 Subject: [PATCH 01/14] Push initial disk state, handle empty media. Stash volume count from last scan, and use it to push initial storage notifications state when listener is first attached. Also omit disks with invalid size, which usually means they're an empty slot with no media. Bug: 20503551 Change-Id: I75097035aebaad70ba32437179a863f6a0910aa5 --- core/java/android/os/storage/DiskInfo.java | 4 ++++ .../src/com/android/systemui/usb/StorageNotification.java | 7 ++++++- services/core/java/com/android/server/MountService.java | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/core/java/android/os/storage/DiskInfo.java b/core/java/android/os/storage/DiskInfo.java index 9623695edbba6..04e54aafbce9a 100644 --- a/core/java/android/os/storage/DiskInfo.java +++ b/core/java/android/os/storage/DiskInfo.java @@ -50,6 +50,8 @@ public class DiskInfo implements Parcelable { public final int flags; public long size; public String label; + /** Hacky; don't rely on this count */ + public int volumeCount; public DiskInfo(String id, int flags) { this.id = Preconditions.checkNotNull(id); @@ -61,6 +63,7 @@ public class DiskInfo implements Parcelable { flags = parcel.readInt(); size = parcel.readLong(); label = parcel.readString(); + volumeCount = parcel.readInt(); } public @NonNull String getId() { @@ -181,5 +184,6 @@ public class DiskInfo implements Parcelable { parcel.writeInt(this.flags); parcel.writeLong(size); parcel.writeString(label); + parcel.writeInt(volumeCount); } } diff --git a/packages/SystemUI/src/com/android/systemui/usb/StorageNotification.java b/packages/SystemUI/src/com/android/systemui/usb/StorageNotification.java index 23813d11919c1..ad215551a90f0 100644 --- a/packages/SystemUI/src/com/android/systemui/usb/StorageNotification.java +++ b/packages/SystemUI/src/com/android/systemui/usb/StorageNotification.java @@ -148,6 +148,11 @@ public class StorageNotification extends SystemUI { android.Manifest.permission.MOUNT_UNMOUNT_FILESYSTEMS, null); // Kick current state into place + final List disks = mStorageManager.getDisks(); + for (DiskInfo disk : disks) { + onDiskScannedInternal(disk, disk.volumeCount); + } + final List vols = mStorageManager.getVolumes(); for (VolumeInfo vol : vols) { onVolumeStateChangedInternal(vol); @@ -194,7 +199,7 @@ public class StorageNotification extends SystemUI { } private void onDiskScannedInternal(DiskInfo disk, int volumeCount) { - if (volumeCount == 0) { + if (volumeCount == 0 && disk.size > 0) { // No supported volumes found, give user option to format final CharSequence title = mContext.getString( R.string.ext_media_unmountable_notification_title, disk.getDescription()); diff --git a/services/core/java/com/android/server/MountService.java b/services/core/java/com/android/server/MountService.java index e00cf5b1cf3d8..d48953db8f45e 100644 --- a/services/core/java/com/android/server/MountService.java +++ b/services/core/java/com/android/server/MountService.java @@ -973,6 +973,7 @@ class MountService extends IMountService.Stub } } + disk.volumeCount = volumeCount; mCallbacks.notifyDiskScanned(disk, volumeCount); } From f92fed8f2f9761ad55d81ba2394b6e8a63183002 Mon Sep 17 00:00:00 2001 From: Jason Monk Date: Tue, 19 May 2015 16:06:52 -0400 Subject: [PATCH 02/14] Don't crash in bugreport on devices without BT Bug: 21160862 Change-Id: Ia3295fc6ebbd8ad1b5d03c70c89370d09f9cbb03 --- .../systemui/statusbar/policy/BluetoothControllerImpl.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/BluetoothControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/BluetoothControllerImpl.java index 114427c1b94f3..ed98a159d10d4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/BluetoothControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/BluetoothControllerImpl.java @@ -59,6 +59,9 @@ public class BluetoothControllerImpl implements BluetoothController, BluetoothCa public void dump(FileDescriptor fd, PrintWriter pw, String[] args) { pw.println("BluetoothController state:"); pw.print(" mLocalBluetoothManager="); pw.println(mLocalBluetoothManager); + if (mLocalBluetoothManager == null) { + return; + } pw.print(" mEnabled="); pw.println(mEnabled); pw.print(" mConnecting="); pw.println(mConnecting); pw.print(" mLastDevice="); pw.println(mLastDevice); From c68772b68ade8feb2babb3fa2ad0f21860d9f316 Mon Sep 17 00:00:00 2001 From: Dan Sandler Date: Tue, 19 May 2015 20:59:12 -0400 Subject: [PATCH 03/14] Deal more gracefully with null smallIcons. First, when parceling a notification with no small icon: Well, you shouldn't attempt to do this anyway, since NoMan will reject a notification without a valid smallIcon. But setServiceForeground parcels up the Notification on its own before handing it off to NoMan, so it will crash on an invalid small icon. (In general, parceling code should never ever crash, even if the object is in an undesirable state.) And when build()ing a notification: Same thing---don't build a notification with no icon; you're going to have a bad time. But maybe you're going to fix it before you hand it off to NoMan. Or maybe it's just one page of a wearable notification, so it doesn't really need its own icon. Either way, Notification shouldn't crash. Bug: 21286186 Bug: 21298403 Change-Id: Ie482cde0a3afe3aaabf07be0536551b8e4bceba0 --- core/java/android/app/Notification.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java index 3309443352c5c..96c6878376b05 100644 --- a/core/java/android/app/Notification.java +++ b/core/java/android/app/Notification.java @@ -1366,7 +1366,9 @@ public class Notification implements Parcelable int version = parcel.readInt(); when = parcel.readLong(); - mSmallIcon = Icon.CREATOR.createFromParcel(parcel); + if (parcel.readInt() != 0) { + mSmallIcon = Icon.CREATOR.createFromParcel(parcel); + } number = parcel.readInt(); if (parcel.readInt() != 0) { contentIntent = PendingIntent.CREATOR.createFromParcel(parcel); @@ -1590,7 +1592,12 @@ public class Notification implements Parcelable parcel.writeInt(1); parcel.writeLong(when); - mSmallIcon.writeToParcel(parcel, 0); + if (mSmallIcon != null) { + parcel.writeInt(1); + mSmallIcon.writeToParcel(parcel, 0); + } else { + parcel.writeInt(0); + } parcel.writeInt(number); if (contentIntent != null) { parcel.writeInt(1); @@ -3241,7 +3248,7 @@ public class Notification implements Parcelable Notification n = new Notification(); n.when = mWhen; n.mSmallIcon = mSmallIcon; - if (mSmallIcon.getType() == Icon.TYPE_RESOURCE) { + if (mSmallIcon != null && mSmallIcon.getType() == Icon.TYPE_RESOURCE) { n.icon = mSmallIcon.getResId(); } n.iconLevel = mSmallIconLevel; From f59392f62d2a45d93feaceb60d03217f7442cab9 Mon Sep 17 00:00:00 2001 From: John Reck Date: Mon, 18 May 2015 15:11:52 -0700 Subject: [PATCH 04/14] Fix NPE in setSurfaceTexure Bug: 20088412 Change-Id: I9b78636a7d89438c8924bb1bf2adba00e74366eb --- core/java/android/view/TextureView.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/core/java/android/view/TextureView.java b/core/java/android/view/TextureView.java index d70712ac327b8..ad7d04e1fe1da 100644 --- a/core/java/android/view/TextureView.java +++ b/core/java/android/view/TextureView.java @@ -730,9 +730,13 @@ public class TextureView extends View { } mSurface = surfaceTexture; - // If the view is visible, update the listener in the new surface to use - // the existing listener in the view. - if (((mViewFlags & VISIBILITY_MASK) == VISIBLE)) { + /* + * If the view is visible and we already made a layer, update the + * listener in the new surface to use the existing listener in the view. + * Otherwise this will be called when the view becomes visible or the + * layer is created + */ + if (((mViewFlags & VISIBILITY_MASK) == VISIBLE) && mLayer != null) { mSurface.setOnFrameAvailableListener(mUpdateListener, mAttachInfo.mHandler); } mUpdateSurface = true; From bbd8c93905c394866527e92e9b660720e691a5dd Mon Sep 17 00:00:00 2001 From: Ruben Brunk Date: Tue, 19 May 2015 17:20:24 -0700 Subject: [PATCH 05/14] camera: Add AIDL interface for CameraServiceProxy. - Adds an AIDL interface to allow the proxy camera service running in system server to accept RPCs from the camera service running in mediaserver. - Request an update to the valid user set from the proxy camera service when mediaserver restarts to initialize properly + avoid DOS after a crash. Bug: 21267484 Change-Id: Ib821582794ddd1e3574b5dc6c79f7cb197b57f10 --- Android.mk | 1 + .../java/android/hardware/ICameraService.aidl | 6 +++- .../android/hardware/ICameraServiceProxy.aidl | 30 +++++++++++++++++ .../android/server/camera/CameraService.java | 33 ++++++++++++++++--- 4 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 core/java/android/hardware/ICameraServiceProxy.aidl diff --git a/Android.mk b/Android.mk index 146afe0e07569..5f4e7a29dc372 100644 --- a/Android.mk +++ b/Android.mk @@ -146,6 +146,7 @@ LOCAL_SRC_FILES += \ core/java/android/database/IContentObserver.aidl \ core/java/android/hardware/ICameraService.aidl \ core/java/android/hardware/ICameraServiceListener.aidl \ + core/java/android/hardware/ICameraServiceProxy.aidl \ core/java/android/hardware/ICamera.aidl \ core/java/android/hardware/ICameraClient.aidl \ core/java/android/hardware/IConsumerIrService.aidl \ diff --git a/core/java/android/hardware/ICameraService.aidl b/core/java/android/hardware/ICameraService.aidl index 9201b614aeda5..c933f923297a5 100644 --- a/core/java/android/hardware/ICameraService.aidl +++ b/core/java/android/hardware/ICameraService.aidl @@ -25,7 +25,11 @@ import android.hardware.camera2.utils.BinderHolder; import android.hardware.ICameraServiceListener; import android.hardware.CameraInfo; -/** @hide */ +/** + * Binder interface for the native camera service running in mediaserver. + * + * @hide + */ interface ICameraService { /** diff --git a/core/java/android/hardware/ICameraServiceProxy.aidl b/core/java/android/hardware/ICameraServiceProxy.aidl new file mode 100644 index 0000000000000..0bb24bc2b04e3 --- /dev/null +++ b/core/java/android/hardware/ICameraServiceProxy.aidl @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.hardware; + +/** + * Binder interface for the camera service proxy running in system_server. + * + * @hide + */ +interface ICameraServiceProxy +{ + /** + * Ping the service proxy to update the valid users for the camera service. + */ + oneway void pingForUserUpdate(); +} diff --git a/services/core/java/com/android/server/camera/CameraService.java b/services/core/java/com/android/server/camera/CameraService.java index 1d77bc28e0dfe..777a9dd1bdcfc 100644 --- a/services/core/java/com/android/server/camera/CameraService.java +++ b/services/core/java/com/android/server/camera/CameraService.java @@ -19,6 +19,7 @@ import android.app.ActivityManager; import android.content.Context; import android.content.pm.UserInfo; import android.hardware.ICameraService; +import android.hardware.ICameraServiceProxy; import android.os.IBinder; import android.os.RemoteException; import android.os.UserManager; @@ -42,14 +43,30 @@ public class CameraService extends SystemService { */ private static final String CAMERA_SERVICE_BINDER_NAME = "media.camera"; + public static final String CAMERA_SERVICE_PROXY_BINDER_NAME = "media.camera.proxy"; + // Event arguments to use with the camera service notifySystemEvent call: public static final int NO_EVENT = 0; // NOOP public static final int USER_SWITCHED = 1; // User changed, argument is the new user handle private final Context mContext; private UserManager mUserManager; + + private final Object mLock = new Object(); private Set mEnabledCameraUsers; + private final ICameraServiceProxy.Stub mCameraServiceProxy = new ICameraServiceProxy.Stub() { + @Override + public void pingForUserUpdate() { + // Binder call + synchronized(mLock) { + if (mEnabledCameraUsers != null) { + notifyMediaserver(USER_SWITCHED, mEnabledCameraUsers); + } + } + } + }; + public CameraService(Context context) { super(context); mContext = context; @@ -62,18 +79,27 @@ public class CameraService extends SystemService { // Should never see this unless someone messes up the SystemServer service boot order. throw new IllegalStateException("UserManagerService must start before CameraService!"); } + publishBinderService(CAMERA_SERVICE_PROXY_BINDER_NAME, mCameraServiceProxy); } @Override public void onStartUser(int userHandle) { - if (mEnabledCameraUsers == null) { - // Initialize mediaserver, or update mediaserver if we are recovering from a crash. - onSwitchUser(userHandle); + synchronized(mLock) { + if (mEnabledCameraUsers == null) { + // Initialize mediaserver, or update mediaserver if we are recovering from a crash. + switchUserLocked(userHandle); + } } } @Override public void onSwitchUser(int userHandle) { + synchronized(mLock) { + switchUserLocked(userHandle); + } + } + + private void switchUserLocked(int userHandle) { Set currentUserHandles = getEnabledUserHandles(userHandle); if (mEnabledCameraUsers == null || !mEnabledCameraUsers.equals(currentUserHandles)) { // Some user handles have been added or removed, update mediaserver. @@ -82,7 +108,6 @@ public class CameraService extends SystemService { } } - private Set getEnabledUserHandles(int currentUserHandle) { List userProfiles = mUserManager.getEnabledProfiles(currentUserHandle); Set handles = new HashSet<>(userProfiles.size()); From 42199446ae12c919d53229d1dbd835d3822d7926 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 20 May 2015 12:04:42 -0700 Subject: [PATCH 06/14] Write packages.list when adding/removing users. FUSE daemons now rely on getting per-user GID information when packages.list is written. Normal secondary user adding/removing usually has enough PackageManager traffic to trigger a side-effect rewrite, but this change writes explicitly to handle guest users. Also obtain the user list once, and exclude dying users. During user creation we manually splice in the user ID that we're bringing online. Bug: 19924661 Change-Id: Icc5b1b169300c9dc12099be12651acbf89d6bea9 --- .../android/server/pm/PermissionsState.java | 4 +- .../java/com/android/server/pm/Settings.java | 173 ++++++++++-------- 2 files changed, 99 insertions(+), 78 deletions(-) diff --git a/services/core/java/com/android/server/pm/PermissionsState.java b/services/core/java/com/android/server/pm/PermissionsState.java index 8942325c799f5..ad662beaaa303 100644 --- a/services/core/java/com/android/server/pm/PermissionsState.java +++ b/services/core/java/com/android/server/pm/PermissionsState.java @@ -381,10 +381,10 @@ public final class PermissionsState { * * @return The gids for all device users. */ - public int[] computeGids() { + public int[] computeGids(int[] userIds) { int[] gids = mGlobalGids; - for (int userId : UserManagerService.getInstance().getUserIds()) { + for (int userId : userIds) { final int[] userGids = computeGids(userId); gids = appendInts(gids, userGids); } diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java index 76ef19f3b0e2a..d2a135c9787b9 100644 --- a/services/core/java/com/android/server/pm/Settings.java +++ b/services/core/java/com/android/server/pm/Settings.java @@ -2022,83 +2022,8 @@ final class Settings { |FileUtils.S_IRGRP|FileUtils.S_IWGRP, -1, -1); - // Write package list file now, use a JournaledFile. - File tempFile = new File(mPackageListFilename.getAbsolutePath() + ".tmp"); - JournaledFile journal = new JournaledFile(mPackageListFilename, tempFile); - - final File writeTarget = journal.chooseForWrite(); - fstr = new FileOutputStream(writeTarget); - str = new BufferedOutputStream(fstr); - try { - FileUtils.setPermissions(fstr.getFD(), 0640, SYSTEM_UID, PACKAGE_INFO_GID); - - StringBuilder sb = new StringBuilder(); - for (final PackageSetting pkg : mPackages.values()) { - if (pkg.pkg == null || pkg.pkg.applicationInfo == null) { - Slog.w(TAG, "Skipping " + pkg + " due to missing metadata"); - continue; - } - - final ApplicationInfo ai = pkg.pkg.applicationInfo; - final String dataPath = ai.dataDir; - final boolean isDebug = (ai.flags & ApplicationInfo.FLAG_DEBUGGABLE) != 0; - final int[] gids = pkg.getPermissionsState().computeGids(); - - // Avoid any application that has a space in its path. - if (dataPath.indexOf(" ") >= 0) - continue; - - // we store on each line the following information for now: - // - // pkgName - package name - // userId - application-specific user id - // debugFlag - 0 or 1 if the package is debuggable. - // dataPath - path to package's data path - // seinfo - seinfo label for the app (assigned at install time) - // gids - supplementary gids this app launches with - // - // NOTE: We prefer not to expose all ApplicationInfo flags for now. - // - // DO NOT MODIFY THIS FORMAT UNLESS YOU CAN ALSO MODIFY ITS USERS - // FROM NATIVE CODE. AT THE MOMENT, LOOK AT THE FOLLOWING SOURCES: - // system/core/logd/LogStatistics.cpp - // system/core/run-as/run-as.c - // system/core/sdcard/sdcard.c - // external/libselinux/src/android.c:package_info_init() - // - sb.setLength(0); - sb.append(ai.packageName); - sb.append(" "); - sb.append((int)ai.uid); - sb.append(isDebug ? " 1 " : " 0 "); - sb.append(dataPath); - sb.append(" "); - sb.append(ai.seinfo); - sb.append(" "); - if (gids != null && gids.length > 0) { - sb.append(gids[0]); - for (int i = 1; i < gids.length; i++) { - sb.append(","); - sb.append(gids[i]); - } - } else { - sb.append("none"); - } - sb.append("\n"); - str.write(sb.toString().getBytes()); - } - str.flush(); - FileUtils.sync(fstr); - str.close(); - journal.commit(); - } catch (Exception e) { - Slog.wtf(TAG, "Failed to write packages.list", e); - IoUtils.closeQuietly(str); - journal.rollback(); - } - + writePackageListLPr(); writeAllUsersPackageRestrictionsLPr(); - writeAllRuntimePermissionsLPr(); return; @@ -2119,6 +2044,99 @@ final class Settings { //Debug.stopMethodTracing(); } + void writePackageListLPr() { + writePackageListLPr(-1); + } + + void writePackageListLPr(int creatingUserId) { + // Only derive GIDs for active users (not dying) + final List users = UserManagerService.getInstance().getUsers(true); + int[] userIds = new int[users.size()]; + for (int i = 0; i < userIds.length; i++) { + userIds[i] = users.get(i).id; + } + if (creatingUserId != -1) { + userIds = ArrayUtils.appendInt(userIds, creatingUserId); + } + + // Write package list file now, use a JournaledFile. + File tempFile = new File(mPackageListFilename.getAbsolutePath() + ".tmp"); + JournaledFile journal = new JournaledFile(mPackageListFilename, tempFile); + + final File writeTarget = journal.chooseForWrite(); + FileOutputStream fstr = null; + BufferedOutputStream str = null; + try { + fstr = new FileOutputStream(writeTarget); + str = new BufferedOutputStream(fstr); + FileUtils.setPermissions(fstr.getFD(), 0640, SYSTEM_UID, PACKAGE_INFO_GID); + + StringBuilder sb = new StringBuilder(); + for (final PackageSetting pkg : mPackages.values()) { + if (pkg.pkg == null || pkg.pkg.applicationInfo == null) { + Slog.w(TAG, "Skipping " + pkg + " due to missing metadata"); + continue; + } + + final ApplicationInfo ai = pkg.pkg.applicationInfo; + final String dataPath = ai.dataDir; + final boolean isDebug = (ai.flags & ApplicationInfo.FLAG_DEBUGGABLE) != 0; + final int[] gids = pkg.getPermissionsState().computeGids(userIds); + + // Avoid any application that has a space in its path. + if (dataPath.indexOf(" ") >= 0) + continue; + + // we store on each line the following information for now: + // + // pkgName - package name + // userId - application-specific user id + // debugFlag - 0 or 1 if the package is debuggable. + // dataPath - path to package's data path + // seinfo - seinfo label for the app (assigned at install time) + // gids - supplementary gids this app launches with + // + // NOTE: We prefer not to expose all ApplicationInfo flags for now. + // + // DO NOT MODIFY THIS FORMAT UNLESS YOU CAN ALSO MODIFY ITS USERS + // FROM NATIVE CODE. AT THE MOMENT, LOOK AT THE FOLLOWING SOURCES: + // system/core/logd/LogStatistics.cpp + // system/core/run-as/run-as.c + // system/core/sdcard/sdcard.c + // external/libselinux/src/android.c:package_info_init() + // + sb.setLength(0); + sb.append(ai.packageName); + sb.append(" "); + sb.append((int)ai.uid); + sb.append(isDebug ? " 1 " : " 0 "); + sb.append(dataPath); + sb.append(" "); + sb.append(ai.seinfo); + sb.append(" "); + if (gids != null && gids.length > 0) { + sb.append(gids[0]); + for (int i = 1; i < gids.length; i++) { + sb.append(","); + sb.append(gids[i]); + } + } else { + sb.append("none"); + } + sb.append("\n"); + str.write(sb.toString().getBytes()); + } + str.flush(); + FileUtils.sync(fstr); + str.close(); + journal.commit(); + } catch (Exception e) { + Slog.wtf(TAG, "Failed to write packages.list", e); + IoUtils.closeQuietly(str); + journal.rollback(); + } + } + void writeDisabledSysPackageLPr(XmlSerializer serializer, final PackageSetting pkg) throws java.io.IOException { serializer.startTag(null, "updated-package"); @@ -3491,6 +3509,7 @@ final class Settings { } readDefaultPreferredAppsLPw(service, userHandle); writePackageRestrictionsLPr(userHandle); + writePackageListLPr(userHandle); } void removeUserLPw(int userId) { @@ -3506,6 +3525,8 @@ final class Settings { removeCrossProfileIntentFiltersLPw(userId); mRuntimePermissionsPersistence.onUserRemoved(userId); + + writePackageListLPr(); } void removeCrossProfileIntentFiltersLPw(int userId) { From ca1002f49e0f817229b04ec93b675861e8bfcb83 Mon Sep 17 00:00:00 2001 From: Christopher Tate Date: Tue, 19 May 2015 18:16:58 -0700 Subject: [PATCH 07/14] Close race condition in binderDied() It was possible for a binderDied() call to occur while the death recipient list containing the object was being iterated, in which case we could invalidate an object reference out from under the iteration, causing a VM abort. We now interlock the binderDied() deref operation with the list's locking semantics to prevent this. Bug 15831054 Change-Id: If0027d3ac4da1153284a425dd9b2819a203481ab --- core/jni/android_util_Binder.cpp | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp index fb8207553ece5..634c9f754fc6f 100644 --- a/core/jni/android_util_Binder.cpp +++ b/core/jni/android_util_Binder.cpp @@ -358,6 +358,8 @@ public: void add(const sp& recipient); void remove(const sp& recipient); sp find(jobject recipient); + + Mutex& lock(); // Use with care; specifically for mutual exclusion during binder death }; // ---------------------------------------------------------------------------- @@ -392,11 +394,18 @@ public: "*** Uncaught exception returned from death notification!"); } - // Demote from strong ref to weak after binderDied() has been delivered, - // to allow the DeathRecipient and BinderProxy to be GC'd if no longer needed. - mObjectWeak = env->NewWeakGlobalRef(mObject); - env->DeleteGlobalRef(mObject); - mObject = NULL; + // Serialize with our containing DeathRecipientList so that we can't + // delete the global ref on mObject while the list is being iterated. + sp list = mList.promote(); + if (list != NULL) { + AutoMutex _l(list->lock()); + + // Demote from strong ref to weak after binderDied() has been delivered, + // to allow the DeathRecipient and BinderProxy to be GC'd if no longer needed. + mObjectWeak = env->NewWeakGlobalRef(mObject); + env->DeleteGlobalRef(mObject); + mObject = NULL; + } } } @@ -518,6 +527,10 @@ sp DeathRecipientList::find(jobject recipient) { return NULL; } +Mutex& DeathRecipientList::lock() { + return mLock; +} + // ---------------------------------------------------------------------------- namespace android { From 34ff521d1fee9b48c46b8841d6e45bbe866c3bb1 Mon Sep 17 00:00:00 2001 From: Narayan Kamath Date: Thu, 21 May 2015 10:50:35 +0100 Subject: [PATCH 08/14] Fix application moves. We don't dex2oat during application moves, so we must scan the package again scanPackageDirtyLI to deduce its ABI. This is unnecessary (since a move cannot change ABIs), but we need some additional refactoring to avoid a second scan. bug: 21337469 Change-Id: I3e9dfd5db1c928847f9d527dc15d29a05ff40e7d --- .../server/pm/PackageManagerService.java | 27 +++++++------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 6a47238d859cf..35317965618d6 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -6347,25 +6347,16 @@ public class PackageManagerService extends IPackageManager.Stub { if ((scanFlags & SCAN_NEW_INSTALL) == 0) { deriveNonSystemPackageAbi(pkg, scanFile, cpuAbiOverride, true /* extract libs */); } else { - // Verify the ABIs haven't changed since we last deduced them. - String oldPrimaryCpuAbi = pkg.applicationInfo.primaryCpuAbi; - String oldSecondaryCpuAbi = pkg.applicationInfo.secondaryCpuAbi; - - // TODO: The only purpose of this code is to update the native library paths - // based on the final install location. We can simplify this and avoid having - // to scan the package again. + // TODO: We need this second call to derive in two cases : + // + // - To update the native library paths based on the final install location. + // - We don't call dexopt when moving packages, and so we have to scan again. + // + // We can simplify this and avoid having to scan the package again by letting + // scanPackageLI know if the current install was a move (and deriving things only + // in that case) and by "reparenting" the native lib directory in the case of + // a normal (non-move) install. deriveNonSystemPackageAbi(pkg, scanFile, cpuAbiOverride, false /* extract libs */); - if (!TextUtils.equals(oldPrimaryCpuAbi, pkg.applicationInfo.primaryCpuAbi)) { - throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, - "unexpected abi change for " + pkg.packageName + " (" - + oldPrimaryCpuAbi + "-> " + pkg.applicationInfo.primaryCpuAbi); - } - - if (!TextUtils.equals(oldSecondaryCpuAbi, pkg.applicationInfo.secondaryCpuAbi)) { - throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, - "unexpected abi change for " + pkg.packageName + " (" - + oldSecondaryCpuAbi + "-> " + pkg.applicationInfo.secondaryCpuAbi); - } } if (DEBUG_INSTALL) Slog.i(TAG, "Linking native library dir for " + path); From 10256a0a256af20fb679e822a217847fd2b09378 Mon Sep 17 00:00:00 2001 From: Craig Mautner Date: Thu, 21 May 2015 15:33:30 -0700 Subject: [PATCH 09/14] For getHomeActivity() only return current user. Previously getHomeActivity() returned the topmost home activity independent of which user was currently running. That defeated the purpose of the method. This fix returns the home activity of the current user or null if one has not yet been created. Also remove some cruft that accumulated. Fixes bug 21055376. Change-Id: Ic1d58129aedbe3624f8a9d12c05c84674687b0a4 --- .../android/app/ActivityManagerNative.java | 21 ------------------- core/java/android/app/IActivityManager.java | 4 +--- .../server/am/ActivityManagerService.java | 9 -------- .../server/am/ActivityStackSupervisor.java | 2 +- 4 files changed, 2 insertions(+), 34 deletions(-) diff --git a/core/java/android/app/ActivityManagerNative.java b/core/java/android/app/ActivityManagerNative.java index cdf15e17407fb..02e0d5b8449fb 100644 --- a/core/java/android/app/ActivityManagerNative.java +++ b/core/java/android/app/ActivityManagerNative.java @@ -2307,14 +2307,6 @@ public abstract class ActivityManagerNative extends Binder implements IActivityM return true; } - case GET_HOME_ACTIVITY_TOKEN_TRANSACTION: { - data.enforceInterface(IActivityManager.descriptor); - IBinder homeActivityToken = getHomeActivityToken(); - reply.writeNoException(); - reply.writeStrongBinder(homeActivityToken); - return true; - } - case START_LOCK_TASK_BY_TASK_ID_TRANSACTION: { data.enforceInterface(IActivityManager.descriptor); final int taskId = data.readInt(); @@ -5531,19 +5523,6 @@ class ActivityManagerProxy implements IActivityManager return displayId; } - @Override - public IBinder getHomeActivityToken() throws RemoteException { - Parcel data = Parcel.obtain(); - Parcel reply = Parcel.obtain(); - data.writeInterfaceToken(IActivityManager.descriptor); - mRemote.transact(GET_HOME_ACTIVITY_TOKEN_TRANSACTION, data, reply, 0); - reply.readException(); - IBinder res = reply.readStrongBinder(); - data.recycle(); - reply.recycle(); - return res; - } - @Override public void startLockTaskMode(int taskId) throws RemoteException { Parcel data = Parcel.obtain(); diff --git a/core/java/android/app/IActivityManager.java b/core/java/android/app/IActivityManager.java index 310c5ef50f489..c42719ba72b81 100644 --- a/core/java/android/app/IActivityManager.java +++ b/core/java/android/app/IActivityManager.java @@ -457,8 +457,6 @@ public interface IActivityManager extends IInterface { public int getActivityDisplayId(IBinder activityToken) throws RemoteException; - public IBinder getHomeActivityToken() throws RemoteException; - public void startLockTaskModeOnCurrent() throws RemoteException; public void startLockTaskMode(int taskId) throws RemoteException; @@ -788,7 +786,7 @@ public interface IActivityManager extends IInterface { int RELEASE_PERSISTABLE_URI_PERMISSION_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+180; int GET_PERSISTED_URI_PERMISSIONS_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+181; int APP_NOT_RESPONDING_VIA_PROVIDER_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+182; - int GET_HOME_ACTIVITY_TOKEN_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+183; + // Available int GET_ACTIVITY_DISPLAY_ID_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+184; int DELETE_ACTIVITY_CONTAINER_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+185; diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 5a9b5b29a5ec0..f85f5b7892d44 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -8703,15 +8703,6 @@ public final class ActivityManagerService extends ActivityManagerNative Slog.e(TAG, "moveTaskBackwards not yet implemented!"); } - @Override - public IBinder getHomeActivityToken() throws RemoteException { - enforceCallingPermission(android.Manifest.permission.MANAGE_ACTIVITY_STACKS, - "getHomeActivityToken()"); - synchronized (this) { - return mStackSupervisor.getHomeActivityToken(); - } - } - @Override public IActivityContainer createVirtualActivityContainer(IBinder parentActivityToken, IActivityContainerCallback callback) throws RemoteException { diff --git a/services/core/java/com/android/server/am/ActivityStackSupervisor.java b/services/core/java/com/android/server/am/ActivityStackSupervisor.java index 5eee34f8d1f26..39df9d1558fb1 100644 --- a/services/core/java/com/android/server/am/ActivityStackSupervisor.java +++ b/services/core/java/com/android/server/am/ActivityStackSupervisor.java @@ -2658,7 +2658,7 @@ public final class ActivityStackSupervisor implements DisplayListener { } ActivityRecord getHomeActivity() { - return getHomeActivityForUser(UserHandle.USER_ALL); + return getHomeActivityForUser(mCurrentUser); } ActivityRecord getHomeActivityForUser(int userId) { From 5048207d983fbff2acec941e5c6c191e0bf7e9be Mon Sep 17 00:00:00 2001 From: Vinit Deshpande Date: Wed, 20 May 2015 14:27:49 -0700 Subject: [PATCH 10/14] Indicate failed scans with EXTRA_RESULTS_UPDATED This flag indicates if scan was successful and results were updated. It will be set to false if a scan is not performed (intentionally) or if it failed to produce any results. Bug: 20642015 Change-Id: I06a1fdd684932db68891ee28d5a049980f483f0f --- api/current.txt | 1 + api/system-current.txt | 1 + wifi/java/android/net/wifi/WifiManager.java | 8 ++++++++ 3 files changed, 10 insertions(+) diff --git a/api/current.txt b/api/current.txt index bbdb878b288f9..c855b62ea77d5 100644 --- a/api/current.txt +++ b/api/current.txt @@ -19255,6 +19255,7 @@ package android.net.wifi { field public static final java.lang.String EXTRA_NEW_RSSI = "newRssi"; field public static final java.lang.String EXTRA_NEW_STATE = "newState"; field public static final java.lang.String EXTRA_PREVIOUS_WIFI_STATE = "previous_wifi_state"; + field public static final java.lang.String EXTRA_RESULTS_UPDATED = "resultsUpdated"; field public static final java.lang.String EXTRA_SUPPLICANT_CONNECTED = "connected"; field public static final java.lang.String EXTRA_SUPPLICANT_ERROR = "supplicantError"; field public static final java.lang.String EXTRA_WIFI_INFO = "wifiInfo"; diff --git a/api/system-current.txt b/api/system-current.txt index 1a3673d8548eb..e6883cd0b26da 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -21016,6 +21016,7 @@ package android.net.wifi { field public static final java.lang.String EXTRA_NEW_RSSI = "newRssi"; field public static final java.lang.String EXTRA_NEW_STATE = "newState"; field public static final java.lang.String EXTRA_PREVIOUS_WIFI_STATE = "previous_wifi_state"; + field public static final java.lang.String EXTRA_RESULTS_UPDATED = "resultsUpdated"; field public static final java.lang.String EXTRA_SUPPLICANT_CONNECTED = "connected"; field public static final java.lang.String EXTRA_SUPPLICANT_ERROR = "supplicantError"; field public static final java.lang.String EXTRA_WIFI_CONFIGURATION = "wifiConfiguration"; diff --git a/wifi/java/android/net/wifi/WifiManager.java b/wifi/java/android/net/wifi/WifiManager.java index f2c2a28764999..64fa0e5226e61 100644 --- a/wifi/java/android/net/wifi/WifiManager.java +++ b/wifi/java/android/net/wifi/WifiManager.java @@ -403,6 +403,14 @@ public class WifiManager { */ @SdkConstant(SdkConstantType.BROADCAST_INTENT_ACTION) public static final String SCAN_RESULTS_AVAILABLE_ACTION = "android.net.wifi.SCAN_RESULTS"; + + /** + * The result of previous scan, reported with {@link #SCAN_RESULTS_AVAILABLE_ACTION}. + * @return true scan was successful, results updated + * @return false scan was not successful, results haven't been updated since previous scan + */ + public static final String EXTRA_RESULTS_UPDATED = "resultsUpdated"; + /** * A batch of access point scans has been completed and the results areavailable. * Call {@link #getBatchedScanResults()} to obtain the results. From f7a06315e77d9de5c87789174edb3b0346cd1a54 Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Thu, 21 May 2015 15:04:18 -0700 Subject: [PATCH 11/14] BatteryStatsService: Only query bluetooth on demand. Bluetooth was being queried too often, leading to more power consumption and wakelock time. Bug:21063567 Bug:21269307 Change-Id: Idddbab46d13016ef8528e095945b7817c12f7266 --- .../server/am/BatteryStatsService.java | 38 +++++++++++++------ 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/am/BatteryStatsService.java b/services/core/java/com/android/server/am/BatteryStatsService.java index a61223a00cb61..b1cc2880e58fa 100644 --- a/services/core/java/com/android/server/am/BatteryStatsService.java +++ b/services/core/java/com/android/server/am/BatteryStatsService.java @@ -83,11 +83,11 @@ public final class BatteryStatsService extends IBatteryStats.Stub public void handleMessage(Message msg) { switch (msg.what) { case MSG_SYNC_EXTERNAL_STATS: - updateExternalStats((String)msg.obj); + updateExternalStats((String)msg.obj, false); break; case MSG_WRITE_TO_DISK: - updateExternalStats("write"); + updateExternalStats("write", true); synchronized (mStats) { mStats.writeAsyncLocked(); } @@ -137,7 +137,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub public void shutdown() { Slog.w("BatteryStats", "Writing battery stats before shutdown..."); - updateExternalStats("shutdown"); + updateExternalStats("shutdown", true); synchronized (mStats) { mStats.shutdownLocked(); } @@ -237,7 +237,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub //Slog.i("foo", "SENDING BATTERY INFO:"); //mStats.dumpLocked(new LogPrinter(Log.INFO, "foo", Log.LOG_ID_SYSTEM)); Parcel out = Parcel.obtain(); - updateExternalStats("get-stats"); + updateExternalStats("get-stats", true); synchronized (mStats) { mStats.writeToParcel(out, 0); } @@ -252,7 +252,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub //Slog.i("foo", "SENDING BATTERY INFO:"); //mStats.dumpLocked(new LogPrinter(Log.INFO, "foo", Log.LOG_ID_SYSTEM)); Parcel out = Parcel.obtain(); - updateExternalStats("get-stats"); + updateExternalStats("get-stats", true); synchronized (mStats) { mStats.writeToParcel(out, 0); } @@ -779,7 +779,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub // Sync external stats first as the battery has changed states. If we don't sync // immediately here, we may not collect the relevant data later. - updateExternalStats("battery-state"); + updateExternalStats("battery-state", false); synchronized (mStats) { mStats.setBatteryStateLocked(status, health, plugType, level, temp, volt); } @@ -933,9 +933,9 @@ public final class BatteryStatsService extends IBatteryStats.Stub pw.println("Battery stats reset."); noOutput = true; } - updateExternalStats("dump"); + updateExternalStats("dump", true); } else if ("--write".equals(arg)) { - updateExternalStats("dump"); + updateExternalStats("dump", true); synchronized (mStats) { mStats.writeSyncLocked(); pw.println("Battery stats written."); @@ -999,7 +999,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub flags |= BatteryStats.DUMP_DEVICE_WIFI_ONLY; } // Fetch data from external sources and update the BatteryStatsImpl object with them. - updateExternalStats("dump"); + updateExternalStats("dump", true); } finally { Binder.restoreCallingIdentity(ident); } @@ -1142,8 +1142,16 @@ public final class BatteryStatsService extends IBatteryStats.Stub * * We first grab a lock specific to this method, then once all the data has been collected, * we grab the mStats lock and update the data. + * + * TODO(adamlesinski): When we start distributing bluetooth data to apps, we'll want to + * separate these external stats so that they can be collected individually and on different + * intervals. + * + * @param reason The reason why this collection was requested. Useful for debugging. + * @param force If false, some stats may decide not to be collected for efficiency as their + * results aren't needed immediately. When true, collect all stats unconditionally. */ - void updateExternalStats(String reason) { + void updateExternalStats(String reason, boolean force) { synchronized (mExternalStatsLock) { if (mContext == null) { // We haven't started yet (which means the BatteryStatsImpl object has @@ -1152,7 +1160,15 @@ public final class BatteryStatsService extends IBatteryStats.Stub } final WifiActivityEnergyInfo wifiEnergyInfo = pullWifiEnergyInfoLocked(); - final BluetoothActivityEnergyInfo bluetoothEnergyInfo = pullBluetoothEnergyInfoLocked(); + final BluetoothActivityEnergyInfo bluetoothEnergyInfo; + if (force) { + // We only pull bluetooth stats when we have to, as we are not distributing its + // use amongst apps and the sampling frequency does not matter. + bluetoothEnergyInfo = pullBluetoothEnergyInfoLocked(); + } else { + bluetoothEnergyInfo = null; + } + synchronized (mStats) { if (mStats.mRecordAllHistory) { final long elapsedRealtime = SystemClock.elapsedRealtime(); From 8a58802b4008abc867e2e20cfc0db5dde50c699c Mon Sep 17 00:00:00 2001 From: Vinit Deshpande Date: Wed, 20 May 2015 14:27:49 -0700 Subject: [PATCH 12/14] Indicate failed scans with EXTRA_RESULTS_UPDATED This flag indicates if scan was successful and results were updated. It will be set to false if a scan is not performed (intentionally) or if it failed to produce any results. Bug: 20642015 Change-Id: I06a1fdd684932db68891ee28d5a049980f483f0f --- api/current.txt | 1 + api/system-current.txt | 1 + wifi/java/android/net/wifi/WifiManager.java | 8 ++++++++ 3 files changed, 10 insertions(+) diff --git a/api/current.txt b/api/current.txt index bbdb878b288f9..c855b62ea77d5 100644 --- a/api/current.txt +++ b/api/current.txt @@ -19255,6 +19255,7 @@ package android.net.wifi { field public static final java.lang.String EXTRA_NEW_RSSI = "newRssi"; field public static final java.lang.String EXTRA_NEW_STATE = "newState"; field public static final java.lang.String EXTRA_PREVIOUS_WIFI_STATE = "previous_wifi_state"; + field public static final java.lang.String EXTRA_RESULTS_UPDATED = "resultsUpdated"; field public static final java.lang.String EXTRA_SUPPLICANT_CONNECTED = "connected"; field public static final java.lang.String EXTRA_SUPPLICANT_ERROR = "supplicantError"; field public static final java.lang.String EXTRA_WIFI_INFO = "wifiInfo"; diff --git a/api/system-current.txt b/api/system-current.txt index 1a3673d8548eb..e6883cd0b26da 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -21016,6 +21016,7 @@ package android.net.wifi { field public static final java.lang.String EXTRA_NEW_RSSI = "newRssi"; field public static final java.lang.String EXTRA_NEW_STATE = "newState"; field public static final java.lang.String EXTRA_PREVIOUS_WIFI_STATE = "previous_wifi_state"; + field public static final java.lang.String EXTRA_RESULTS_UPDATED = "resultsUpdated"; field public static final java.lang.String EXTRA_SUPPLICANT_CONNECTED = "connected"; field public static final java.lang.String EXTRA_SUPPLICANT_ERROR = "supplicantError"; field public static final java.lang.String EXTRA_WIFI_CONFIGURATION = "wifiConfiguration"; diff --git a/wifi/java/android/net/wifi/WifiManager.java b/wifi/java/android/net/wifi/WifiManager.java index f2c2a28764999..64fa0e5226e61 100644 --- a/wifi/java/android/net/wifi/WifiManager.java +++ b/wifi/java/android/net/wifi/WifiManager.java @@ -403,6 +403,14 @@ public class WifiManager { */ @SdkConstant(SdkConstantType.BROADCAST_INTENT_ACTION) public static final String SCAN_RESULTS_AVAILABLE_ACTION = "android.net.wifi.SCAN_RESULTS"; + + /** + * The result of previous scan, reported with {@link #SCAN_RESULTS_AVAILABLE_ACTION}. + * @return true scan was successful, results updated + * @return false scan was not successful, results haven't been updated since previous scan + */ + public static final String EXTRA_RESULTS_UPDATED = "resultsUpdated"; + /** * A batch of access point scans has been completed and the results areavailable. * Call {@link #getBatchedScanResults()} to obtain the results. From 16310ffdb9f00e704c17567e20b3f011bed5b271 Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Thu, 21 May 2015 15:04:18 -0700 Subject: [PATCH 13/14] BatteryStatsService: Only query bluetooth on demand. Bluetooth was being queried too often, leading to more power consumption and wakelock time. Bug:21063567 Bug:21269307 Change-Id: Idddbab46d13016ef8528e095945b7817c12f7266 --- .../server/am/BatteryStatsService.java | 38 +++++++++++++------ 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/am/BatteryStatsService.java b/services/core/java/com/android/server/am/BatteryStatsService.java index a61223a00cb61..b1cc2880e58fa 100644 --- a/services/core/java/com/android/server/am/BatteryStatsService.java +++ b/services/core/java/com/android/server/am/BatteryStatsService.java @@ -83,11 +83,11 @@ public final class BatteryStatsService extends IBatteryStats.Stub public void handleMessage(Message msg) { switch (msg.what) { case MSG_SYNC_EXTERNAL_STATS: - updateExternalStats((String)msg.obj); + updateExternalStats((String)msg.obj, false); break; case MSG_WRITE_TO_DISK: - updateExternalStats("write"); + updateExternalStats("write", true); synchronized (mStats) { mStats.writeAsyncLocked(); } @@ -137,7 +137,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub public void shutdown() { Slog.w("BatteryStats", "Writing battery stats before shutdown..."); - updateExternalStats("shutdown"); + updateExternalStats("shutdown", true); synchronized (mStats) { mStats.shutdownLocked(); } @@ -237,7 +237,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub //Slog.i("foo", "SENDING BATTERY INFO:"); //mStats.dumpLocked(new LogPrinter(Log.INFO, "foo", Log.LOG_ID_SYSTEM)); Parcel out = Parcel.obtain(); - updateExternalStats("get-stats"); + updateExternalStats("get-stats", true); synchronized (mStats) { mStats.writeToParcel(out, 0); } @@ -252,7 +252,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub //Slog.i("foo", "SENDING BATTERY INFO:"); //mStats.dumpLocked(new LogPrinter(Log.INFO, "foo", Log.LOG_ID_SYSTEM)); Parcel out = Parcel.obtain(); - updateExternalStats("get-stats"); + updateExternalStats("get-stats", true); synchronized (mStats) { mStats.writeToParcel(out, 0); } @@ -779,7 +779,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub // Sync external stats first as the battery has changed states. If we don't sync // immediately here, we may not collect the relevant data later. - updateExternalStats("battery-state"); + updateExternalStats("battery-state", false); synchronized (mStats) { mStats.setBatteryStateLocked(status, health, plugType, level, temp, volt); } @@ -933,9 +933,9 @@ public final class BatteryStatsService extends IBatteryStats.Stub pw.println("Battery stats reset."); noOutput = true; } - updateExternalStats("dump"); + updateExternalStats("dump", true); } else if ("--write".equals(arg)) { - updateExternalStats("dump"); + updateExternalStats("dump", true); synchronized (mStats) { mStats.writeSyncLocked(); pw.println("Battery stats written."); @@ -999,7 +999,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub flags |= BatteryStats.DUMP_DEVICE_WIFI_ONLY; } // Fetch data from external sources and update the BatteryStatsImpl object with them. - updateExternalStats("dump"); + updateExternalStats("dump", true); } finally { Binder.restoreCallingIdentity(ident); } @@ -1142,8 +1142,16 @@ public final class BatteryStatsService extends IBatteryStats.Stub * * We first grab a lock specific to this method, then once all the data has been collected, * we grab the mStats lock and update the data. + * + * TODO(adamlesinski): When we start distributing bluetooth data to apps, we'll want to + * separate these external stats so that they can be collected individually and on different + * intervals. + * + * @param reason The reason why this collection was requested. Useful for debugging. + * @param force If false, some stats may decide not to be collected for efficiency as their + * results aren't needed immediately. When true, collect all stats unconditionally. */ - void updateExternalStats(String reason) { + void updateExternalStats(String reason, boolean force) { synchronized (mExternalStatsLock) { if (mContext == null) { // We haven't started yet (which means the BatteryStatsImpl object has @@ -1152,7 +1160,15 @@ public final class BatteryStatsService extends IBatteryStats.Stub } final WifiActivityEnergyInfo wifiEnergyInfo = pullWifiEnergyInfoLocked(); - final BluetoothActivityEnergyInfo bluetoothEnergyInfo = pullBluetoothEnergyInfoLocked(); + final BluetoothActivityEnergyInfo bluetoothEnergyInfo; + if (force) { + // We only pull bluetooth stats when we have to, as we are not distributing its + // use amongst apps and the sampling frequency does not matter. + bluetoothEnergyInfo = pullBluetoothEnergyInfoLocked(); + } else { + bluetoothEnergyInfo = null; + } + synchronized (mStats) { if (mStats.mRecordAllHistory) { final long elapsedRealtime = SystemClock.elapsedRealtime(); From 2c838fbd87ad5685c0008b419ea02421159b9b70 Mon Sep 17 00:00:00 2001 From: Robert Shih Date: Thu, 21 May 2015 15:15:50 -0700 Subject: [PATCH 14/14] MediaPlayer: add mPreparing to weed out unwanted prepared messages Bug: 21266735 Change-Id: Ie4fe76533c9b7f505c57ba63df7992f2490942cc --- media/java/android/media/MediaPlayer.java | 33 +++++++++++++++++++++-- media/jni/android_media_MediaPlayer.cpp | 2 +- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/media/java/android/media/MediaPlayer.java b/media/java/android/media/MediaPlayer.java index f14860643c4b5..c24077eb20cd3 100644 --- a/media/java/android/media/MediaPlayer.java +++ b/media/java/android/media/MediaPlayer.java @@ -77,6 +77,7 @@ import java.util.Map; import java.util.Scanner; import java.util.Set; import java.util.Vector; +import java.util.concurrent.atomic.AtomicBoolean; import java.lang.ref.WeakReference; /** @@ -623,6 +624,9 @@ public class MediaPlayer implements SubtitleController.Listener private int mUsage = -1; private boolean mBypassInterruptionPolicy; + // use AtomicBoolean instead of boolean so we can use the same member both as a flag and a lock. + private AtomicBoolean mPreparing = new AtomicBoolean(); + /** * Default constructor. Consider using one of the create() methods for * synchronously instantiating a MediaPlayer from a Uri or resource. @@ -1162,6 +1166,10 @@ public class MediaPlayer implements SubtitleController.Listener * @throws IllegalStateException if it is called in an invalid state */ public void prepare() throws IOException, IllegalStateException { + // The synchronous version of prepare also recieves a MEDIA_PREPARED message. + synchronized (mPreparing) { + mPreparing.set(true); + } _prepare(); scanInternalSubtitleTracks(); } @@ -1178,7 +1186,14 @@ public class MediaPlayer implements SubtitleController.Listener * * @throws IllegalStateException if it is called in an invalid state */ - public native void prepareAsync() throws IllegalStateException; + public void prepareAsync() throws IllegalStateException { + synchronized (mPreparing) { + mPreparing.set(true); + } + _prepareAsync(); + } + + private native void _prepareAsync() throws IllegalStateException; /** * Starts or resumes playback. If playback had previously been paused, @@ -1229,6 +1244,9 @@ public class MediaPlayer implements SubtitleController.Listener * initialized. */ public void stop() throws IllegalStateException { + synchronized (mPreparing) { + mPreparing.set(false); + } stayAwake(false); _stop(); } @@ -1658,6 +1676,9 @@ public class MediaPlayer implements SubtitleController.Listener * at the same time. */ public void release() { + synchronized (mPreparing) { + mPreparing.set(false); + } stayAwake(false); updateSurfaceScreenOn(); mOnPreparedListener = null; @@ -1684,6 +1705,9 @@ public class MediaPlayer implements SubtitleController.Listener * data source and calling prepare(). */ public void reset() { + synchronized (mPreparing) { + mPreparing.set(false); + } mSelectedSubtitleTrackIndex = -1; synchronized(mOpenSubtitleSources) { for (final InputStream is: mOpenSubtitleSources) { @@ -2804,7 +2828,12 @@ public class MediaPlayer implements SubtitleController.Listener } switch(msg.what) { case MEDIA_PREPARED: - scanInternalSubtitleTracks(); + synchronized (mPreparing) { + if (mPreparing.get()) { + scanInternalSubtitleTracks(); + mPreparing.set(false); + } + } if (mOnPreparedListener != null) mOnPreparedListener.onPrepared(mMediaPlayer); return; diff --git a/media/jni/android_media_MediaPlayer.cpp b/media/jni/android_media_MediaPlayer.cpp index d8041f4bff044..9c6727869478e 100644 --- a/media/jni/android_media_MediaPlayer.cpp +++ b/media/jni/android_media_MediaPlayer.cpp @@ -1045,7 +1045,7 @@ static JNINativeMethod gMethods[] = { {"_setDataSource", "(Landroid/media/MediaDataSource;)V",(void *)android_media_MediaPlayer_setDataSourceCallback }, {"_setVideoSurface", "(Landroid/view/Surface;)V", (void *)android_media_MediaPlayer_setVideoSurface}, {"_prepare", "()V", (void *)android_media_MediaPlayer_prepare}, - {"prepareAsync", "()V", (void *)android_media_MediaPlayer_prepareAsync}, + {"_prepareAsync", "()V", (void *)android_media_MediaPlayer_prepareAsync}, {"_start", "()V", (void *)android_media_MediaPlayer_start}, {"_stop", "()V", (void *)android_media_MediaPlayer_stop}, {"getVideoWidth", "()I", (void *)android_media_MediaPlayer_getVideoWidth},