Skip to content

Commit

Permalink
setup test to use custom queue for RCTNetworking operations instead o…
Browse files Browse the repository at this point in the history
…f module queue

Summary:
Changelog: [Internal]

remerge of facebook#41183

>in my quest to get rid of all synthesized methodQueues, we have RCTNetworking which uses it internally as well as exposes its underlying execution queue. in this diff, i add a config that replaces that queue with one that is managed by the module itself instead of the one generated by the infra.
this is the last one!

Differential Revision: D50764523
  • Loading branch information
philIip authored and facebook-github-bot committed Oct 30, 2023
1 parent cfb4af3 commit 92762d6
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 16 deletions.
4 changes: 2 additions & 2 deletions packages/react-native/Libraries/Blob/RCTBlobManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,11 @@ - (void)remove:(NSString *)blobId

// TODO(T63516227): Why can methodQueue be nil here?
// We don't want to do anything when methodQueue is nil.
if (!networking.methodQueue) {
if (![networking requestQueue]) {
return;
}

dispatch_async(networking.methodQueue, ^{
dispatch_async([networking requestQueue], ^{
[networking addRequestHandler:self];
[networking addResponseHandler:self];
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ - (NSURLSessionDataTask *)sendRequest:(NSURLRequest *)request withDelegate:(id<R

NSOperationQueue *callbackQueue = [NSOperationQueue new];
callbackQueue.maxConcurrentOperationCount = 1;
callbackQueue.underlyingQueue = [[_moduleRegistry moduleForName:"Networking"] methodQueue];
RCTNetworking *networking = [_moduleRegistry moduleForName:"Networking"];
callbackQueue.underlyingQueue = [networking requestQueue];
NSURLSessionConfiguration *configuration;
if (urlSessionConfigurationProvider) {
configuration = urlSessionConfigurationProvider();
Expand Down
4 changes: 4 additions & 0 deletions packages/react-native/Libraries/Network/RCTNetworking.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#import <React/RCTNetworkTask.h>
#import <React/RCTURLRequestHandler.h>

RCT_EXTERN void RCTEnableNetworkingRequestQueue(BOOL enabled);

@protocol RCTNetworkingRequestHandler <NSObject>

// @lint-ignore FBOBJCUNTYPEDCOLLECTION1
Expand Down Expand Up @@ -56,6 +58,8 @@

- (void)removeResponseHandler:(id<RCTNetworkingResponseHandler>)handler;

- (dispatch_queue_t)requestQueue;

@end

@interface RCTBridge (RCTNetworking)
Expand Down
46 changes: 33 additions & 13 deletions packages/react-native/Libraries/Network/RCTNetworking.mm
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@

#import "RCTNetworkPlugins.h"

static BOOL gEnableNetworkingRequestQueue = NO;

RCT_EXTERN void RCTEnableNetworkingRequestQueue(BOOL enabled)
{
gEnableNetworkingRequestQueue = enabled;
}

typedef RCTURLRequestCancellationBlock (^RCTHTTPQueryResult)(NSError *error, NSDictionary<NSString *, id> *result);

NSString *const RCTNetworkingPHUploadHackScheme = @"ph-upload";
Expand Down Expand Up @@ -69,7 +76,7 @@ @implementation RCTHTTPFormDataHelper {

- (RCTURLRequestCancellationBlock)process:(NSArray<NSDictionary *> *)formData callback:(RCTHTTPQueryResult)callback
{
RCTAssertThread(_networker.methodQueue, @"process: must be called on method queue");
RCTAssertThread([_networker requestQueue], @"process: must be called on request queue");

if (formData.count == 0) {
return callback(nil, nil);
Expand Down Expand Up @@ -98,7 +105,7 @@ - (RCTURLRequestCancellationBlock)process:(NSArray<NSDictionary *> *)formData ca

- (RCTURLRequestCancellationBlock)handleResult:(NSDictionary<NSString *, id> *)result error:(NSError *)error
{
RCTAssertThread(_networker.methodQueue, @"handleResult: must be called on method queue");
RCTAssertThread([_networker requestQueue], @"handleResult: must be called on request queue");

if (error) {
return _callback(error, nil);
Expand Down Expand Up @@ -151,6 +158,7 @@ @implementation RCTNetworking {
NSArray<id<RCTURLRequestHandler>> * (^_handlersProvider)(RCTModuleRegistry *);
NSMutableArray<id<RCTNetworkingRequestHandler>> *_requestHandlers;
NSMutableArray<id<RCTNetworkingResponseHandler>> *_responseHandlers;
dispatch_queue_t _requestQueue;
}

@synthesize methodQueue = _methodQueue;
Expand All @@ -164,7 +172,12 @@ + (BOOL)requiresMainQueueSetup

- (instancetype)init
{
return [super initWithDisabledObservation];
if (self = [super initWithDisabledObservation]) {
if (gEnableNetworkingRequestQueue) {
_requestQueue = dispatch_queue_create("com.facebook.react.network.request", DISPATCH_QUEUE_SERIAL);
}
}
return self;
}

- (instancetype)initWithHandlersProvider:
Expand Down Expand Up @@ -294,7 +307,7 @@ - (void)invalidate
- (RCTURLRequestCancellationBlock)buildRequest:(NSDictionary<NSString *, id> *)query
completionBlock:(void (^)(NSURLRequest *request))block
{
RCTAssertThread(_methodQueue, @"buildRequest: must be called on method queue");
RCTAssertThread([self requestQueue], @"buildRequest: must be called on request queue");

NSURL *URL = [RCTConvert NSURL:query[@"url"]]; // this is marked as nullable in JS, but should not be null
NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:URL];
Expand Down Expand Up @@ -347,7 +360,7 @@ - (RCTURLRequestCancellationBlock)buildRequest:(NSDictionary<NSString *, id> *)q
forHTTPHeaderField:@"Content-Length"];
}

dispatch_async(self->_methodQueue, ^{
dispatch_async([self requestQueue], ^{
block(request);
});

Expand Down Expand Up @@ -385,7 +398,7 @@ - (BOOL)canHandleRequest:(NSURLRequest *)request
callback:(RCTURLRequestCancellationBlock (^)(NSError *error, NSDictionary<NSString *, id> *result))
callback
{
RCTAssertThread(_methodQueue, @"processDataForHTTPQuery: must be called on method queue");
RCTAssertThread([self requestQueue], @"processDataForHTTPQuery: must be called on request queue");

if (!query) {
return callback(nil, nil);
Expand Down Expand Up @@ -414,7 +427,7 @@ - (BOOL)canHandleRequest:(NSURLRequest *)request
RCTNetworkTask *task =
[self networkTaskWithRequest:request
completionBlock:^(NSURLResponse *response, NSData *data, NSError *error) {
dispatch_async(self->_methodQueue, ^{
dispatch_async([self requestQueue], ^{
cancellationBlock = callback(
error, data ? @{@"body" : data, @"contentType" : RCTNullIfNil(response.MIMEType)} : nil);
});
Expand Down Expand Up @@ -514,7 +527,7 @@ - (void)sendData:(NSData *)data
response:(NSURLResponse *)response
forTask:(RCTNetworkTask *)task
{
RCTAssertThread(_methodQueue, @"sendData: must be called on method queue");
RCTAssertThread([self requestQueue], @"sendData: must be called on request queue");

id responseData = nil;
for (id<RCTNetworkingResponseHandler> handler in _responseHandlers) {
Expand Down Expand Up @@ -552,7 +565,7 @@ - (void)sendRequest:(NSURLRequest *)request
incrementalUpdates:(BOOL)incrementalUpdates
responseSender:(RCTResponseSenderBlock)responseSender
{
RCTAssertThread(_methodQueue, @"sendRequest: must be called on method queue");
RCTAssertThread([self requestQueue], @"sendRequest: must be called on request queue");
__weak __typeof(self) weakSelf = self;
__block RCTNetworkTask *task;
RCTURLRequestProgressBlock uploadProgressBlock = ^(int64_t progress, int64_t total) {
Expand Down Expand Up @@ -689,7 +702,9 @@ - (RCTNetworkTask *)networkTaskWithRequest:(NSURLRequest *)request
return nil;
}

RCTNetworkTask *task = [[RCTNetworkTask alloc] initWithRequest:request handler:handler callbackQueue:_methodQueue];
RCTNetworkTask *task = [[RCTNetworkTask alloc] initWithRequest:request
handler:handler
callbackQueue:[self requestQueue]];
task.completionBlock = completionBlock;
return task;
}
Expand All @@ -709,7 +724,7 @@ - (RCTNetworkTask *)networkTaskWithRequest:(NSURLRequest *)request
double timeout = query.timeout();
bool withCredentials = query.withCredentials();

dispatch_async(_methodQueue, ^{
dispatch_async([self requestQueue], ^{
NSDictionary *queryDict = @{
@"method" : method,
@"url" : url,
Expand Down Expand Up @@ -738,15 +753,15 @@ - (RCTNetworkTask *)networkTaskWithRequest:(NSURLRequest *)request

RCT_EXPORT_METHOD(abortRequest : (double)requestID)
{
dispatch_async(_methodQueue, ^{
dispatch_async([self requestQueue], ^{
[self->_tasksByRequestID[[NSNumber numberWithDouble:requestID]] cancel];
[self->_tasksByRequestID removeObjectForKey:[NSNumber numberWithDouble:requestID]];
});
}

RCT_EXPORT_METHOD(clearCookies : (RCTResponseSenderBlock)responseSender)
{
dispatch_async(_methodQueue, ^{
dispatch_async([self requestQueue], ^{
NSHTTPCookieStorage *storage = [NSHTTPCookieStorage sharedHTTPCookieStorage];
if (!storage.cookies.count) {
responseSender(@[ @NO ]);
Expand All @@ -760,6 +775,11 @@ - (RCTNetworkTask *)networkTaskWithRequest:(NSURLRequest *)request
});
}

- (dispatch_queue_t)requestQueue
{
return gEnableNetworkingRequestQueue ? _requestQueue : _methodQueue;
}

- (std::shared_ptr<facebook::react::TurboModule>)getTurboModule:
(const facebook::react::ObjCTurboModule::InitParams &)params
{
Expand Down

0 comments on commit 92762d6

Please sign in to comment.