Handling of inclusion from arc and non-arc TUs is unrelated to AccessibilityBridge. Can you separate this out into its own patch and add a unit-test (in the fml_unittests harness) that ensures objects are correctly released. Crucially, I am not able to fully reason about the correctness of what happens when say an ARC TU vends an API to a non-ARC TU using this header and vice-versa.

cc @stuartmorgan who has experience with ARC migrations.

Unfortunately it is not unrelated, it is necessary for this test to function. Without it code that includes accessibility_bridge.h won't compile under ARC and OCMock breaks without ARC.

I could refactor the bridge to use the pimpl pattern but I think the right move is just to fix scoped_nsobject so we fix it everywhere once.

I can add a test that shows this works.

Looks like there isn't any tests for scoped_nsobject. Here is sample code that shows it works: https://gist.github.com/gaaclarke/27aba0a4952f36591d05f164993ba95f

Unfortunately it is not unrelated, it is necessary for this test to function.

Can you clarify? I am not sure why the test cannot use manual reference counting in the test and compile the OCMock TUs with ARC. I'd like to understand why that causes an issue because I am afraid of introducing exactly the same kinds of issues here in the engine.

Sure. In my experience OCMock has problems in manually reference counted code. I tried to find reference to the problems but couldn't pull anything up. What it comes down to is that ARC has been adopted by the majority of developers and OCMock is doing funky things to work. It just isn't as tested for manual reference counting. I wouldn't consider it a concern for ARC adoption. It is only an issue because of the odd nature of mocking libraries.

This change doesn't introduce ARC to the engine. The engine will execute exactly the same. It just allows us to continue writing our unit tests in ARC. You'll have to take my word that OCMock works better with ARC. This PR just makes our code ARC compatible, which is a positive move.

Since there is still concern I added a unit-test.

Have you looked at Chromium's current scoped_ns* implementations? Getting it right for cross-mode TUs was a nightmare, involving several iterations that turned out to have subtle undefined behavior bugs (once due to a non-obvious ODR violation, and once due to what eventually turned out to be a compiler bug IIRC).

https://chromium.googlesource.com/chromium/src/base/+/master/mac/scoped_nsobject.h#49

Thanks Stuart, they use an extra template parameter which allows you to supply retain and release implementations (presumably they pass in noops in ARC code).

This PR is slightly different. I first tried to to make scoped_nsobject work for ARC and non-ARC instances and ran into weirdness. I suspect it was due to ARC's usage of the autorelease pool so that deallocation time isn't deterministic.

This doesn't have that problem because it just wraps objc message calls in c functions. I didn't try to make scoped_nsobject usable with ARC instances. In fact you get a linker error if you try to do that. I'm only interested in letting ARC instances use objects that have scoped_nsobjects. If I'm writing ARC C++ code I won't need scoped_nsobject, I can just rely on ARC. The fact that this works is exercised in the new unit test I added.

As far as concerns about weirdness, this just wraps objc messages in c functions, otherwise behaves the same. The PR passes integration tests and unit tests. Since the only ARC code is unit tests, the worse case scenario is that our tests will flake or crash.

I'm only interested in letting ARC instances use objects that have scoped_nsobjects.

That's what we were doing too, and we hit UB trying.

Do you have any documentation on what didn't work previously so we can see exactly what was tried and what went wrong? Right now we have reason and empirical evidence that this works versus anecdotal evidence that something like it didn't work in a different context.

I appreciate the perspective. The stakes are low here since this only affects testing targets. Let's not hold back progress for a hypothetical, let's try to pin down the actual problem if one exists.

Standard Flutter license verbiage.

Done.

NS_UNAVAILABLE

Also need to mark initWithAccessibilityContainer and +new unavailable.

Done.

Can you explain why these exist?

Sure. It's a bit of an oddity, a collision between how C++ macros templates work and ARC. Get ready for a long explanation.

The goal of this PR is to test the C++ class flutter::AccessibilityBridge, that class has instance variables of type fml::scoped_nsobject. The way C++ works is that it must declare its instance variables in the header so that the size of the object is known. fml::scoped_nsobject is a template class which will compile for each compilation unit the instances that it finds are actually used, when it is used. They aren't precompiled and may actually result in no code getting generated if they are never used.

So, fml::scoped_nsobject is just a RAII wrapper around objective-c manual reference counting. The problem is that when we compile accessibility_bridge_test.mm (which uses ARC like all our tests), it includes accessibility_bridge.h, which includes scoped_nsobject.h, which uses manual reference counting methods. This creates an error when compiling accessibility_bridge_test.mm because it is compiled under ARC and can't deal with manual reference counting methods.

So, this PR wraps up the manual reference counting methods in C functions whose declaration is known to clients and users of the fml::scoped_nsobject, but whose actual definition is only known to the fml library, effectively hiding the forbidden methods and guaranteeing that the fml::scoped_nsobject will behave the same in an ARC or non-ARC compilation unit.

Long story short, If you remove these functions the test won't compile.

If you try to use an ARC managed Objective-C class with fml::scoped_nsobject you get a linker error because it won't be able to find the retain and release methods on that class. This is how my second approach is different than my first, I tried to make fml::scoped_nsobject work for both type of objects (ARC and non-ARC) which didn't work out.

Any way we can start a policy of prefixing all classes with the FLT prefix (since this one has never been in a header before it's eligible even though it's not public)? It's a poor namespacing substitute but it's all Obj-C's got. We have random plugins using that prefix too, which is also not the right thing to do across frameworks... The other classes at least start with Flutter which is still not good, but is at least some kind of prefix.
http://google.github.io/styleguide/objcguide.html#prefixes
https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/Conventions/Conventions.html#//apple_ref/doc/uid/TP40011210-CH10-SW4