Commit d9836842 authored by boliu's avatar boliu Committed by Commit bot

aw: Cancel pending callbacks after native destroy

Normal destroy already calls removeCallbacksAndMessages. This Cl covers
when the CleanupReference runs before the java side is garbage
collected.

Add a test for this, and stop this test suite repeating since they are
only unit tests.

Review-Url: https://codereview.chromium.org/2292393002
Cr-Commit-Position: refs/heads/master@{#415629}
parent e283632b
......@@ -759,6 +759,13 @@ public class AwContents implements SmartClipProvider,
mInitialFunctor = new AwGLFunctor(mNativeDrawGLFunctorFactory, mContainerView);
mCurrentFunctor = mInitialFunctor;
mContentsClient = contentsClient;
mContentsClient.getCallbackHelper().setCancelCallbackPoller(
new AwContentsClientCallbackHelper.CancelCallbackPoller() {
@Override
public boolean cancelAllCallbacks() {
return AwContents.this.isDestroyed(NO_WARN);
}
});
mAwViewMethods = new AwViewMethodsImpl();
mFullScreenTransitionsState = new FullScreenTransitionsState(
mContainerView, mInternalAccessAdapter, mAwViewMethods);
......@@ -1218,12 +1225,7 @@ public class AwContents implements SmartClipProvider,
* @return whether this instance of WebView is flagged as destroyed.
*/
private boolean isDestroyed(int warnIfDestroyed) {
if (!mIsDestroyed) {
assert mContentViewCore != null;
assert mWebContents != null;
assert mNavigationController != null;
assert mNativeAwContents != 0;
} else if (warnIfDestroyed == WARN) {
if (mIsDestroyed && warnIfDestroyed == WARN) {
Log.w(TAG, "Application attempted to call on a destroyed WebView", new Throwable());
}
boolean destroyRunnableHasRun =
......
......@@ -22,6 +22,12 @@ import java.util.concurrent.Callable;
*/
@VisibleForTesting
public class AwContentsClientCallbackHelper {
/**
* Interface to tell CallbackHelper to cancel posted callbacks.
*/
public static interface CancelCallbackPoller {
boolean cancelAllCallbacks();
}
// TODO(boliu): Consider removing DownloadInfo and LoginRequestInfo by using native
// MessageLoop to post directly to AwContents.
......@@ -126,6 +132,8 @@ public class AwContentsClientCallbackHelper {
private final Handler mHandler;
private CancelCallbackPoller mCancelCallbackPoller;
private class MyHandler extends Handler {
private MyHandler(Looper looper) {
super(looper);
......@@ -133,6 +141,11 @@ public class AwContentsClientCallbackHelper {
@Override
public void handleMessage(Message msg) {
if (mCancelCallbackPoller != null && mCancelCallbackPoller.cancelAllCallbacks()) {
removeCallbacksAndMessages(null);
return;
}
switch(msg.what) {
case MSG_ON_LOAD_RESOURCE: {
final String url = (String) msg.obj;
......@@ -227,6 +240,11 @@ public class AwContentsClientCallbackHelper {
mContentsClient = contentsClient;
}
// Public for tests.
public void setCancelCallbackPoller(CancelCallbackPoller poller) {
mCancelCallbackPoller = poller;
}
public void postOnLoadResource(String url) {
mHandler.sendMessage(mHandler.obtainMessage(MSG_ON_LOAD_RESOURCE, url));
}
......
......@@ -15,6 +15,7 @@ import org.chromium.android_webview.test.TestAwContentsClient.OnDownloadStartHel
import org.chromium.android_webview.test.TestAwContentsClient.OnReceivedLoginRequestHelper;
import org.chromium.android_webview.test.TestAwContentsClient.PictureListenerHelper;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.parameter.ParameterizedTest;
import org.chromium.content.browser.test.util.CallbackHelper;
import org.chromium.content.browser.test.util.TestCallbackHelperContainer.OnPageStartedHelper;
import org.chromium.content.browser.test.util.TestCallbackHelperContainer.OnReceivedErrorHelper;
......@@ -24,6 +25,7 @@ import java.util.concurrent.Callable;
/**
* Test suite for AwContentsClientCallbackHelper.
*/
@ParameterizedTest.Set // These are unit tests. No need to repeat for multiprocess.
public class AwContentsClientCallbackHelperTest extends AwTestBase {
/**
* Callback helper for OnLoadedResource.
......@@ -65,6 +67,26 @@ public class AwContentsClientCallbackHelperTest extends AwTestBase {
}
}
private static class TestCancelCallbackPoller
implements AwContentsClientCallbackHelper.CancelCallbackPoller {
private boolean mCancelled;
private final CallbackHelper mCallbackHelper = new CallbackHelper();
public void setCancelled() {
mCancelled = true;
}
public CallbackHelper getCallbackHelper() {
return mCallbackHelper;
}
@Override
public boolean cancelAllCallbacks() {
mCallbackHelper.notifyCalled();
return mCancelled;
}
}
static final int PICTURE_TIMEOUT = 5000;
static final String TEST_URL = "www.example.com";
static final String REALM = "www.example.com";
......@@ -82,6 +104,7 @@ public class AwContentsClientCallbackHelperTest extends AwTestBase {
private TestAwContentsClient mContentsClient;
private AwContentsClientCallbackHelper mClientHelper;
private TestCancelCallbackPoller mCancelCallbackPoller;
private Looper mLooper;
@Override
......@@ -90,6 +113,8 @@ public class AwContentsClientCallbackHelperTest extends AwTestBase {
mLooper = Looper.getMainLooper();
mContentsClient = new TestAwContentsClient();
mClientHelper = new AwContentsClientCallbackHelper(mLooper, mContentsClient);
mCancelCallbackPoller = new TestCancelCallbackPoller();
mClientHelper.setCancelCallbackPoller(mCancelCallbackPoller);
}
@Feature({"AndroidWebView"})
......@@ -226,4 +251,31 @@ public class AwContentsClientCallbackHelperTest extends AwTestBase {
assertEquals(OLD_SCALE, scaleChangedHelper.getOldScale());
assertEquals(NEW_SCALE, scaleChangedHelper.getNewScale());
}
@Feature({"AndroidWebView"})
@SmallTest
public void testCancelCallbackPoller() throws Exception {
mCancelCallbackPoller.setCancelled();
CallbackHelper cancelCallbackPollerHelper = mCancelCallbackPoller.getCallbackHelper();
OnPageStartedHelper pageStartedHelper = mContentsClient.getOnPageStartedHelper();
int pollCount = pageStartedHelper.getCallCount();
int onPageStartedCount = pageStartedHelper.getCallCount();
// Post two callbacks.
mClientHelper.postOnPageStarted(TEST_URL);
mClientHelper.postOnPageStarted(TEST_URL);
// Wait for at least one poll.
cancelCallbackPollerHelper.waitForCallback(pollCount);
// Flush main queue.
getInstrumentation().runOnMainSync(new Runnable() {
@Override
public void run() {
}
});
// Neither callback should actually happen.
assertEquals(onPageStartedCount, pageStartedHelper.getCallCount());
}
}
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