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

Commit 67ff2481 authored by Selim Cinek's avatar Selim Cinek
Browse files

Fixed memory leak with the inflater

Because min priority children could be removed from
their parents after the removal, a new inflation task
would be started, leading to the view being instantly
readded again. This lead to memory leaks.

It also fixes a bug where the inflation would not inflate
enough views that could lead to crashes / wrong layouts.

Finally there was a indexing error when handling removal
of group summaries.

Test: runtest -x packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationInflaterTest.java
Change-Id: Iac242946bd30060967ee7877560d40e63f39f996
Fixes: 62067645
parent 605351c5
Loading
Loading
Loading
Loading
+32 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2017 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 com.android.systemui.statusbar;

/**
 * An interface for running inflation tasks that allows aborting and superseding existing
 * operations.
 */
public interface InflationTask {
    void abort();

    /**
     * Supersedes an existing task. i.e another task was superceeded by this.
     *
     * @param task the task that was previously running
     */
    default void supersedeTask(InflationTask task) {}
}
+7 −4
Original line number Diff line number Diff line
@@ -24,7 +24,6 @@ import android.content.pm.IPackageManager;
import android.content.pm.PackageManager;
import android.content.Context;
import android.graphics.drawable.Icon;
import android.os.AsyncTask;
import android.os.RemoteException;
import android.os.SystemClock;
import android.service.notification.NotificationListenerService;
@@ -84,7 +83,7 @@ public class NotificationData {
        public List<SnoozeCriterion> snoozeCriteria;
        private int mCachedContrastColor = COLOR_INVALID;
        private int mCachedContrastColorIsFor = COLOR_INVALID;
        private Abortable mRunningTask = null;
        private InflationTask mRunningTask = null;

        public Entry(StatusBarNotification n) {
            this.key = n.getKey();
@@ -225,10 +224,14 @@ public class NotificationData {
            }
        }

        public void setInflationTask(Abortable abortableTask) {
        public void setInflationTask(InflationTask abortableTask) {
            // abort any existing inflation
            InflationTask existing = mRunningTask;
            abortTask();
            mRunningTask = abortableTask;
            if (existing != null && mRunningTask != null) {
                mRunningTask.supersedeTask(existing);
            }
        }

        public void onInflationTaskFinished() {
@@ -236,7 +239,7 @@ public class NotificationData {
        }

        @VisibleForTesting
        public Abortable getRunningTask() {
        public InflationTask getRunningTask() {
            return mRunningTask;
        }
    }
+24 −5
Original line number Diff line number Diff line
@@ -28,7 +28,7 @@ import android.widget.RemoteViews;

import com.android.internal.annotations.VisibleForTesting;
import com.android.systemui.R;
import com.android.systemui.statusbar.Abortable;
import com.android.systemui.statusbar.InflationTask;
import com.android.systemui.statusbar.ExpandableNotificationRow;
import com.android.systemui.statusbar.NotificationContentView;
import com.android.systemui.statusbar.NotificationData;
@@ -122,6 +122,12 @@ public class NotificationInflater {
     */
    @VisibleForTesting
    void inflateNotificationViews(int reInflateFlags) {
        if (mRow.isRemoved()) {
            // We don't want to reinflate anything for removed notifications. Otherwise views might
            // be readded to the stack, leading to leaks. This may happen with low-priority groups
            // where the removal of already removed children can lead to a reinflation.
            return;
        }
        StatusBarNotification sbn = mRow.getEntry().notification;
        new AsyncInflationTask(sbn, reInflateFlags, mRow, mIsLowPriority,
                mIsChildInGroup, mUsesIncreasedHeight, mUsesIncreasedHeadsUpHeight, mRedactAmbient,
@@ -477,17 +483,17 @@ public class NotificationInflater {
    }

    public static class AsyncInflationTask extends AsyncTask<Void, Void, InflationProgress>
            implements InflationCallback, Abortable {
            implements InflationCallback, InflationTask {

        private final StatusBarNotification mSbn;
        private final Context mContext;
        private final int mReInflateFlags;
        private final boolean mIsLowPriority;
        private final boolean mIsChildInGroup;
        private final boolean mUsesIncreasedHeight;
        private final InflationCallback mCallback;
        private final boolean mUsesIncreasedHeadsUpHeight;
        private final boolean mRedactAmbient;
        private int mReInflateFlags;
        private ExpandableNotificationRow mRow;
        private Exception mError;
        private RemoteViews.OnClickHandler mRemoteViewClickHandler;
@@ -500,8 +506,6 @@ public class NotificationInflater {
                InflationCallback callback,
                RemoteViews.OnClickHandler remoteViewClickHandler) {
            mRow = row;
            NotificationData.Entry entry = row.getEntry();
            entry.setInflationTask(this);
            mSbn = notification;
            mReInflateFlags = reInflateFlags;
            mContext = mRow.getContext();
@@ -512,6 +516,13 @@ public class NotificationInflater {
            mRedactAmbient = redactAmbient;
            mRemoteViewClickHandler = remoteViewClickHandler;
            mCallback = callback;
            NotificationData.Entry entry = row.getEntry();
            entry.setInflationTask(this);
        }

        @VisibleForTesting
        public int getReInflateFlags() {
            return mReInflateFlags;
        }

        @Override
@@ -571,6 +582,14 @@ public class NotificationInflater {
            }
        }

        @Override
        public void supersedeTask(InflationTask task) {
            if (task instanceof AsyncInflationTask) {
                // We want to inflate all flags of the previous task as well
                mReInflateFlags |= ((AsyncInflationTask) task).mReInflateFlags;
            }
        }

        @Override
        public void handleInflationException(StatusBarNotification notification, Exception e) {
            handleError(e);
+2 −2
Original line number Diff line number Diff line
@@ -22,14 +22,14 @@ import android.view.View;
import android.view.ViewGroup;

import com.android.systemui.R;
import com.android.systemui.statusbar.Abortable;
import com.android.systemui.statusbar.InflationTask;
import com.android.systemui.statusbar.ExpandableNotificationRow;
import com.android.systemui.statusbar.NotificationData;

/**
 * An inflater task that asynchronously inflates a ExpandableNotificationRow
 */
public class RowInflaterTask implements Abortable, AsyncLayoutInflater.OnInflateFinishedListener {
public class RowInflaterTask implements InflationTask, AsyncLayoutInflater.OnInflateFinishedListener {
    private RowInflationFinishedListener mListener;
    private NotificationData.Entry mEntry;
    private boolean mCancelled;
+5 −3
Original line number Diff line number Diff line
@@ -1618,7 +1618,9 @@ public class StatusBar extends SystemUI implements DemoMode,
    @Override
    public void onAsyncInflationFinished(Entry entry) {
        mPendingNotifications.remove(entry.key);
        if (mNotificationData.get(entry.key) == null) {
        // If there was an async task started after the removal, we don't want to add it back to
        // the list, otherwise we might get leaks.
        if (mNotificationData.get(entry.key) == null && !entry.row.isRemoved()) {
            addEntry(entry);
        }
    }
@@ -1768,10 +1770,10 @@ public class StatusBar extends SystemUI implements DemoMode,
                    continue;
                }
                toRemove.add(row);
                toRemove.get(i).setKeepInParent(true);
                row.setKeepInParent(true);
                // we need to set this state earlier as otherwise we might generate some weird
                // animations
                toRemove.get(i).setRemoved();
                row.setRemoved();
            }
        }
    }
Loading