CrNet: add pauseable NSURLProtocol and switch to using it

ios/net/crn_http_protocol_handler_proxy_with_client_thread.mm

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ios/net/crn_http_protocol_handler_proxy_with_client_thread.h"
 
 #include "base/logging.h"
 #import "base/mac/scoped_nsobject.h"
 #include "base/time/time.h"
 #import "ios/net/protocol_handler_util.h"
@@ skipping 18 matching lines @@
   __weak NSThread* _clientThread;
   // The run loop modes to use when posting tasks to |clientThread_|.
   base::scoped_nsobject<NSArray> _runLoopModes;
   // The request URL.
   base::scoped_nsobject<NSString> _url;
   // The creation time of the request.
   base::Time _creationTime;
   // |requestComplete_| is used in debug to check that the client is not called
   // after completion.
   BOOL _requestComplete;
+  BOOL _paused;
+
+  base::scoped_nsobject<NSMutableArray> _queuedInvocations;
 }
 
 // Performs the selector on |clientThread_| using |runLoopModes_|.
+- (void)runInvocationQueueOnClientThread;
+- (void)enqueueInvocationOnClientThread:(NSInvocation*)invocation;
 - (void)performSelectorOnClientThread:(SEL)aSelector withObject:(id)arg;
 // These functions are just wrappers around the corresponding
 // NSURLProtocolClient methods, used for task posting.
 - (void)didFailWithErrorOnClientThread:(NSError*)error;
 - (void)didLoadDataOnClientThread:(NSData*)data;
 - (void)didReceiveResponseOnClientThread:(NSURLResponse*)response;
 - (void)wasRedirectedToRequestOnClientThread:(NSArray*)params;
 - (void)didFinishLoadingOnClientThread;
 @end
 
 @implementation CRNHTTPProtocolHandlerProxyWithClientThread
 
 - (instancetype)initWithProtocol:(NSURLProtocol*)protocol
                     clientThread:(NSThread*)clientThread
                      runLoopMode:(NSString*)mode {
   DCHECK(protocol);
   DCHECK(clientThread);
   if ((self = [super init])) {
     _protocol = protocol;
     _url.reset([[[[protocol request] URL] absoluteString] copy]);
     _creationTime = base::Time::Now();
     _clientThread = clientThread;
     // Use the common run loop mode in addition to the client thread mode, in
     // hope that our tasks are executed even if the client thread changes mode
     // later on.
     if ([mode isEqualToString:NSRunLoopCommonModes])
       _runLoopModes.reset([@[ NSRunLoopCommonModes ] retain]);
     else
       _runLoopModes.reset([@[ mode, NSRunLoopCommonModes ] retain]);
+    _paused = NO;

[droger, 2015/05/27 17:09:00]

Nit: in objective-C, instance variables are auto-zeroed.
The usual style is to not re-initialize them to zero.

[Elly Fong-Jones, 2015/06/10 17:06:35]

Done.

+    _queuedInvocations.reset([[NSMutableArray alloc] init]);
   }
   return self;
 }
 
 - (void)invalidate {
   DCHECK([NSThread currentThread] == _clientThread);
   _protocol = nil;
   _requestComplete = YES;
+  // TODO(ellyjones): This DCHECK fails sometimes. What this implies is that a
+  // callback is coming from the chrome network stack, being enqueued, and then
+  // this proxy is being invalidated before that callback has ever been
+  // delivered. Disturbingly, in practice these seem to be didLoadData and
+  // similar...
+  // DCHECK(_queuedInvocations.get().count == 0);

[droger, 2015/05/27 17:09:01]

This might be caused by the problem with |_paused| below.

[Elly Fong-Jones, 2015/06/10 17:06:35]

Done.

+}
+
+- (void)enqueueInvocationOnClientThread:(NSInvocation*)inv {
+  DCHECK([NSThread currentThread] == _clientThread);
+  DCHECK(!_requestComplete || !_protocol);
+  [_queuedInvocations addObject:inv];
+  // This function receives these two references from
+  // |performSelectorOnClientThread:| below.
+  [inv release];
+  [self release];
+}
+
+- (void)runInvocationQueueOnClientThread {
+  DCHECK([NSThread currentThread] == _clientThread);
+  DCHECK(!_requestComplete || !_protocol);
+  while (!_paused && _queuedInvocations.get().count > 0) {
+    NSInvocation* inv = [_queuedInvocations objectAtIndex:0];
+    // Since |_queuedInvocations| owns the only reference to each queued
+    // invocation, this function has to retain another reference before removing
+    // the queued invocation from the array.
+    [inv retain];
+    [_queuedInvocations removeObjectAtIndex:0];
+    [inv invoke];
+    [inv release];
+  }
 }
 
 - (void)performSelectorOnClientThread:(SEL)aSelector withObject:(id)arg {
-  [self performSelector:aSelector
-               onThread:_clientThread
-             withObject:arg
-          waitUntilDone:NO
-                  modes:_runLoopModes];
+  if (!_paused) {

[droger, 2015/05/27 17:09:00]

I don't think it is safe to access _paused here.

I don't know what is the best way to do it, but I think we should be able to do
it without locks, by catching the tasks on the other thread, instead of catching
them before posting them.

[Elly Fong-Jones, 2015/06/10 17:06:35]

Done.

+    [self performSelector:aSelector
+                 onThread:_clientThread
+               withObject:arg
+            waitUntilDone:NO
+                    modes:_runLoopModes];
+  } else {
+    // The memory management for this case is tricky. The |inv| NSInvocation
+    // object created here is autoreleased, so it would be destroyed by the
+    // autorelease pool if this function did not retain another reference to it.
+    // That reference is given to |enqueueInvocationOnClientThread:| via
+    // |performSelector:|, and |enqueueInvocationOnClientThread:| is responsible
+    // for releasing it.
+    //
+    // This function also needs an extra reference to |self|. Otherwise, this
+    // object can be destroyed by the time the |enqueueInvocationOnClientThread|
+    // function runs, with disastrous results. |enqueueInvocationOnClientThread|
+    // is responsible for releasing this extra reference as well; it receives
+    // ownership of it through |performSelector|.
+    NSMethodSignature* sig = [self methodSignatureForSelector:aSelector];
+    DCHECK(sig != nil);
+    NSInvocation* inv = [NSInvocation invocationWithMethodSignature:sig];
+    [inv setTarget:self];
+    if (sig.numberOfArguments > 2) {
+      [inv setArgument:&arg atIndex:2];
+    }
+    [inv setSelector:aSelector];
+    [inv retainArguments];
+    [inv retain];
+
+    [self retain];
+    [self performSelector:@selector(enqueueInvocationOnClientThread:)
+                 onThread:_clientThread
+               withObject:inv
+            waitUntilDone:NO
+                    modes:_runLoopModes];
+  }
 }
 
 #pragma mark Proxy methods called from any thread.
 
 - (void)didFailWithNSErrorCode:(NSInteger)nsErrorCode
                   netErrorCode:(int)netErrorCode {
   DCHECK(_clientThread);
   if (!_protocol)
     return;
   NSError* error =
@@ skipping 94 matching lines @@
                  redirectResponse:params[1]];
 }
 
 - (void)didFinishLoadingOnClientThread {
   DCHECK([NSThread currentThread] == _clientThread);
   DCHECK(!_requestComplete || !_protocol);
   _requestComplete = YES;
   [[_protocol client] URLProtocolDidFinishLoading:_protocol];
 }
 
+- (void)pause {
+  _paused = YES;

[droger, 2015/05/27 17:09:01]

We probably need thread checks here
DCHECK([NSThread currentThread] == _clientThread);

|_paused| should only be accessed from one thread.

[Elly Fong-Jones, 2015/06/10 17:06:35]

Done.

+}
+
+- (void)resume {
+  _paused = NO;

[droger, 2015/05/27 17:09:01]

Thread checks.

[Elly Fong-Jones, 2015/06/10 17:06:35]

Done.

+  [self runInvocationQueueOnClientThread];
+}
+
 @end