The NSTimer in WebGeolocationProviderIOS.mm fires on the main thread without the WebThreadLock. This causes a fairly rare crash (usually in random WebCore rending code) if the JavaScript that runs after the timer fires causes layout on the main thread. The fix is simple; fire the NSTimer on the WebThread where it's safe to do the work.
<rdar://problem/19360382>
Created attachment 271570 [details] Patch v1
Comment on attachment 271570 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=271570&action=review > Source/WebKit/ios/Misc/WebGeolocationProviderIOS.mm:201 > + _sendLastPositionAsynchronouslyTimer = [NSTimer timerWithTimeInterval:0 target:self selector:@selector(_handlePendingInitialPosition:) userInfo:nil repeats:NO]; Nit: you could avoid some refcount churn by using NSTimer's designated initializer instead: _sendLastPositionAsynchronouslyTimer = adoptNS([[NSTimer alloc] initWithFireDate:[NSDate now] ...]);
Comment on attachment 271570 [details] Patch v1 Clearing flags on attachment: 271570 Committed r196714: <http://trac.webkit.org/changeset/196714>
All reviewed patches have been landed. Closing bug.
(In reply to comment #3) > Comment on attachment 271570 [details] > Patch v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271570&action=review > > > Source/WebKit/ios/Misc/WebGeolocationProviderIOS.mm:201 > > + _sendLastPositionAsynchronouslyTimer = [NSTimer timerWithTimeInterval:0 target:self selector:@selector(_handlePendingInitialPosition:) userInfo:nil repeats:NO]; > > Nit: you could avoid some refcount churn by using NSTimer's designated > initializer instead: > > _sendLastPositionAsynchronouslyTimer = adoptNS([[NSTimer alloc] > initWithFireDate:[NSDate now] ...]); Good point! Won't there be less editing when we enable ARC, though? :)
(In reply to comment #6) > (In reply to comment #3) > > Comment on attachment 271570 [details] > > Patch v1 > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=271570&action=review > > > > > Source/WebKit/ios/Misc/WebGeolocationProviderIOS.mm:201 > > > + _sendLastPositionAsynchronouslyTimer = [NSTimer timerWithTimeInterval:0 target:self selector:@selector(_handlePendingInitialPosition:) userInfo:nil repeats:NO]; > > > > Nit: you could avoid some refcount churn by using NSTimer's designated > > initializer instead: > > > > _sendLastPositionAsynchronouslyTimer = adoptNS([[NSTimer alloc] > > initWithFireDate:[NSDate now] ...]); > > Good point! Won't there be less editing when we enable ARC, though? :) RetainPtr is already compatible with ARC code (it just ifdefs out ), so either approach would work in ARC without modification (I'm sure we'd eventually remove the RetainPtrs, though).