We're seeing some crashes (e.g. bug 1579431 and bug 1581669) where [CATransaction commit] on the compositor thread causes a bunch of code to run that's only supposed to run on the main thread, mainly CALayer display methods which call NSView drawing methods which only expect to be called on the main thread.

CATransaction has a private method called activateBackground. +[NSAnimationContext beginGrouping] calls this method when it's called on a non-main thread. I don't know what that method does, but there's a theoretical chance that it'll help with the problems we're seeing.
Commiting CATransactions on a non-main thread is not a very common thing to do for regular macOS apps. The one case I can think of is off-main-thread scrolling: -[_NSScrollingConcurrentVBLMonitor _updateScrollAnimation] updates the CALayer tree from a background thread, and it also calls +[CATransaction activateBackground:].

So I think we should give NSAnimationContext a try.

When called on a non-main thread, [NSAnimationContext beginGrouping] calls [CATransaction activateBackground:].
There's a chance that doing so might help with some of the crashes we're seeing during [CATransaction commit].

This bug might benefit from an analysis using a HookCase hook library (https://github.com/steven-michaud/HookCase).

https://hg.mozilla.org/mozilla-central/rev/b386ee32d8f6

Comment on attachment 9097840 [details]
Bug 1585523 - Use NSAnimationContext instead of CATransaction when triggering off-main-thread composites. r=mattwoodrow

Beta/Release Uplift Approval Request

• User impact if declined: Random crashes
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): I'm pretty sure this patch has zero risk. There's a good chance that it won't help, but there is some chance that it will.
  The crashes from bug 1579431 only show up on Beta except for one report on Nightly. I think we need wider testing before we know whether this patch will actually help with those, and we'll only get wide enough testing on Beta.
• String changes made/needed:

Jeff and I have done more investigation here and we think there is a fairly high chance that this patch will fix bug 1579431.

In QuartzCore on 10.12, +[CATransaction activateBackground:] sets bit 0x4 in a flags field in the CA::Transaction struct. Let's call that bit kTransactionFlagBackground. CA::Layer::latch_thread_flags converts those flags to a different flags representation (let's call it threadFlags), and only sets bits 0x100 and 0x200 in threadFlags if kTransactionFlagBackground is set - otherwise it clears those two bits (and others). CA::Layer::thread_flags_ is a wrapper function around latch_thread_flags that just passes threadFlags on to the caller.
CA::Layer::display_if_needed calls CA::Layer::threadFlags_, and only calls some methods, including -[CALayer _canDisplayConcurrently] and -[CALayer display] if bits 0x100 and 0x200 are not set in the thread flags.
It's the calls to display that crash in most of the crash reports in bug 1579431.

Comment on attachment 9097840 [details]
Bug 1585523 - Use NSAnimationContext instead of CATransaction when triggering off-main-thread composites. r=mattwoodrow

Fix for a moderate volume crash in beta, let's go ahead with it in beta 14.

https://hg.mozilla.org/releases/mozilla-beta/rev/e8fb03fbdda7