From 4e78706f439d318ae7a78927d98f734351a89f64 Mon Sep 17 00:00:00 2001 From: Dan Sandler Date: Wed, 17 Jun 2015 15:09:48 -0400 Subject: [PATCH] Patch up certain kinds of broken notifications. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Notifications in which the icon resource ID is changed after Builder.build() is called (even, and particularly, as the last step in the current implementation of setLatestEventInfo()) were not having their icons properly parceled. In these cases we now attempt to catch this at parcel time and construct the necessary Icon object. But wait! Parceling does not require a Context. So we don't actually know which package to load the resource from. Therefore we now allow an Icon to be constructed with an empty ("") package name, which allows us to complete this parceling task despite the fact that a Notification does not know its own package name. (In case you attempt to load a drawable for such an Icon, loadDrawable will spot the "" package and instead substitute the Context from its parameters to try to load the resource.) As it happens, even though the Notification does not know its own package name, BaseStatusBar does, because it was provided at NM.notify() time and is therefore included in the StatusBarNotification structure. So we can actually patch up the Icon (if it is TYPE_RESOURCE) and be sure to get the icon loaded out of the correct package. While we've got the hood open, this change fixes a couple of related problems: • Foreground service notifications synthetically constructed for naughty icon==0 notifications (which we are still allowing...FOR NOW) were losing the FLAG_FOREGROUND_SERVICE flag (because we're re-build()-ing them from scratch rather than rewriting the provided Notification object). Now we set the flag and hang onto the new notification for next time setForeground() is called. • We now allow media notifications to avoid getting bumped to the top of the notification list if they're PRIORITY_MIN. You might want to do that, I guess? Bug: 21333763 Change-Id: Ia5d1f1acb594c7677bcc75ee3d624da4ffca671f --- core/java/android/app/Notification.java | 14 +++++++-- .../java/android/app/NotificationManager.java | 7 +++++ .../internal/statusbar/StatusBarIcon.java | 26 ++++++++++++---- .../java/android/graphics/drawable/Icon.java | 30 +++++++++++++------ .../systemui/statusbar/BaseStatusBar.java | 13 +++++--- .../systemui/statusbar/NotificationData.java | 18 +++++++---- .../com/android/server/am/ServiceRecord.java | 13 ++++++++ 7 files changed, 94 insertions(+), 27 deletions(-) diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java index 33a47b24b79de..5a0d246afc648 100644 --- a/core/java/android/app/Notification.java +++ b/core/java/android/app/Notification.java @@ -1371,6 +1371,9 @@ public class Notification implements Parcelable when = parcel.readLong(); if (parcel.readInt() != 0) { mSmallIcon = Icon.CREATOR.createFromParcel(parcel); + if (mSmallIcon.getType() == Icon.TYPE_RESOURCE) { + icon = mSmallIcon.getResId(); + } } number = parcel.readInt(); if (parcel.readInt() != 0) { @@ -1588,13 +1591,17 @@ public class Notification implements Parcelable } /** - * Flatten this notification from a parcel. + * Flatten this notification into a parcel. */ public void writeToParcel(Parcel parcel, int flags) { parcel.writeInt(1); parcel.writeLong(when); + if (mSmallIcon == null && icon != 0) { + // you snuck an icon in here without using the builder; let's try to keep it + mSmallIcon = Icon.createWithResource("", icon); + } if (mSmallIcon != null) { parcel.writeInt(1); mSmallIcon.writeToParcel(parcel, 0); @@ -2791,7 +2798,10 @@ public class Notification implements Parcelable return this; } - private void setFlag(int mask, boolean value) { + /** + * @hide + */ + public void setFlag(int mask, boolean value) { if (value) { mFlags |= mask; } else { diff --git a/core/java/android/app/NotificationManager.java b/core/java/android/app/NotificationManager.java index 0904e216d0818..605c006130878 100644 --- a/core/java/android/app/NotificationManager.java +++ b/core/java/android/app/NotificationManager.java @@ -25,6 +25,7 @@ import android.content.Context; import android.content.pm.ParceledListSlice; import android.graphics.drawable.Icon; import android.net.Uri; +import android.os.Build; import android.os.Bundle; import android.os.Handler; import android.os.IBinder; @@ -216,6 +217,12 @@ public class NotificationManager } } fixLegacySmallIcon(notification, pkg); + if (mContext.getApplicationInfo().targetSdkVersion > Build.VERSION_CODES.LOLLIPOP_MR1) { + if (notification.getSmallIcon() == null) { + throw new IllegalArgumentException("Invalid notification (no valid small icon): " + + notification); + } + } if (localLOGV) Log.v(TAG, pkg + ": notify(" + id + ", " + notification + ")"); Notification stripped = notification.clone(); Builder.stripForDelivery(stripped); diff --git a/core/java/com/android/internal/statusbar/StatusBarIcon.java b/core/java/com/android/internal/statusbar/StatusBarIcon.java index 4693d4b7c840c..1d626235c4d2b 100644 --- a/core/java/com/android/internal/statusbar/StatusBarIcon.java +++ b/core/java/com/android/internal/statusbar/StatusBarIcon.java @@ -20,17 +20,27 @@ import android.graphics.drawable.Icon; import android.os.Parcel; import android.os.Parcelable; import android.os.UserHandle; +import android.text.TextUtils; public class StatusBarIcon implements Parcelable { public UserHandle user; + public String pkg; public Icon icon; public int iconLevel; public boolean visible = true; public int number; public CharSequence contentDescription; - public StatusBarIcon(UserHandle user, Icon icon, int iconLevel, int number, + public StatusBarIcon(UserHandle user, String resPackage, Icon icon, int iconLevel, int number, CharSequence contentDescription) { + if (icon.getType() == Icon.TYPE_RESOURCE + && TextUtils.isEmpty(icon.getResPackage())) { + // This is an odd situation where someone's managed to hand us an icon without a + // package inside, probably by mashing an int res into a Notification object. + // Now that we have the correct package name handy, let's fix it. + icon = Icon.createWithResource(resPackage, icon.getResId()); + } + this.pkg = resPackage; this.user = user; this.icon = icon; this.iconLevel = iconLevel; @@ -41,21 +51,23 @@ public class StatusBarIcon implements Parcelable { public StatusBarIcon(String iconPackage, UserHandle user, int iconId, int iconLevel, int number, CharSequence contentDescription) { - this(user, Icon.createWithResource(iconPackage, iconId), + this(user, iconPackage, Icon.createWithResource(iconPackage, iconId), iconLevel, number, contentDescription); } @Override public String toString() { - return "StatusBarIcon(icon=" + this.icon + return "StatusBarIcon(icon=" + icon + + ((iconLevel != 0)?(" level=" + iconLevel):"") + + (visible?" visible":"") + " user=" + user.getIdentifier() - + " level=" + this.iconLevel + " visible=" + visible - + " num=" + this.number + " )"; + + ((number != 0)?(" num=" + number):"") + + " )"; } @Override public StatusBarIcon clone() { - StatusBarIcon that = new StatusBarIcon(this.user, this.icon, + StatusBarIcon that = new StatusBarIcon(this.user, this.pkg, this.icon, this.iconLevel, this.number, this.contentDescription); that.visible = this.visible; return that; @@ -70,6 +82,7 @@ public class StatusBarIcon implements Parcelable { public void readFromParcel(Parcel in) { this.icon = (Icon) in.readParcelable(null); + this.pkg = in.readString(); this.user = (UserHandle) in.readParcelable(null); this.iconLevel = in.readInt(); this.visible = in.readInt() != 0; @@ -79,6 +92,7 @@ public class StatusBarIcon implements Parcelable { public void writeToParcel(Parcel out, int flags) { out.writeParcelable(this.icon, 0); + out.writeString(this.pkg); out.writeParcelable(this.user, 0); out.writeInt(this.iconLevel); out.writeInt(this.visible ? 1 : 0); diff --git a/graphics/java/android/graphics/drawable/Icon.java b/graphics/java/android/graphics/drawable/Icon.java index 7b4329a519b77..85db6a1e006ef 100644 --- a/graphics/java/android/graphics/drawable/Icon.java +++ b/graphics/java/android/graphics/drawable/Icon.java @@ -29,6 +29,7 @@ import android.os.Handler; import android.os.Message; import android.os.Parcel; import android.os.Parcelable; +import android.text.TextUtils; import android.util.Log; import java.io.DataInputStream; @@ -258,16 +259,21 @@ public final class Icon implements Parcelable { return new BitmapDrawable(context.getResources(), getBitmap()); case TYPE_RESOURCE: if (getResources() == null) { - if (getResPackage() == null || "android".equals(getResPackage())) { + // figure out where to load resources from + String resPackage = getResPackage(); + if (TextUtils.isEmpty(resPackage)) { + // if none is specified, try the given context + resPackage = context.getPackageName(); + } + if ("android".equals(resPackage)) { mObj1 = Resources.getSystem(); } else { final PackageManager pm = context.getPackageManager(); try { - mObj1 = pm.getResourcesForApplication(getResPackage()); + mObj1 = pm.getResourcesForApplication(resPackage); } catch (PackageManager.NameNotFoundException e) { - Log.e(TAG, String.format("Unable to find pkg=%s", - getResPackage()), - e); + Log.e(TAG, String.format("Unable to find pkg=%s for icon %s", + resPackage, this), e); break; } } @@ -320,12 +326,15 @@ public final class Icon implements Parcelable { */ public Drawable loadDrawableAsUser(Context context, int userId) { if (mType == TYPE_RESOURCE) { - if (getResources() == null - && getResPackage() != null - && !(getResPackage().equals("android"))) { + String resPackage = getResPackage(); + if (TextUtils.isEmpty(resPackage)) { + resPackage = context.getPackageName(); + } + if (getResources() == null && !(getResPackage().equals("android"))) { final PackageManager pm = context.getPackageManager(); try { - mObj1 = pm.getResourcesForApplicationAsUser(getResPackage(), userId); + // assign getResources() as the correct user + mObj1 = pm.getResourcesForApplicationAsUser(resPackage, userId); } catch (PackageManager.NameNotFoundException e) { Log.e(TAG, String.format("Unable to find pkg=%s user=%d", getResPackage(), @@ -410,6 +419,9 @@ public final class Icon implements Parcelable { * @param resId ID of the drawable resource */ public static Icon createWithResource(Context context, @DrawableRes int resId) { + if (context == null) { + throw new IllegalArgumentException("Context must not be null."); + } final Icon rep = new Icon(TYPE_RESOURCE); rep.mInt1 = resId; rep.mString1 = context.getPackageName(); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/BaseStatusBar.java b/packages/SystemUI/src/com/android/systemui/statusbar/BaseStatusBar.java index 79761ec790284..a4965488e34dd 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/BaseStatusBar.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/BaseStatusBar.java @@ -41,6 +41,7 @@ import android.content.res.Resources; import android.database.ContentObserver; import android.graphics.PorterDuff; import android.graphics.drawable.Drawable; +import android.graphics.drawable.Icon; import android.os.AsyncTask; import android.os.Build; import android.os.Handler; @@ -1396,6 +1397,7 @@ public abstract class BaseStatusBar extends SystemUI implements final StatusBarIcon ic = new StatusBarIcon( entry.notification.getUser(), + entry.notification.getPackageName(), entry.notification.getNotification().getSmallIcon(), entry.notification.getNotification().iconLevel, entry.notification.getNotification().number, @@ -1682,10 +1684,11 @@ public abstract class BaseStatusBar extends SystemUI implements final StatusBarIcon ic = new StatusBarIcon( sbn.getUser(), - n.getSmallIcon(), - n.iconLevel, - n.number, - n.tickerText); + sbn.getPackageName(), + n.getSmallIcon(), + n.iconLevel, + n.number, + n.tickerText); if (!iconView.set(ic)) { handleNotificationError(sbn, "Couldn't create icon: " + ic); return null; @@ -1825,6 +1828,7 @@ public abstract class BaseStatusBar extends SystemUI implements // Update the icon final StatusBarIcon ic = new StatusBarIcon( notification.getUser(), + notification.getPackageName(), n.getSmallIcon(), n.iconLevel, n.number, @@ -1847,6 +1851,7 @@ public abstract class BaseStatusBar extends SystemUI implements if (DEBUG) Log.d(TAG, "not reusing notification for key: " + key); final StatusBarIcon ic = new StatusBarIcon( notification.getUser(), + notification.getPackageName(), n.getSmallIcon(), n.iconLevel, n.number, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationData.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationData.java index dbabe3f610253..aedae521a7d48 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationData.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationData.java @@ -125,16 +125,22 @@ public class NotificationData { @Override public int compare(Entry a, Entry b) { - String mediaNotification = mEnvironment.getCurrentMediaNotificationKey(); - boolean aMedia = a.key.equals(mediaNotification); - boolean bMedia = b.key.equals(mediaNotification); - final StatusBarNotification na = a.notification; final StatusBarNotification nb = b.notification; + final int aPriority = na.getNotification().priority; + final int bPriority = nb.getNotification().priority; - boolean aSystemMax = na.getNotification().priority >= Notification.PRIORITY_MAX && + String mediaNotification = mEnvironment.getCurrentMediaNotificationKey(); + + // PRIORITY_MIN media streams are allowed to drift to the bottom + final boolean aMedia = a.key.equals(mediaNotification) + && aPriority > Notification.PRIORITY_MIN; + final boolean bMedia = b.key.equals(mediaNotification) + && bPriority > Notification.PRIORITY_MIN; + + boolean aSystemMax = aPriority >= Notification.PRIORITY_MAX && isSystemNotification(na); - boolean bSystemMax = nb.getNotification().priority >= Notification.PRIORITY_MAX && + boolean bSystemMax = bPriority >= Notification.PRIORITY_MAX && isSystemNotification(nb); int d = nb.getScore() - na.getScore(); diff --git a/services/core/java/com/android/server/am/ServiceRecord.java b/services/core/java/com/android/server/am/ServiceRecord.java index 236af375ffae7..87cb40ea3e9fd 100644 --- a/services/core/java/com/android/server/am/ServiceRecord.java +++ b/services/core/java/com/android/server/am/ServiceRecord.java @@ -445,6 +445,11 @@ final class ServiceRecord extends Binder { // icon, but this used to be able to slip through, so for // those dirty apps we will create a notification clearly // blaming the app. + Slog.v(TAG, "Attempted to start a foreground service (" + + name + + ") with a broken notification (no icon: " + + localForegroundNoti + + ")"); CharSequence appName = appInfo.loadLabel( ams.mContext.getPackageManager()); @@ -461,6 +466,12 @@ final class ServiceRecord extends Binder { // it's ugly, but it clearly identifies the app notiBuilder.setSmallIcon(appInfo.icon); + // mark as foreground + notiBuilder.setFlag(Notification.FLAG_FOREGROUND_SERVICE, true); + + // we are doing the app a kindness here + notiBuilder.setPriority(Notification.PRIORITY_MIN); + Intent runningIntent = new Intent( Settings.ACTION_APPLICATION_DETAILS_SETTINGS); runningIntent.setData(Uri.fromParts("package", @@ -498,6 +509,8 @@ final class ServiceRecord extends Binder { nm.enqueueNotification(localPackageName, localPackageName, appUid, appPid, null, localForegroundId, localForegroundNoti, outId, userId); + + foregroundNoti = localForegroundNoti; // save it for amending next time } catch (RuntimeException e) { Slog.w(TAG, "Error showing notification for service", e); // If it gave us a garbage notification, it doesn't