Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Explicitly hold onto objects that are required for migration
Browse files Browse the repository at this point in the history
The TabContentManager was being garbage collected before migration
finished, resulting in native background threads attempting to
execute JNI on the dead TabContentManager.

More explicitly hold the TabContentManager inside of the FaviconImageCallback
(even though it was already doing so) as a member field instead, which seems
to prevent the GC from nuking it.

Possible reasoning on why the GC was being irrationally exuberant:
http://stackoverflow.com/questions/26642153/finalize-called-on-strongly-reachable-object-in-java-8

BUG=513130

Review URL: https://codereview.chromium.org/1260043007

Cr-Commit-Position: refs/heads/master@{#341971}
TBR=dtrainor

Review URL: https://codereview.chromium.org/1277593003 .

Cr-Commit-Position: refs/branch-heads/2454@{#240}
Cr-Branched-From: 12bfc33-refs/heads/master@{#338390}
  • Loading branch information
[email protected] committed Aug 5, 2015
1 parent b9e66b1 commit 1b83485
Showing 1 changed file with 42 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,45 @@ protected void updateEntryInfoFromTabState(Entry entry, TabState tabState) {
}
}

private static class MigrationImageCallback implements FaviconImageCallback {
private final TabContentManager mTabContentManager;
private final int mTabId;
private final DecompressThumbnailCallback mThumbnailCallback;
private Bitmap mFavicon;

public MigrationImageCallback(final AtomicInteger completedCount, final Activity activity,
final TabContentManager tabContentManager, final MigrationTabModel tabModel,
final int tabId, final String url, final String title, final int finalizeMode) {
mTabId = tabId;
mTabContentManager = tabContentManager;

// Even if the favicon or thumbnail are null, we still add the AppTask: the framework
// handles null favicons and addAppTask() below handles null thumbnails.
mThumbnailCallback = new DecompressThumbnailCallback() {
@Override
public void onFinishGetBitmap(Bitmap bitmap) {
if (!NativePageFactory.isNativePageUrl(url, false)
&& !url.startsWith(UrlConstants.CHROME_SCHEME)) {
addAppTask(activity, tabId,
tabModel.getTabStateForDocument(tabId),
url, title, mFavicon, bitmap);
}

if (completedCount.incrementAndGet() == tabModel.getCount()) {
mTabContentManager.destroy();
finalizeMigration(activity, finalizeMode);
}
}
};
}

@Override
public void onFaviconAvailable(final Bitmap favicon, String iconUrl) {
mFavicon = favicon;
mTabContentManager.getThumbnailForId(mTabId, mThumbnailCallback);
}
}

/**
* Migrates all tab state to classic mode and creates a tab model file using the current
* {@link DocumentTabModel} instances.
Expand Down Expand Up @@ -397,34 +436,11 @@ public int getOverlayTranslateY() {
final String url = currentUrl;
final String title = currentTitle;

FaviconImageCallback imageCallback = new MigrationImageCallback(completedCount,
activity, contentManager, tabModel, tabId, url, title, finalizeMode);
faviconHelper.getLargestRawFaviconForUrl(
Profile.getLastUsedProfile().getOriginalProfile(),
url, ICON_TYPES, DESIRED_ICON_SIZE_DP,
new FaviconImageCallback() {
@Override
public void onFaviconAvailable(final Bitmap favicon, String iconUrl) {
// Even if either the favicon or the thumbnail comes back null
// add the AppTask with the return values. The framework handles a null
// favicon and addAppTask below handles null thumbnails.
DecompressThumbnailCallback thumbnailCallback =
new DecompressThumbnailCallback() {
@Override
public void onFinishGetBitmap(Bitmap bitmap) {
if (!NativePageFactory.isNativePageUrl(url, false)
&& !url.startsWith(UrlConstants.CHROME_SCHEME)) {
addAppTask(activity, tabId,
tabModel.getTabStateForDocument(tabId),
url, title, favicon, bitmap);
}
if (completedCount.incrementAndGet() == tabModel.getCount()) {
contentManager.destroy();
finalizeMigration(activity, finalizeMode);
}
}
};
contentManager.getThumbnailForId(tabId, thumbnailCallback);
}
});
url, ICON_TYPES, DESIRED_ICON_SIZE_DP, imageCallback);
}
}

Expand Down

0 comments on commit 1b83485

Please sign in to comment.