Merge "Fixed memory leak with the inflater" into oc-dev

am: 3d5fd7a959

Change-Id: I90d93918d1b188d09f6d2cf3ed70ceed38b07086
This commit is contained in:
Selim Cinek
2017-05-25 23:08:11 +00:00
committed by android-build-merger
6 changed files with 94 additions and 15 deletions

View File

@@ -0,0 +1,32 @@
/*
* 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) {}
}

View File

@@ -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;
}
}

View File

@@ -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;
@@ -130,6 +130,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,
@@ -485,17 +491,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;
@@ -508,8 +514,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();
@@ -520,6 +524,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
@@ -579,6 +590,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);

View File

@@ -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;

View File

@@ -1612,7 +1612,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);
}
}
@@ -1762,10 +1764,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();
}
}
}

View File

@@ -37,6 +37,7 @@ import com.android.systemui.SysuiTestCase;
import com.android.systemui.statusbar.ExpandableNotificationRow;
import com.android.systemui.statusbar.NotificationData;
import com.android.systemui.statusbar.NotificationTestHelper;
import com.android.systemui.statusbar.InflationTask;
import org.junit.Assert;
import org.junit.Before;
@@ -130,7 +131,29 @@ public class NotificationInflaterTest extends SysuiTestCase {
mRow.getEntry().abortTask();
runThenWaitForInflation(() -> mNotificationInflater.inflateNotificationViews(),
mNotificationInflater);
Assert.assertNull(mRow.getEntry().getRunningTask() );
verify(mRow).onNotificationUpdated();
}
@Test
public void testRemovedNotInflated() throws Exception {
mRow.setRemoved();
mNotificationInflater.inflateNotificationViews();
Assert.assertNull(mRow.getEntry().getRunningTask());
}
@Test
public void testSupersedesExistingTask() throws Exception {
mNotificationInflater.inflateNotificationViews();
mNotificationInflater.setIsLowPriority(true);
mNotificationInflater.setIsChildInGroup(true);
InflationTask runningTask = mRow.getEntry().getRunningTask();
NotificationInflater.AsyncInflationTask asyncInflationTask =
(NotificationInflater.AsyncInflationTask) runningTask;
Assert.assertSame("Successive inflations don't inherit the previous flags!",
asyncInflationTask.getReInflateFlags(),
NotificationInflater.FLAG_REINFLATE_ALL);
runningTask.abort();
}
public static void runThenWaitForInflation(Runnable block,