Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4fce120b authored by Tony Mak's avatar Tony Mak
Browse files

Fix NPE in logActionClick 

Problem:
The issue could happen when clicking on the archive button on the
notification from Gmail repeatedly and quickly. Tapping the Archive
button will change the notification layout to something custom
with a single UNDO button. The bug happens when the notification object
is already updated, while logActionClick of the old action button is
still running.

Proposed solution:
1. Add sanity check to avoid NPE, array index out of bounds.
2. To ensure that the action object is  the one we want, use PendingIntent in
Notification.Action as a token.

logActionClick is just used as logging, and the problem seems only happen
in repetitive clicks, so just bailing out should be fine.

Test: Have a Gmail notification, taps archive button repeatedly.
      Observe no crash.
Test: Ensure that log is sent in normal cases by checking logcat.

FIXES: 128804769

Change-Id: Ic04fd9288cba5253179bf1f478a7454e44f31703
parent 3acdae62

packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java

+16 −3
Original line number Diff line number Diff line
@@ -63,6 +63,7 @@ import com.android.systemui.statusbar.policy.RemoteInputView; 
import java.io.FileDescriptor;

import java.io.PrintWriter;

import java.util.ArrayList;

import java.util.Objects;

import java.util.Set;



import javax.inject.Inject;
@@ -142,7 +143,7 @@ public class NotificationRemoteInputManager implements Dumpable { 
            if (DEBUG) {

                Log.v(TAG, "Notification click handler invoked for intent: " + pendingIntent);

            }

            logActionClick(view);

            logActionClick(view, pendingIntent);

            // The intent we are sending is for the application, which

            // won't have permission to immediately start an activity after

            // the user switches to home.  We know it is safe to do at this
@@ -159,11 +160,11 @@ public class NotificationRemoteInputManager implements Dumpable { 
            });

        }



        private void logActionClick(View view) {

        private void logActionClick(View view, PendingIntent actionIntent) {

            Integer actionIndex = (Integer)

                    view.getTag(com.android.internal.R.id.notification_action_index_tag);

            if (actionIndex == null) {

                Log.e(TAG, "Couldn't retrieve the actionIndex from the clicked button");

                // Custom action button, not logging.

                return;

            }

            ViewParent parent = view.getParent();
@@ -182,8 +183,20 @@ public class NotificationRemoteInputManager implements Dumpable { 
            }

            final int count = mEntryManager.getNotificationData().getActiveNotifications().size();

            final int rank = mEntryManager.getNotificationData().getRank(key);



            // Notification may be updated before this function is executed, and thus play safe

            // here and verify that the action object is still the one that where the click happens.

            Notification.Action[] actions = statusBarNotification.getNotification().actions;

            if (actions == null || actionIndex >= actions.length) {

                Log.w(TAG, "statusBarNotification.getNotification().actions is null or invalid");

                return;

            }

            final Notification.Action action =

                    statusBarNotification.getNotification().actions[actionIndex];

            if (Objects.equals(action.actionIntent, actionIntent)) {

                Log.w(TAG, "actionIntent does not match");

                return;

            }

            NotificationVisibility.NotificationLocation location =

                    NotificationLogger.getNotificationLocation(

                            mEntryManager.getNotificationData().get(key));