fix(runtime): Expose base class missing properties in inheritors

Fix issue with missing `imageInsets`, `landscapeImagePhone`, `title` properties of UITabBarItem.
These properties are declared in `UIBarItem` but it actually doesn't implement them. Some of its
successors like `UITabBarItem` however do, and we need to expose them there.

This regression has been introduced with #1086.

Author: mbektchiev

src/NativeScript/GlobalObject.mm

@@ -353,10 +353,12 @@ static void runLoopBeforeWaitingPerformWork(CFRunLoopObserverRef observer, CFRun
 
         auto protocol = createProtocolWrapper(globalObject, static_cast<const ProtocolMeta*>(symbolMeta), aProtocol);
         strongSymbolWrapper = protocol;
+        // Protocols that are not implemented or referred with @protocol at compile time do not have corresponding
+        // protocol objects at runtime. `objc_getProtocol` returns `nullptr for them!
+        // See Protocol Objects section at https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ObjectiveC/Chapters/ocProtocols.html#//apple_ref/doc/uid/TP30001163-CH15
         if (aProtocol) {
             globalObject->_objCProtocolWrappers.insert({ aProtocol, protocol });
         }
-
         break;
     }
     case Union: {

src/NativeScript/ObjC/ObjCPrototype.h

@@ -13,8 +13,11 @@
 #include <wtf/RetainPtr.h>
 
 namespace Metadata {
+
 struct BaseClassMeta;
-}
+struct PropertyMeta;
+
+} // namespace Metadata
 
 namespace NativeScript {
 class ObjCPrototype : public JSC::JSDestructibleObject {
@@ -39,6 +42,8 @@ class ObjCPrototype : public JSC::JSDestructibleObject {
 
     void materializeProperties(JSC::VM& vm, GlobalObject* globalObject);
 
+    void defineNativeProperty(JSC::VM& vm, GlobalObject* globalObject, const Metadata::PropertyMeta* propertyMeta);
+
     const Metadata::BaseClassMeta* metadata() const {
         return this->_metadata;
     }

src/NativeScript/ObjC/ObjCPrototype.mm

@@ -171,32 +171,86 @@ static EncodedJSValue JSC_HOST_CALL getIterator(ExecState* execState) {
     Base::getOwnPropertyNames(object, execState, propertyNames, enumerationMode);
 }
 
+void ObjCPrototype::defineNativeProperty(VM& vm, GlobalObject* globalObject, const PropertyMeta* propertyMeta) {
+    SymbolLoader::instance().ensureModule(propertyMeta->topLevelModule());
+
+    const MethodMeta* getter = (propertyMeta->hasGetter() && propertyMeta->getter()->isAvailable()) ? propertyMeta->getter() : nullptr;
+    const MethodMeta* setter = (propertyMeta->hasSetter() && propertyMeta->setter()->isAvailable()) ? propertyMeta->setter() : nullptr;
+
+    PropertyDescriptor descriptor;
+    descriptor.setConfigurable(true);
+    Strong<ObjCMethodWrapper> strongGetter;
+    Strong<ObjCMethodWrapper> strongSetter;
+    if (getter) {
+        MembersCollection getters = { getter };
+        strongGetter = ObjCMethodWrapper::create(vm, globalObject, globalObject->objCMethodWrapperStructure(), getters);
+        descriptor.setGetter(strongGetter.get());
+    }
+
+    if (setter) {
+        MembersCollection setters = { setter };
+        strongSetter = ObjCMethodWrapper::create(vm, globalObject, globalObject->objCMethodWrapperStructure(), setters);
+        descriptor.setSetter(strongSetter.get());
+    }
+
+    Base::defineOwnProperty(this, globalObject->globalExec(), Identifier::fromString(globalObject->globalExec(), propertyMeta->jsName()), descriptor, false);
+}
+
 void ObjCPrototype::materializeProperties(VM& vm, GlobalObject* globalObject) {
-    std::vector<const PropertyMeta*> properties = this->_metadata->instancePropertiesWithProtocols(this->klass());
-
-    for (const PropertyMeta* propertyMeta : properties) {
-        SymbolLoader::instance().ensureModule(propertyMeta->topLevelModule());
-
-        const MethodMeta* getter = (propertyMeta->getter() != nullptr && propertyMeta->getter()->isAvailable()) ? propertyMeta->getter() : nullptr;
-        const MethodMeta* setter = (propertyMeta->setter() != nullptr && propertyMeta->setter()->isAvailable()) ? propertyMeta->setter() : nullptr;
-
-        PropertyDescriptor descriptor;
-        descriptor.setConfigurable(true);
-        Strong<ObjCMethodWrapper> strongGetter;
-        Strong<ObjCMethodWrapper> strongSetter;
-        if (getter) {
-            MembersCollection getters = { getter };
-            strongGetter = ObjCMethodWrapper::create(vm, globalObject, globalObject->objCMethodWrapperStructure(), getters);
-            descriptor.setGetter(strongGetter.get());
-        }
+    // The cycle here works around an issue with incorrect public headers of some iOS system frameworks.
+    // In particular:
+    //   * UIBarItem doesn't define 6 of its declared properties (enabled, image, imageInsets,
+    //     landscapeImagePhone, landscapeImagePhoneInsets and title) but its inheritors UIBarButtonItem and
+    //     UITabBarItem do
+    //   * MTLRenderPassAttachmentDescriptor doesn't define 11 of its properties but it's inheritors
+    //     MTLRenderPassDepthAttachmentDescriptor, MTLRenderPassColorAttachmentDescriptor and
+    //     MTLRenderPassStencilAttachmentDescriptor do.
+    // As a result we were not providing their implementation in JS before we started looking for missing
+    // properties in the base class. This additional overhead increased the time spent in materializeProperties
+    // from ~5.3 sec to ~7.5 sec (~40%) when running TestRunner with ApiIterator test enabled in RelWithDebInfo configuration
+    // on an iPhone 6s device and from 3.0-3.2 to 4.4 sec (~40%) on an iPhone X
+
+    //    std::chrono::time_point<std::chrono::system_clock> startTime = std::chrono::system_clock::now();
+    //    int addedProps = 0;
+
+    const BaseClassMeta* meta = this->metadata();
+    Class klass = this->klass();
+    while (meta) {
+        std::vector<const PropertyMeta*> properties = meta->instancePropertiesWithProtocols(nullptr);
+
+        for (const PropertyMeta* propertyMeta : properties) {
+            bool shouldDefine = false;
+
+            if (klass == this->klass()) {
+                // Property is coming from this class, define it if available
+                shouldDefine = propertyMeta->isAvailableInClass(klass, false);
+            } else {
+                // Property is coming from a base class, define it as our property if isn't available there, but we've got it
+                shouldDefine = !propertyMeta->isAvailableInClass(klass, false) && propertyMeta->isAvailableInClass(this->klass(), false);
+                //                addedProps += shouldDefine ? 1 : 0;
+            }
 
-        if (setter) {
-            MembersCollection setters = { setter };
-            strongSetter = ObjCMethodWrapper::create(vm, globalObject, globalObject->objCMethodWrapperStructure(), setters);
-            descriptor.setSetter(strongSetter.get());
+            if (shouldDefine) {
+                this->defineNativeProperty(vm, globalObject, propertyMeta);
+            }
         }
 
-        Base::defineOwnProperty(this, globalObject->globalExec(), Identifier::fromString(globalObject->globalExec(), propertyMeta->jsName()), descriptor, false);
+        if (klass == this->klass() && meta->type() == Interface) {
+            meta = static_cast<const InterfaceMeta*>(meta)->baseMeta();
+            klass = meta ? objc_getClass(meta->name()) : nullptr;
+        } else {
+            // Check only properties from the direct base class and then stop.
+            // All the cases that we need to fix are like that so we don't have
+            // to pay the additional overhead of looking intothe whole inheritance chain.
+            meta = nullptr;
+        }
     }
+    //    std::chrono::time_point<std::chrono::system_clock> endTime = std::chrono::system_clock::now();
+    //    double duration = std::chrono::duration_cast<std::chrono::microseconds>(endTime - startTime).count() / 1000.;
+    //    static double totalDuration = 0;
+    //    totalDuration += duration;
+    //
+    //    std::cout << "**** materializeProperties " << this->metadata()->jsName() << ": " << duration << "(Total: " << totalDuration << ") added " << addedProps << std::endl;
 }
+
 }

tests/TestRunner/app/ApiTests.js

@@ -687,6 +687,16 @@ describe(module.id, function () {
         expect(result).toBe('method called');
     });
 
+    it("Unimplemented properties from UIBarItem class should be provided by the inheritors", function () {
+        var classConstructors = [ UIBarButtonItem, UITabBarItem ];
+        var props = ["enabled", "image", "imageInsets", "landscapeImagePhone", "landscapeImagePhoneInsets", "title"];
+        for (var klass of classConstructors) {
+            for (var prop of props) {
+                expect(klass.prototype[prop]).toBeDefined(`${prop} must be defined in the prototype of ${klass}`);
+            }
+        }
+    });
+
     if (TNSIsConfigurationDebug) {
         it("ApiIterator", function () {
             var counter = 0;