Commit 4aa673fc authored by Pavel Shmakov's avatar Pavel Shmakov Committed by Commit Bot

Fix several activity leaks:

1) A listener of sync state changes was not always removed in ChromeActivity.
2) A static variable sCleanupTask in TabPersistensePolicy implementations held a reference to activity and was never nulled after completion/cancellation of the task.
3) ToolbarModel was never destroy()-ed.

Note that none of these issues are specific to Custom Tabs. They manifest themselves more severely with Custom Tabs since the CustomTabActivity gets destroyed when the tab is closed, whereas ChromeTabbedActivity is only stopped when the app is minimized.
However, with "don't keep activities" turned on, the same leakage is seen when opening and closing Chrome in ordinary way.

Bug: 872661

Change-Id: I5802690eb9d7a733426a7ea7ac3fd969c8514810
Reviewed-on: https://chromium-review.googlesource.com/1172425Reviewed-by: default avatarBernhard Bauer <bauerb@chromium.org>
Reviewed-by: default avatarYusuf Ozuysal <yusufo@chromium.org>
Commit-Queue: Pavel Shmakov <pshmakov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583998}
parent 7cf15f28
......@@ -296,10 +296,8 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
private ActivityTabStartupMetricsTracker mActivityTabStartupMetricsTracker;
/** Used to detect in {@link #postInflationStartup} if onStart was called in the mean time
* (while the inflation was being done on a background thread) and thus some onStart work that
* requires the UI to be inflated has to be done belatedly after inflation. */
private boolean mOnStartCalled;
/** Whether or not the activity is in started state. */
private boolean mStarted;
/**
* @param factory The {@link AppMenuHandlerFactory} for creating {@link #mAppMenuHandler}
......@@ -403,7 +401,7 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
// If onStart was called before postLayoutInflation (because inflation was done in a
// background thread) then make sure to call the relevant methods belatedly.
if (mOnStartCalled) {
if (mStarted) {
mCompositorViewHolder.onStart();
mSnackbarManager.onStart();
}
......@@ -889,6 +887,7 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
}
private void createContextReporterIfNeeded() {
if (!mStarted) return; // Sync state reporting should work only in started state.
if (mContextReporter != null || getActivityTab() == null) return;
final SyncController syncController = SyncController.get(this);
......@@ -976,13 +975,13 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
if (GSAState.getInstance(this).isGsaAvailable() && !SysUtils.isLowEndDevice()) {
GSAAccountChangeListener.getInstance().disconnect();
if (mSyncStateChangedListener != null) {
ProfileSyncService syncService = ProfileSyncService.get();
if (syncService != null) {
syncService.removeSyncStateChangedListener(mSyncStateChangedListener);
}
mSyncStateChangedListener = null;
}
if (mSyncStateChangedListener != null) {
ProfileSyncService syncService = ProfileSyncService.get();
if (syncService != null) {
syncService.removeSyncStateChangedListener(mSyncStateChangedListener);
}
mSyncStateChangedListener = null;
}
if (mContextReporter != null) mContextReporter.disable();
......@@ -1156,7 +1155,7 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
}
mScreenWidthDp = config.screenWidthDp;
mScreenHeightDp = config.screenHeightDp;
mOnStartCalled = true;
mStarted = true;
}
@Override
......@@ -1171,7 +1170,7 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
// If postInflationStartup hasn't been called yet (because inflation was done asynchronously
// and has not yet completed), it no longer needs to do the belated onStart code since we
// were stopped in the mean time.
mOnStartCalled = false;
mStarted = false;
}
@Override
......
......@@ -29,7 +29,6 @@ import java.util.List;
*/
@JNINamespace("android")
public class TabContentManager {
private final Context mContext;
private final float mThumbnailScale;
private final int mFullResThumbnailsMaxSize;
private final ContentOffsetProvider mContentOffsetProvider;
......@@ -75,32 +74,31 @@ public class TabContentManager {
*/
public TabContentManager(Context context, ContentOffsetProvider contentOffsetProvider,
boolean snapshotsEnabled) {
mContext = context;
mContentOffsetProvider = contentOffsetProvider;
mSnapshotsEnabled = snapshotsEnabled;
// Override the cache size on the command line with --thumbnails=100
int defaultCacheSize = getIntegerResourceWithOverride(mContext,
R.integer.default_thumbnail_cache_size, ChromeSwitches.THUMBNAILS);
int defaultCacheSize = getIntegerResourceWithOverride(
context, R.integer.default_thumbnail_cache_size, ChromeSwitches.THUMBNAILS);
mFullResThumbnailsMaxSize = defaultCacheSize;
int compressionQueueMaxSize = mContext.getResources().getInteger(
R.integer.default_compression_queue_size);
int writeQueueMaxSize = mContext.getResources().getInteger(
R.integer.default_write_queue_size);
int compressionQueueMaxSize =
context.getResources().getInteger(R.integer.default_compression_queue_size);
int writeQueueMaxSize =
context.getResources().getInteger(R.integer.default_write_queue_size);
// Override the cache size on the command line with
// --approximation-thumbnails=100
int approximationCacheSize = getIntegerResourceWithOverride(mContext,
int approximationCacheSize = getIntegerResourceWithOverride(context,
R.integer.default_approximation_thumbnail_cache_size,
ChromeSwitches.APPROXIMATION_THUMBNAILS);
float thumbnailScale = 1.f;
boolean useApproximationThumbnails;
DisplayAndroid display = DisplayAndroid.getNonMultiDisplay(mContext);
DisplayAndroid display = DisplayAndroid.getNonMultiDisplay(context);
float deviceDensity = display.getDipScale();
if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(mContext)) {
if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(context)) {
// Scale all tablets to MDPI.
thumbnailScale = 1.f / deviceDensity;
useApproximationThumbnails = false;
......
......@@ -393,6 +393,10 @@ public class CustomTabTabPersistencePolicy implements TabPersistencePolicy {
}
mFilesToDeleteCallback.onResult(filesToDelete);
synchronized (CLEAN_UP_TASK_LOCK) {
sCleanupTask = null; // Release static reference to external callback
}
}
private void getTabsFromStateFile(SparseBooleanArray tabIds, File metadataFile) {
......@@ -407,5 +411,13 @@ public class CustomTabTabPersistencePolicy implements TabPersistencePolicy {
StreamUtil.closeQuietly(stream);
}
}
@Override
protected void onCancelled(Void result) {
super.onCancelled(result);
synchronized (CLEAN_UP_TASK_LOCK) {
sCleanupTask = null;
}
}
}
}
......@@ -438,6 +438,10 @@ public class TabbedModeTabPersistencePolicy implements TabPersistencePolicy {
}
}
}
synchronized (CLEAN_UP_TASK_LOCK) {
sCleanupTask = null;
}
}
private boolean shouldDeleteTabFile(int tabId, TabWindowManager tabWindowManager) {
......@@ -471,5 +475,13 @@ public class TabbedModeTabPersistencePolicy implements TabPersistencePolicy {
}
}
}
@Override
protected void onCancelled(Void result) {
super.onCancelled(result);
synchronized (CLEAN_UP_TASK_LOCK) {
sCleanupTask = null;
}
}
}
}
......@@ -1059,6 +1059,8 @@ public class ToolbarManager implements ToolbarTabController, UrlFocusChangeListe
Tab currentTab = mToolbarModel.getTab();
if (currentTab != null) currentTab.removeObserver(mTabObserver);
mToolbar.destroy();
mToolbarModel.destroy();
mHandler.removeCallbacksAndMessages(null); // Cancel delayed tasks.
}
/**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment