Commit c34ce1fb authored by Scott Violet's avatar Scott Violet Committed by Commit Bot

weblayer: more tab destruction cleanup

When the BrowserFragment is destroyed cleanup of the Tab in the
client library wasn't happening. This is because in this case
onTabRemoved is not called on the client, which was where cleanup
was being done.

This patch makes this work by handling the case in onTabDestroyed.

BUG=1134085
TEST=covered by tests

Change-Id: Ia36f5eeec75192b989114e329156c9cec558670c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2446780Reviewed-by: default avatarBo <boliu@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813733}
parent 152b2aea
...@@ -743,7 +743,9 @@ public class Tab { ...@@ -743,7 +743,9 @@ public class Tab {
// Called by Browser when removed. // Called by Browser when removed.
void onRemovedFromBrowser() { void onRemovedFromBrowser() {
if (mDestroyOnRemove) { if (mDestroyOnRemove) {
unregisterTab(this); // If this is destroyed as part of destroying the browser, then the tab has already been
// unregistered.
if (getTabById(mId) == this) unregisterTab(this);
mDestroyOnRemove = false; mDestroyOnRemove = false;
mImpl = null; mImpl = null;
} }
...@@ -809,9 +811,9 @@ public class Tab { ...@@ -809,9 +811,9 @@ public class Tab {
@Override @Override
public void onTabDestroyed() { public void onTabDestroyed() {
// Prior to 87 this was called *before* onTabRemoved(). Post 87 this is called after // Prior to 87 this was potentially called *before* onTabRemoved(). Post 87 this is
// onTabRemoved(). onTabRemoved() needs the Tab to be registered, so unregisterTab() // called after onTabRemoved(). onTabRemoved() needs the Tab to be registered, so
// should only be called in >= 87 (in < 87 it is called from // unregisterTab() should only be called in >= 87 (in < 87 it is called from
// Browser.prepareForDestroy()). // Browser.prepareForDestroy()).
if (WebLayer.getSupportedMajorVersionInternal() >= 87) { if (WebLayer.getSupportedMajorVersionInternal() >= 87) {
unregisterTab(Tab.this); unregisterTab(Tab.this);
...@@ -819,7 +821,12 @@ public class Tab { ...@@ -819,7 +821,12 @@ public class Tab {
// into the implementation via this Tab. // into the implementation via this Tab.
mImpl = null; mImpl = null;
} else { } else {
// This Tab should not have been destroyed yet.
assert mImpl != null;
mDestroyOnRemove = true; mDestroyOnRemove = true;
// If the tab isn't registered, it means the Browser fragment was destroyed, in
// which case there is no call to onTabRemoved().
if (getTabById(mId) == null) onRemovedFromBrowser();
} }
} }
......
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