Commit 79db916e authored by Scott Violet's avatar Scott Violet Committed by Commit Bot

weblayer: fix tab destruction

This is a regression from
https://chromium-review.googlesource.com/c/chromium/src/+/2411161 .
That patch meant that a newer client run with an implementation older than
86 would not properly clean up Tab's static map. This meant any calls to
Tab.getTabsInBrowser would throw an exception because the tab was
marked as destroyed but not removed from the map.

The skew tests cover this.

BUG=1131953
TEST=covered by skew tests

Change-Id: I8f9ce175b316e75cfd752c2afa587ea3ad517393
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2442001Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812742}
parent cbf50724
...@@ -388,6 +388,7 @@ public class Browser { ...@@ -388,6 +388,7 @@ public class Browser {
for (TabListCallback callback : mTabListCallbacks) { for (TabListCallback callback : mTabListCallbacks) {
callback.onTabRemoved(tab); callback.onTabRemoved(tab);
} }
tab.onRemovedFromBrowser();
} }
@Override @Override
......
...@@ -59,6 +59,10 @@ public class Tab { ...@@ -59,6 +59,10 @@ public class Tab {
// Id from the remote side. // Id from the remote side.
private final int mId; private final int mId;
// See comments in onTabDestroyed() for details.
// TODO(sky): this is necessary for < 87. Remove in 90.
private boolean mDestroyOnRemove;
// Constructor for test mocking. // Constructor for test mocking.
protected Tab() { protected Tab() {
mImpl = null; mImpl = null;
...@@ -728,6 +732,15 @@ public class Tab { ...@@ -728,6 +732,15 @@ public class Tab {
} }
} }
// Called by Browser when removed.
void onRemovedFromBrowser() {
if (mDestroyOnRemove) {
unregisterTab(this);
mDestroyOnRemove = false;
mImpl = null;
}
}
private static final class WebMessageCallbackClientImpl extends IWebMessageCallbackClient.Stub { private static final class WebMessageCallbackClientImpl extends IWebMessageCallbackClient.Stub {
private final WebMessageCallback mCallback; private final WebMessageCallback mCallback;
// Maps from id of IWebMessageReplyProxy to WebMessageReplyProxy. This is done to avoid AIDL // Maps from id of IWebMessageReplyProxy to WebMessageReplyProxy. This is done to avoid AIDL
...@@ -794,10 +807,12 @@ public class Tab { ...@@ -794,10 +807,12 @@ public class Tab {
// Browser.prepareForDestroy()). // Browser.prepareForDestroy()).
if (WebLayer.getSupportedMajorVersionInternal() >= 87) { if (WebLayer.getSupportedMajorVersionInternal() >= 87) {
unregisterTab(Tab.this); unregisterTab(Tab.this);
// Ensure that the app will fail fast if the embedder mistakenly tries to call back
// into the implementation via this Tab.
mImpl = null;
} else {
mDestroyOnRemove = true;
} }
// Ensure that the app will fail fast if the embedder mistakenly tries to call back
// into the implementation via this Tab.
mImpl = null;
} }
@Override @Override
......
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