CrNet: add pauseable NSURLProtocol and switch to using it

ios/net/crn_http_protocol_handler.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.h"
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/mac/bind_objc_block.h"
 #include "base/mac/scoped_nsobject.h"
@@ skipping 848 matching lines @@
     _core = core;
   return self;
 }
 
 - (void)stream:(NSStream*)theStream handleEvent:(NSStreamEvent)streamEvent {
   _core->HandleStreamEvent(theStream, streamEvent);
 }
 @end
 
 #pragma mark -
+#pragma mark DeferredCancellation
+
+@interface DeferredCancellation : NSObject {}
+
+- (instancetype)initWithCore:(scoped_refptr<net::HttpProtocolHandlerCore>)core;
+- (void)cancel;
+
+@end
+
+@implementation DeferredCancellation {
+  scoped_refptr<net::HttpProtocolHandlerCore> _core;
+}
+
+- (instancetype)initWithCore:(scoped_refptr<net::HttpProtocolHandlerCore>)core {
+  if ((self = [super init])) {
+    _core.swap(core);

[droger, 2015/06/11 08:56:19]

Nit: We're not swapping anything here,
_core = core;
seems clearer.

[Elly Fong-Jones, 2015/06/11 13:49:33]

Done.

+  }
+  return self;
+}
+
+- (void)cancel {
+  g_protocol_handler_delegate->GetDefaultURLRequestContext()
+      ->GetNetworkTaskRunner()
+      ->PostTask(FROM_HERE,
+                 base::Bind(&net::HttpProtocolHandlerCore::Cancel, _core));
+}
+
+@end
+
+#pragma mark -
 #pragma mark HttpProtocolHandler
 
+@interface CRNHTTPProtocolHandler (Private) {}
+
+- (id<CRNHTTPProtocolHandlerProxy>)getProtocolHandlerProxy;
+- (void)cancelRequest;
+
+@end
+
 // The HttpProtocolHandler is called by the iOS system to handle the
 // NSURLRequest.
 @implementation CRNHTTPProtocolHandler {
   scoped_refptr<net::HttpProtocolHandlerCore> _core;
   base::scoped_nsprotocol<id<CRNHTTPProtocolHandlerProxy>> _protocolProxy;
+  NSThread* _clientThread;
+  NSString* _clientRunLoopMode;
   BOOL _supportedURL;
 }
 
 #pragma mark NSURLProtocol methods
 
 + (BOOL)canInitWithRequest:(NSURLRequest*)request {
   DVLOG(5) << "canInitWithRequest " << net::FormatUrlRequestForLogging(request);
   return g_protocol_handler_delegate->CanHandleRequest(request);
 }
 
@@ skipping 37 matching lines @@
     if (url)
       [dictionary setObject:url forKey:NSURLErrorKey];
 
     NSError* error = [NSError errorWithDomain:NSURLErrorDomain
                                          code:NSURLErrorUnsupportedURL
                                      userInfo:dictionary];
     [[self client] URLProtocol:self didFailWithError:error];
     return;
   }
 
-  _protocolProxy.reset([[CRNHTTPProtocolHandlerProxyWithClientThread alloc]
-      initWithProtocol:self
-          clientThread:[NSThread currentThread]
-           runLoopMode:[[NSRunLoop currentRunLoop] currentMode]]);
+  _clientThread = [NSThread currentThread];
+
   g_protocol_handler_delegate->GetDefaultURLRequestContext()
       ->GetNetworkTaskRunner()
       ->PostTask(FROM_HERE, base::Bind(&net::HttpProtocolHandlerCore::Start,
-                                       _core, _protocolProxy));
+                                       _core, [self getProtocolHandlerProxy]));
 }
 
-- (void)stopLoading {
+- (id<CRNHTTPProtocolHandlerProxy>)getProtocolHandlerProxy {

[droger, 2015/06/11 08:56:19]

DCHECK that the current thread is _clientThread here or inside the if.

[Elly Fong-Jones, 2015/06/11 13:49:33]

Done.

+  if (!_protocolProxy.get()) {
+    _protocolProxy.reset([[CRNHTTPProtocolHandlerProxyWithClientThread alloc]
+        initWithProtocol:self
+            clientThread:[NSThread currentThread]

[droger, 2015/06/11 08:56:19]

Why not using _clientThread here?

[Elly Fong-Jones, 2015/06/11 13:49:33]

Done.

+             runLoopMode:[[NSRunLoop currentRunLoop] currentMode]]);
+  }
+  return _protocolProxy.get();
+}
+
+// This method has unusual concurrency properties. It can be called on any
+// thread, but it must be called from |-dealloc|, which guarantees that no other
+// method of this object is running concurrently (since |-dealloc| is only
+// called when the last reference to the object drops).
+//
+// This method takes a reference to _core to ensure that _core lives long enough
+// to have the request cleanly cancelled.
+- (void)scheduleCancelRequest {

[droger, 2015/06/11 08:56:19]

Optional: 
Why is this method on CRNHTTPProtocolHandler and not on
PauseableHttpProtocolHandler?
I see that moving it would require exposing _clientThread and _core, but maybe
that's better?

[Elly Fong-Jones, 2015/06/11 13:49:33]

Done. I have added accessors for _clientThread and _core and moved this method
onto PauseableHTTPProtocolHandler.

+  DeferredCancellation* cancellation =
+      [[DeferredCancellation alloc] initWithCore:_core];
+  NSArray* modes = @[ [[NSRunLoop currentRunLoop] currentMode] ];
+  [cancellation performSelector:@selector(cancel)
+                       onThread:_clientThread
+                     withObject:nil
+                  waitUntilDone:NO
+                          modes:modes];
+}
+
+- (void)cancelRequest {
   g_protocol_handler_delegate->GetDefaultURLRequestContext()
       ->GetNetworkTaskRunner()
       ->PostTask(FROM_HERE,
                  base::Bind(&net::HttpProtocolHandlerCore::Cancel, _core));
   [_protocolProxy invalidate];
+}
+
+- (void)stopLoading {
+  [self cancelRequest];
   _protocolProxy.reset();
 }
 
 @end
+
+#pragma mark -
+#pragma mark PauseableHttpProtocolHandler
+
+// The HttpProtocolHandler is called by the iOS system to handle the
+// NSURLRequest. This HttpProtocolHandler conforms to the observed semantics of
+// NSURLProtocol when used with NSURLSession on iOS 8 - i.e., |-startLoading|
+// means "start or resume request" and |-stopLoading| means "pause request".
+// Since there is no way to actually pause a request in the network stack, this
+// is implemented using a subclass of CRNHTTPProtocolHandlerProxy that knows how
+// to defer callbacks.
+//
+// Note that this class conforms to somewhat complex threading rules:
+// 1) |initWithRequest:cachedResponse:client:| and |dealloc| can be called on
+//    any thread.
+// 2) |startLoading| and |stopLoading| are always called on the client thread.
+// 3) |stopLoading| is called before |dealloc| is called.
+//
+// The main wrinkle is that |dealloc|, which may be called on any thread, needs
+// to clean up a running network request. To do this, |dealloc| needs to run
+// |cancelRequest|, which needs to be run on the client thread. Since it is
+// guaranteed that |startLoading| is called before |dealloc| is called, the
+// |startLoading| method stores a pointer to the client thread, then |dealloc|
+// asks that client thread to perform the |cancelRequest| selector via
+// |scheduleCancelRequest|.
+//
+// Some of the above logic is implemented in the parent class
+// (CRNHTTPProtocolHandler) because it is convenient.
+@implementation CRNPauseableHTTPProtocolHandler {
+  BOOL _started;
+  dispatch_queue_t _queue;
+}
+
+#pragma mark NSURLProtocol methods
+
+- (instancetype)initWithRequest:(NSURLRequest*)request

[droger, 2015/06/11 08:56:19]

Is this constructor useful? Is it only here for the DCHECK?

[Elly Fong-Jones, 2015/06/11 13:49:33]

It is not. The parent class duplicates the DCHECK anyway, so I've removed it.

+                 cachedResponse:(NSCachedURLResponse*)cachedResponse
+                         client:(id<NSURLProtocolClient>)client {
+  DCHECK(!cachedResponse);
+  self = [super initWithRequest:request
+                 cachedResponse:cachedResponse
+                         client:client];
+  return self;
+}
+
+- (void)dealloc {
+  [self scheduleCancelRequest];
+  [super dealloc];
+}
+
+#pragma mark NSURLProtocol overrides.
+
+- (void)startLoading {
+  if (_started) {
+    [[self getProtocolHandlerProxy] resume];
+    return;
+  }
+
+  _started = YES;
+  [super startLoading];
+}
+
+- (void)stopLoading {
+  [[self getProtocolHandlerProxy] pause];
+}
+
+@end