Skip to content

Commit

Permalink
Fix three crashes in the recent tabs dialog.
Browse files Browse the repository at this point in the history
This fixes a crash in the recent tabs dialog when clicking on a recently
closed tab and clicking the dialog's "X" button almost simultaneously.

The crash happened because when "X" is clicked, the dialog would be
dismissed and the RecentTabsPage would be destroyed. Soon thereafter,
the click on the recently closed item would be processed and it would
try to use the RecentTabsPage object.

This CL fixes the crash by tracking whether the RecentTabsManager has
been destroyed and ignoring clicks that happen after destroy has been
called. This also fixes an identical crash with clicking a foreign
session tab, and a related (but rarer) crash where sync or sign-in
related events were being processed after the RecentTabsManager was
destroyed.

BUG=567891

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

Cr-Commit-Position: refs/heads/master@{#365075}
(cherry picked from commit 7f87dbd)

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

Cr-Commit-Position: refs/branch-heads/2564@{crosswalk-project#352}
Cr-Branched-From: 1283eca-refs/heads/master@{#359700}
  • Loading branch information
Newton Allen committed Dec 14, 2015
1 parent 6a618f5 commit ed6983c
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ public void openForeignSessionTab(final ForeignSession session, final ForeignSes
ThreadUtils.postOnUiThreadDelayed(new Runnable() {
@Override
public void run() {
if (isDestroyed()) return;
DocumentRecentTabsManager.super.openForeignSessionTab(
session, tab, WindowOpenDisposition.NEW_FOREGROUND_TAB);
if (mDialog != null) mDialog.dismiss();
Expand All @@ -131,6 +132,7 @@ public void openRecentlyClosedTab(final RecentlyClosedTab tab, int windowDisposi
ThreadUtils.postOnUiThreadDelayed(new Runnable() {
@Override
public void run() {
if (isDestroyed()) return;
DocumentRecentTabsManager.super.openRecentlyClosedTab(
tab, WindowOpenDisposition.NEW_FOREGROUND_TAB);
if (mDialog != null) mDialog.dismiss();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ interface UpdatedCallback {
private SigninManager mSignInManager;
private UpdatedCallback mUpdatedCallback;
private ProfileDataCache mProfileDataCache;
private boolean mIsDestroyed;

/**
* Create an RecentTabsManager to be used with RecentTabsPage and RecentTabsRowAdapter.
Expand Down Expand Up @@ -98,6 +99,7 @@ public RecentTabsManager(Tab tab, Profile profile, Context context) {
* Should be called when this object is no longer needed. Performs necessary listener tear down.
*/
public void destroy() {
mIsDestroyed = true;
AndroidSyncSettings.unregisterObserver(mContext, this);

mSignInManager.removeSignInStateObserver(this);
Expand Down Expand Up @@ -125,6 +127,13 @@ public void destroy() {
InvalidationController.get(mContext).onRecentTabsPageClosed();
}

/**
* Returns true if destroy() has been called.
*/
public boolean isDestroyed() {
return mIsDestroyed;
}

private static ForeignSessionHelper buildForeignSessionHelper(Profile profile) {
return new ForeignSessionHelper(profile);
}
Expand Down Expand Up @@ -420,6 +429,7 @@ public void androidSyncSettingsChanged() {
ThreadUtils.runOnUiThread(new Runnable() {
@Override
public void run() {
if (mIsDestroyed) return;
updateForeignSessions();
postUpdate();
for (AndroidSyncSettingsObserver observer : mObservers) {
Expand Down

0 comments on commit ed6983c

Please sign in to comment.