From 8a9e9b803e2f619262927f75c75508c0ff254599 Mon Sep 17 00:00:00 2001 From: vng Date: Fri, 18 May 2012 12:01:43 +0300 Subject: [PATCH] [android] Fix bug with native nested Java object creation (simply avoid this). --- .../jni/com/mapswithme/core/jni_helper.cpp | 21 +++++++ .../jni/com/mapswithme/core/jni_helper.hpp | 12 +++- .../maps/DownloadResourcesActivity.cpp | 16 ++--- .../jni/com/mapswithme/maps/MapStorage.cpp | 59 +++++++++---------- .../src/com/mapswithme/maps/DownloadUI.java | 2 + .../src/com/mapswithme/maps/MapStorage.java | 23 +++++++- storage/storage.cpp | 7 +++ storage/storage.hpp | 2 + 8 files changed, 96 insertions(+), 46 deletions(-) diff --git a/android/jni/com/mapswithme/core/jni_helper.cpp b/android/jni/com/mapswithme/core/jni_helper.cpp index 90e087150d..60491d1141 100644 --- a/android/jni/com/mapswithme/core/jni_helper.cpp +++ b/android/jni/com/mapswithme/core/jni_helper.cpp @@ -35,13 +35,34 @@ namespace jni { ASSERT(env, ("JNIEnv can't be 0")); ASSERT(obj, ("jobject can't be 0")); + jclass cls = env->GetObjectClass(obj); ASSERT(cls, ("Can't get java class")); + jmethodID mid = env->GetMethodID(cls, fn, sig); ASSERT(mid, ("Can't find java method", fn, sig)); return mid; } + /* + jobject CreateJavaObject(JNIEnv * env, char const * klassName, char const * sig, ...) + { + jclass klass = env->FindClass(klassName); + ASSERT(klass, ("Can't find java class", klassName)); + + jmethodID methodId = env->GetMethodID(klass, "", sig); + ASSERT(methodId, ("Can't find java constructor", sig)); + + va_list args; + va_start(args, sig); + jobject res = env->NewObject(klass, methodId, args); + ASSERT(res, ()); + va_end(args); + + return res; + } + */ + string ToString(jstring str) { JNIEnv * env = GetEnv(); diff --git a/android/jni/com/mapswithme/core/jni_helper.hpp b/android/jni/com/mapswithme/core/jni_helper.hpp index 44c2b4a7d3..72ef06fe97 100644 --- a/android/jni/com/mapswithme/core/jni_helper.hpp +++ b/android/jni/com/mapswithme/core/jni_helper.hpp @@ -7,10 +7,16 @@ namespace jni { - // Some examples of signature: - // "()V" - void function returning void; - // "(Ljava/lang/String;)V" - String function returning void; + /// @name Some examples of signature: + /// "()V" - void function returning void; + /// "(Ljava/lang/String;)V" - String function returning void; + /// "com/mapswithme/maps/MapStorage$Index" - class MapStorage.Index + //@{ jmethodID GetJavaMethodID(JNIEnv * env, jobject obj, char const * fn, char const * sig); + + //jobject CreateJavaObject(JNIEnv * env, char const * klass, char const * sig, ...); + //@} + string ToString(jstring str); JNIEnv * GetEnv(); JavaVM * GetJVM(); diff --git a/android/jni/com/mapswithme/maps/DownloadResourcesActivity.cpp b/android/jni/com/mapswithme/maps/DownloadResourcesActivity.cpp index e9001646da..e9accdcc1c 100644 --- a/android/jni/com/mapswithme/maps/DownloadResourcesActivity.cpp +++ b/android/jni/com/mapswithme/maps/DownloadResourcesActivity.cpp @@ -165,21 +165,14 @@ extern "C" JNIEnv * env = jni::GetEnv(); - jmethodID onFinishMethod = env->GetMethodID(env->GetObjectClass(*obj.get()), "onDownloadFinished", "(I)V"); - ASSERT(onFinishMethod, ("Not existing method: void onDownloadFinished(int)")); - - env->CallVoidMethod(*obj.get(), onFinishMethod, errorCode); + jmethodID methodID = jni::GetJavaMethodID(env, *obj.get(), "onDownloadFinished", "(I)V"); + env->CallVoidMethod(*obj.get(), methodID, errorCode); } void DownloadFileProgress(shared_ptr obj, downloader::HttpRequest & req) { LOG(LDEBUG, (req.Progress().first, "bytes for", g_filesToDownload.back().m_fileName, "was downloaded")); - JNIEnv * env = jni::GetEnv(); - - jmethodID onProgressMethod = env->GetMethodID(env->GetObjectClass(*obj.get()), "onDownloadProgress", "(IIII)V"); - ASSERT(onProgressMethod, ("Not existing method: void onDownloadProgress(int, int, int, int)")); - FileToDownload & curFile = g_filesToDownload.back(); jint curTotal = req.Progress().second; @@ -187,7 +180,10 @@ extern "C" jint glbTotal = g_totalBytesToDownload; jint glbProgress = g_totalDownloadedBytes + req.Progress().first; - env->CallVoidMethod(*obj.get(), onProgressMethod, + JNIEnv * env = jni::GetEnv(); + + jmethodID methodID = jni::GetJavaMethodID(env, *obj.get(), "onDownloadProgress", "(IIII)V"); + env->CallVoidMethod(*obj.get(), methodID, curTotal, curProgress, glbTotal, glbProgress); } diff --git a/android/jni/com/mapswithme/maps/MapStorage.cpp b/android/jni/com/mapswithme/maps/MapStorage.cpp index b273fda77d..8fca668bcf 100644 --- a/android/jni/com/mapswithme/maps/MapStorage.cpp +++ b/android/jni/com/mapswithme/maps/MapStorage.cpp @@ -3,44 +3,46 @@ /////////////////////////////////////////////////////////////////////////////////// #include "Framework.hpp" + #include "../core/jni_helper.hpp" + extern "C" { class IndexBinding { private: - shared_ptr m_self; jfieldID m_groupID; jfieldID m_countryID; jfieldID m_regionID; - public: + jobject object() const { return *m_self.get(); } + public: IndexBinding(jobject self) : m_self(jni::make_global_ref(self)) { - jclass cls = jni::GetEnv()->GetObjectClass(*m_self.get()); + jclass klass = jni::GetEnv()->GetObjectClass(object()); - m_groupID = jni::GetEnv()->GetFieldID(cls, "mGroup", "I"); - m_countryID = jni::GetEnv()->GetFieldID(cls, "mCountry", "I"); - m_regionID = jni::GetEnv()->GetFieldID(cls, "mRegion", "I"); + m_groupID = jni::GetEnv()->GetFieldID(klass, "mGroup", "I"); + m_countryID = jni::GetEnv()->GetFieldID(klass, "mCountry", "I"); + m_regionID = jni::GetEnv()->GetFieldID(klass, "mRegion", "I"); } int group() const { - return jni::GetEnv()->GetIntField(*m_self.get(), m_groupID); + return jni::GetEnv()->GetIntField(object(), m_groupID); } int country() const { - return jni::GetEnv()->GetIntField(*m_self.get(), m_countryID); + return jni::GetEnv()->GetIntField(object(), m_countryID); } int region() const { - return jni::GetEnv()->GetIntField(*m_self.get(), m_regionID); + return jni::GetEnv()->GetIntField(object(), m_regionID); } storage::TIndex const toNative() const @@ -50,17 +52,16 @@ extern "C" static jobject toJava(storage::TIndex const & idx) { - LOG(LDEBUG, ("constructing Java Index object from ", idx.m_group, idx.m_country, idx.m_region)); - JNIEnv * env = jni::GetEnv(); - jclass klass = env->FindClass("com/mapswithme/maps/MapStorage$Index"); - ASSERT(klass, ("Can't find java class com/mapswithme/maps/MapStorage$Index")); + jclass klass = env->FindClass("com/mapswithme/maps/MapStorage"); + ASSERT(klass, ()); - jmethodID methodId = env->GetMethodID(klass, "", "(III)V"); - ASSERT(methodId, ("Can't find java constructor in com/mapswithme/maps/MapStorage$Index")); + jmethodID methodId = env->GetStaticMethodID(klass, + "createIndex", "(III)Lcom/mapswithme/maps/MapStorage$Index;"); + ASSERT(methodId, ()); - return env->NewObject(klass, methodId, idx.m_group, idx.m_country, idx.m_region); + return env->CallStaticObjectMethod(klass, methodId, idx.m_group, idx.m_country, idx.m_region); } }; @@ -136,37 +137,35 @@ extern "C" { JNIEnv * env = jni::GetEnv(); - jclass klass = env->GetObjectClass(*obj.get()); - - jmethodID methodID = env->GetMethodID(klass, "onCountryStatusChanged", "(Lcom/mapswithme/maps/MapStorage$Index;)V"); - + jmethodID methodID = jni::GetJavaMethodID(env, *obj.get(), "onCountryStatusChanged", "(Lcom/mapswithme/maps/MapStorage$Index;)V"); env->CallVoidMethod(*obj.get(), methodID, IndexBinding::toJava(idx)); } void ReportCountryProgress(shared_ptr const & obj, storage::TIndex const & idx, pair const & p) { + jlong const current = p.first; + jlong const total = p.second; + JNIEnv * env = jni::GetEnv(); - jclass klass = env->GetObjectClass(*obj.get()); - - jmethodID methodID = env->GetMethodID(klass, "onCountryProgress", "(Lcom/mapswithme/maps/MapStorage$Index;JJ)V"); - - jlong current = p.first; - jlong total = p.second; + jmethodID methodID = jni::GetJavaMethodID(env, *obj.get(), "onCountryProgress", "(Lcom/mapswithme/maps/MapStorage$Index;JJ)V"); env->CallVoidMethod(*obj.get(), methodID, IndexBinding::toJava(idx), current, total); } JNIEXPORT jint JNICALL - Java_com_mapswithme_maps_MapStorage_subscribe(JNIEnv * env, jobject thiz, jobject obs) + Java_com_mapswithme_maps_MapStorage_subscribe(JNIEnv * env, jobject thiz, jobject obj) { - jint res = g_framework->Storage().Subscribe(bind(&ReportChangeCountryStatus, jni::make_global_ref(obs), _1), - bind(&ReportCountryProgress, jni::make_global_ref(obs), _1, _2)); - return res; + LOG(LDEBUG, ("Subscribe on storage")); + + return g_framework->Storage().Subscribe(bind(&ReportChangeCountryStatus, jni::make_global_ref(obj), _1), + bind(&ReportCountryProgress, jni::make_global_ref(obj), _1, _2)); } JNIEXPORT void JNICALL Java_com_mapswithme_maps_MapStorage_unsubscribe(JNIEnv * env, jobject thiz, jint slotID) { + LOG(LDEBUG, ("UnSubscribe from storage")); + g_framework->Storage().Unsubscribe(slotID); } } diff --git a/android/src/com/mapswithme/maps/DownloadUI.java b/android/src/com/mapswithme/maps/DownloadUI.java index 9c7c106a33..dedb9f15dd 100644 --- a/android/src/com/mapswithme/maps/DownloadUI.java +++ b/android/src/com/mapswithme/maps/DownloadUI.java @@ -506,6 +506,8 @@ public class DownloadUI extends ListActivity implements MapStorage.Listener public void onCountryStatusChanged(Index idx) { + Log.d(TAG, "onCountryStatusChanged for index: " + idx.toString()); + final int position = getItemPosition(idx); if (position != -1) { diff --git a/android/src/com/mapswithme/maps/MapStorage.java b/android/src/com/mapswithme/maps/MapStorage.java index 74a737c59e..3fdf7048ae 100644 --- a/android/src/com/mapswithme/maps/MapStorage.java +++ b/android/src/com/mapswithme/maps/MapStorage.java @@ -76,9 +76,14 @@ public class MapStorage return ret; } + public boolean isEqual(Index idx) + { + return (mGroup == idx.mGroup && mCountry == idx.mCountry && mRegion == idx.mRegion); + } + public boolean isChild(Index idx) { - return (idx.getParent().equals(this)); + return (idx.getParent().isEqual(this)); } public int getPosition() @@ -87,6 +92,12 @@ public class MapStorage else if (mCountry != -1) return mCountry; else return mGroup; } + + @Override + public String toString() + { + return ("Index(" + mGroup + ", " + mCountry + ", " + mRegion + ")"); + } } public native int countriesCount(Index idx); @@ -103,6 +114,10 @@ public class MapStorage public native void showCountry(Index idx); + public native int subscribe(Listener l); + public native void unsubscribe(int slotId); + + private MapStorage() { } @@ -117,6 +132,8 @@ public class MapStorage return mInstance; } - public native int subscribe(Listener l); - public native void unsubscribe(int slotId); + public static Index createIndex(int group, int country, int region) + { + return new Index(group, country, region); + } } diff --git a/storage/storage.cpp b/storage/storage.cpp index 2a08f057cb..78564ebc6b 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -33,6 +33,13 @@ namespace storage { const int TIndex::INVALID = -1; + string DebugPrint(TIndex const & r) + { + ostringstream out; + out << "storage::TIndex(" << r.m_group << ", " << r.m_country << ", " << r.m_region << ")"; + return out.str(); + } + // static string ErrorString(DownloadResultT res) // { // switch (res) diff --git a/storage/storage.hpp b/storage/storage.hpp index ca9ad44fe1..afae87ce11 100644 --- a/storage/storage.hpp +++ b/storage/storage.hpp @@ -53,6 +53,8 @@ namespace storage } }; + string DebugPrint(TIndex const & r); + /// Can be used to store local maps and/or maps available for download class Storage {