Skip to content

Commit

Permalink
Remove GuardedAsyncTask wrapper for call.cancel() (facebook#48251)
Browse files Browse the repository at this point in the history
Summary:

The original comment mentioning square/okhttp#869 predates the [upgrade to okhttp3](facebook@6bbaff2), which resolved the issue via square/okhttp#1592.

Cancel calls should now be async and don't need to be guarded.  Removing this also fixes a discrepancy in NetworkingModule unit test verification.

Changelog: [Internal]

Reviewed By: javache

Differential Revision: D67157018
  • Loading branch information
Thomas Nardone authored and facebook-github-bot committed Dec 17, 2024
1 parent f017d3f commit 9d76233
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import com.facebook.common.logging.FLog;
import com.facebook.fbreact.specs.NativeNetworkingAndroidSpec;
import com.facebook.react.bridge.Arguments;
import com.facebook.react.bridge.GuardedAsyncTask;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactMethod;
import com.facebook.react.bridge.ReadableArray;
Expand Down Expand Up @@ -673,14 +672,7 @@ public void abortRequest(double requestIdAsDouble) {
}

private void cancelRequest(final int requestId) {
// We have to use AsyncTask since this might trigger a NetworkOnMainThreadException, this is an
// open issue on OkHttp: https://github.com/square/okhttp/issues/869
new GuardedAsyncTask<Void, Void>(getReactApplicationContext()) {
@Override
protected void doInBackgroundGuarded(Void... params) {
OkHttpCallUtil.cancelTag(mClient, Integer.valueOf(requestId));
}
}.execute();
OkHttpCallUtil.cancelTag(mClient, Integer.valueOf(requestId));
}

@ReactMethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import okio.Buffer
import org.assertj.core.api.Assertions.assertThat
import org.junit.After
import org.junit.Before
import org.junit.Ignore
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.MockedStatic
Expand Down Expand Up @@ -73,7 +72,7 @@ class NetworkingModuleTest {
context = mock()
whenever(context.hasActiveReactInstance()).thenReturn(true)

networkingModule = NetworkingModule(context, "", httpClient)
networkingModule = NetworkingModule(context, "", httpClient, null)

arguments = mockStatic(Arguments::class.java)
arguments.`when`<WritableArray> { Arguments.createArray() }.thenAnswer { JavaOnlyArray() }
Expand Down Expand Up @@ -477,12 +476,11 @@ class NetworkingModuleTest {
}

@Test
@Ignore("TODO: Fix me (T171890419)")
fun testCancelAllCallsInvalidate() {
val requests = 3
val calls = arrayOfNulls<Call>(requests)
val calls = mutableListOf<Call>()
for (idx in 0 until requests) {
calls[idx] = mock<Call>()
calls.add(mock<Call>())
}

whenever(httpClient.newCall(any())).thenAnswer { invocation ->
Expand All @@ -507,11 +505,9 @@ class NetworkingModuleTest {
verify(httpClient, times(3)).newCall(any())

networkingModule.invalidate()
val clientArguments = argumentCaptor<OkHttpClient>()
val requestIdArguments = argumentCaptor<Int>()
okHttpCallUtil.verify(
{ OkHttpCallUtil.cancelTag(clientArguments.capture(), requestIdArguments.capture()) },
times(3))
{ OkHttpCallUtil.cancelTag(any(), requestIdArguments.capture()) }, times(requests))

assertThat(requestIdArguments.allValues.size).isEqualTo(requests)
for (idx in 0 until requests) {
Expand All @@ -520,7 +516,6 @@ class NetworkingModuleTest {
}

@Test
@Ignore("TODO: Fix me (T171890419)")
fun testCancelSomeCallsInvalidate() {
val requests = 3
val calls = arrayOfNulls<Call>(requests)
Expand Down

0 comments on commit 9d76233

Please sign in to comment.