Fix placeholder

[strangeliu]

Placeholder is not working anymore since placeholderEnabled is read from _flags but never set to it.

if (_flags.placeholderEnabled && !_placeholderImage && [self _locked_displaysAsynchronously]) {

- (BOOL)_locked_shouldHavePlaceholderLayer
{
  DISABLED_ASAssertLocked(__instanceLock__);
  return (_flags.placeholderEnabled && [self _implementsDisplay]);
}

[maicki]

@bolsinga Hey Greg can you take a look please? Thanks!

[bolsinga]

In #1484 (where the line quoted above changed) I see -setPlaceholderEnabled: which is setting the flag. In #1498 I'd removed those (due to the atomic / locking conundrum we'd discussed in Texture). This is the only one that had _flags in the removal, so I think it is the only that was broken.

@strangeliu Do you have a test that demonstrates this problem? If so it would be terrific to get a unit test that covers it so this doesn't break again.

@Adlai-Holler @maicki How should this one property be reconciled with the discussion in #1479?

[maicki]

Looking into ASControlNode it seems there are issues with the _flags.placeholderEnabled as @strangeliu correctly pointed out ...

• The placeholderEnabled flag should should not exist if we use an atomic property for it. If we would use the atomic property, this mean that every time we are accessing or setting it internally we have to use the property directly and can not use the instance variable.
• In the placeholderEnabled property case we have it here it's a bit more complicated though, as we are accessing it in code that is internally locked via the instance lock (calling _locked_shouldHavePlaceholderLayer within _locked_layoutPlaceholderIfNecessary). In this case we should in fact use an internal flag for placeholderEnabled and exposing setters and getters locked with the instance lock.

Therefore I think @strangeliu PR makes sense and we should go with it.

[strangeliu]

@bolsinga I added a unit test for this.

[bolsinga]

Thanks for the test! And thanks for the advice @maicki

[maicki]

Glad it's resolved!

We may should add some documentation around the property implementations based on what I wrote above and adjust the places where we are not doing it. cc @bolsinga @nguyenhuy @mikezucc