Convert Android bindings to use high-level jni.hpp constructs

[kkaefer]

In some places, we're still using the low-level bindings of jni.hpp. We should convert them to the high-level bindings, since they provide more type safety and less room for error.

This PR also contains mbgl::android::ClassBinding, and mbgl::android::MethodBinding, which are intended as generic wrapper classes, and provide a way to lazily retrieve the underlying JNI class object, and, for the method binding, a way to declare and invoke object member functions. It also removes a lot of boilerplate code from declaring these class/method wrappers. For example, declaring a class with three member functions looks like this:

struct Number : public ClassBinding<Number> {
    static constexpr auto Name() { return "java/lang/Number"; }

    struct FloatValue : public MethodBinding<Number, FloatValue, jni::jfloat(void)> {
        static constexpr auto Name() { return "floatValue"; }
    };

    struct DoubleValue : public MethodBinding<Number, DoubleValue, jni::jdouble(void)> {
        static constexpr auto Name() { return "doubleValue"; }
    };

    struct LongValue : public MethodBinding<Number, LongValue, jni::jlong(void)> {
        static constexpr auto Name() { return "longValue"; }
    };
};

and you can call them with Number::FloatValue::Call(env, obj), which returns a jni::float (as declared in the function signature. All of this is implemented in the header, and it shouldn't add any overhead.

Since lazily retrieving functions/methods is prone to runtime errors, the debug build collects all the defined bindings at program startup and runs the "lazy" getter on all of them. This allows us to catch missing objects and typos early on.

[kkaefer]

Also noting that this is a WIP; review request is for the move to *Binding

[ivovandongen]

@kkaefer Looks great, thanks for picking this up. In general I think the lazy loading alone is already worth it, to shave some time off of the library loading. Love how it clears away the repetition in the the class bindings too.

We do need to document this properly as jni.hpp is already on the complicated side and adding more templates in the mix might not be intuitive for everybody.

[jfirebaugh]

Looks like ObjectDeleter is no longer being used. I think this was important for conversions that create many Value instances within a loop, to avoid overflowing the local reference table. We should use jni::UniqueLocalObject to make this explicit.

[kkaefer]

I changed this PR to remove inheritance from *Binding. Instead, they're now typedefs. The *Tag classes now exclusively have the static Name() function, and we're defining typedefs to jni::Object<*Tag> to mirror how jni::String and jni::StringTag work. The premise of the tagging system is that there exists only one single Tag for a particular string (e.g. having two tags that both return java/lang/Object breaks type conversion).

At the example of java/lang/Boolean, here's how to define the tag itself, a constructor, a member function and a jni::Make implementation:

struct BooleanTag { static constexpr auto Name() { return "java/lang/Boolean"; } };

// Mirrors jni::String
using Boolean = jni::Object<BooleanTag>;

// Naming convention is "Class name"_constructor. It declares the constructor that takes a `boolean` primitive
using Boolean_constructor = binding::Constructor<BooleanTag, jni::jboolean>;

// Defines a tag for the "booleanValue" member function, and declares the member function with a signature of boolean(void)
struct Boolean_booleanValueTag { static constexpr auto Name() { return "booleanValue"; } };
using Boolean_booleanValue = binding::Method<BooleanTag, Boolean_booleanValueTag, jni::jboolean(void)>;

// Convenience overload that uses the Java primitive constructor to convert C++ primitives.
template <typename T>
auto MakeAnything(jni::ThingToMake<Boolean>, jni::JNIEnv& env, T value) {
    return jni::Make<Boolean>(env, Boolean_constructor(), jni::jboolean(value));
}

[kkaefer]

Here's how to create a new Boolean object with the helper function:

 jni::Make<java::lang::Boolean>(env, true);

Here's how to call the member function:

java::lang::Boolean_booleanValue::Call(env, value);

We could add a dispatch system similar to jni::Make named jni::Call that calls free CallAnything functions, which would change the call to

jni::Call<java::lang::Boolean_booleanValue>(env, value);

but that'd require an update to jni.hpp

[jfirebaugh]

Nice, I like this.

Is this syntax feasible: java::lang::Boolean_booleanValue(env, value)?

Since there's no way to define a static operator(), I think this would require java::lang::Boolean_booleanValue to be a value rather than a type; i.e.:

extern binding::Method<BooleanTag, Boolean_booleanValueTag, jni::jboolean(void)>
    Boolean_booleanValue;

plus a definition in a .cpp file, with:

template <class ClassTag, class MethodTag, class R, class... Args>
struct Method<ClassTag, MethodTag, R(Args...)> {
private:
#ifndef NDEBUG
    Method() {
        RegisterNative(&Method::Get);
    }
#endif

    static auto Get(jni::JNIEnv& env) {
        static const auto javaMethod =
            Class<ClassTag>::Get(env).template GetMethod<R(Args...)>(env, MethodTag::Name());
        return javaMethod;
    }

public:
    template <class... Params>
    auto operator()(jni::JNIEnv& env, jni::Object<ClassTag> obj, Params&&... params) {
        return obj.Call(env, Get(env), std::forward<Params>(params)...);
    }
};

[jfirebaugh]

Is the void in the parameter list necessary or can this be jni::jboolean ()?

[kkaefer]

It's not necessary, but I added it for clarity and to disambiguate it from a function call. We can remove it if you prefer.

[kkaefer]

Is this syntax feasible: java::lang::Boolean_booleanValue(env, value)?

It is with the way you're describing, but I opted to not do it that way because it means we'd have to add implementation files whereas the current implementation is header-only.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

[ChrisLoer]

@kkaefer @ivovandongen Was this PR ready to merge? Should we still do it or is this closing appropriate?

[kkaefer]

It has been superseded by later changes to jni.hpp in #12716