From 38e532067881a3c00a618dfc203cdac0989c9031 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D0=BB=D0=B5=D0=BA=D1=81=D0=B0=D0=BD=D0=B4=D1=80=20?= =?UTF-8?q?=D0=97=D0=B0=D1=86=D0=B5=D0=BF=D0=B8=D0=BD?= Date: Mon, 23 Dec 2019 17:25:09 +0300 Subject: [android] Fixed crash during access to C++ framework place page object while it's not intitialized. It may happen when the app was killed/dumped by OS when PP is opened (launched by incoming intent or deeplin) and then core doesn't have place page info and synchronization between platform and C++ framework is broken. So, we don't restore place page info from java map object and it's by design for a while. --- android/jni/com/mapswithme/maps/Framework.cpp | 6 ++++++ android/jni/com/mapswithme/maps/MapManager.cpp | 3 +++ android/jni/com/mapswithme/maps/Sponsored.cpp | 3 +++ .../maps/bookmarks/data/BookmarkManager.cpp | 6 ++++++ android/src/com/mapswithme/maps/Framework.java | 13 +++++++++++-- .../maps/bookmarks/data/BookmarkManager.java | 19 +++++++++++-------- .../placepage/BottomSheetPlacePageController.java | 16 +++++++++++----- .../maps/widget/placepage/PlacePageView.java | 6 +++++- map/framework.cpp | 4 ++-- map/framework.hpp | 2 +- 10 files changed, 59 insertions(+), 19 deletions(-) diff --git a/android/jni/com/mapswithme/maps/Framework.cpp b/android/jni/com/mapswithme/maps/Framework.cpp index d31dbd04fe..ec44257a21 100644 --- a/android/jni/com/mapswithme/maps/Framework.cpp +++ b/android/jni/com/mapswithme/maps/Framework.cpp @@ -2211,4 +2211,10 @@ Java_com_mapswithme_maps_Framework_nativeSetSearchViewport(JNIEnv *, jclass, jdo auto const rect = df::GetRectForDrawScale(static_cast(zoom), center); frm()->GetSearchAPI().OnViewportChanged(rect); } + +JNIEXPORT jboolean JNICALL +Java_com_mapswithme_maps_Framework_nativeHasPlacePageInfo(JNIEnv *, jclass) +{ + return static_cast(frm()->HasPlacePageInfo()); +} } // extern "C" diff --git a/android/jni/com/mapswithme/maps/MapManager.cpp b/android/jni/com/mapswithme/maps/MapManager.cpp index 5bb351f88e..330cbbb04a 100644 --- a/android/jni/com/mapswithme/maps/MapManager.cpp +++ b/android/jni/com/mapswithme/maps/MapManager.cpp @@ -569,6 +569,9 @@ Java_com_mapswithme_maps_downloader_MapManager_nativeEnableDownloadOn3g(JNIEnv * JNIEXPORT jstring JNICALL Java_com_mapswithme_maps_downloader_MapManager_nativeGetSelectedCountry(JNIEnv * env, jclass clazz) { + if (!g_framework->NativeFramework()->HasPlacePageInfo()) + return nullptr; + storage::CountryId const & res = g_framework->GetPlacePageInfo().GetCountryId(); return (res == storage::kInvalidCountryId ? nullptr : jni::ToJavaString(env, res)); } diff --git a/android/jni/com/mapswithme/maps/Sponsored.cpp b/android/jni/com/mapswithme/maps/Sponsored.cpp index 0e9eb09b09..fd67333481 100644 --- a/android/jni/com/mapswithme/maps/Sponsored.cpp +++ b/android/jni/com/mapswithme/maps/Sponsored.cpp @@ -126,6 +126,9 @@ JNIEXPORT jobject JNICALL Java_com_mapswithme_maps_widget_placepage_Sponsored_na { PrepareClassRefs(env, clazz); + if (!g_framework->NativeFramework()->HasPlacePageInfo()) + return nullptr; + place_page::Info const & ppInfo = g_framework->GetPlacePageInfo(); if (!ppInfo.IsSponsored()) return nullptr; diff --git a/android/jni/com/mapswithme/maps/bookmarks/data/BookmarkManager.cpp b/android/jni/com/mapswithme/maps/bookmarks/data/BookmarkManager.cpp index 45bb90e74a..cec418c841 100644 --- a/android/jni/com/mapswithme/maps/bookmarks/data/BookmarkManager.cpp +++ b/android/jni/com/mapswithme/maps/bookmarks/data/BookmarkManager.cpp @@ -622,6 +622,9 @@ JNIEXPORT jobject JNICALL Java_com_mapswithme_maps_bookmarks_data_BookmarkManager_nativeAddBookmarkToLastEditedCategory( JNIEnv * env, jobject thiz, double lat, double lon) { + if (!frm()->HasPlacePageInfo()) + return nullptr; + BookmarkManager & bmMng = frm()->GetBookmarkManager(); place_page::Info const & info = g_framework->GetPlacePageInfo(); @@ -775,6 +778,9 @@ JNIEXPORT jobject JNICALL Java_com_mapswithme_maps_bookmarks_data_BookmarkManager_nativeUpdateBookmarkPlacePage( JNIEnv * env, jobject thiz, jlong bmkId) { + if (!frm()->HasPlacePageInfo()) + return nullptr; + auto & info = g_framework->GetPlacePageInfo(); auto buildInfo = info.GetBuildInfo(); buildInfo.m_userMarkId = static_cast(bmkId); diff --git a/android/src/com/mapswithme/maps/Framework.java b/android/src/com/mapswithme/maps/Framework.java index bb3766b848..1e8da8a563 100644 --- a/android/src/com/mapswithme/maps/Framework.java +++ b/android/src/com/mapswithme/maps/Framework.java @@ -2,14 +2,14 @@ package com.mapswithme.maps; import android.graphics.Bitmap; import android.location.Location; +import android.text.TextUtils; + import androidx.annotation.IntDef; import androidx.annotation.MainThread; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.Size; import androidx.annotation.UiThread; -import android.text.TextUtils; - import com.mapswithme.maps.ads.Banner; import com.mapswithme.maps.ads.LocalAdInfo; import com.mapswithme.maps.api.ParsedRoutingData; @@ -528,6 +528,15 @@ public class Framework public static native void nativeSetSearchViewport(double lat, double lon, int zoom); + /** + * In case of the app was dumped by system to the hard drive, Java map object can be + * restored from parcelable, but c++ framework is created from scratch and internal + * place page object is not initialized. So, do not restore place page in this case. + * + * @return true if c++ framework has initialized internal place page object, otherwise - false. + */ + public static native boolean nativeHasPlacePageInfo(); + public enum LocalAdsEventType { LOCAL_ADS_EVENT_SHOW_POINT, diff --git a/android/src/com/mapswithme/maps/bookmarks/data/BookmarkManager.java b/android/src/com/mapswithme/maps/bookmarks/data/BookmarkManager.java index 871b583185..0b720e5a61 100644 --- a/android/src/com/mapswithme/maps/bookmarks/data/BookmarkManager.java +++ b/android/src/com/mapswithme/maps/bookmarks/data/BookmarkManager.java @@ -1,14 +1,13 @@ package com.mapswithme.maps.bookmarks.data; import androidx.annotation.IntDef; +import androidx.annotation.IntRange; import androidx.annotation.MainThread; import androidx.annotation.NonNull; -import androidx.annotation.IntRange; import androidx.annotation.Nullable; - +import com.mapswithme.maps.PrivateVariables; import com.mapswithme.maps.base.DataChangedListener; import com.mapswithme.maps.base.Observable; -import com.mapswithme.maps.PrivateVariables; import com.mapswithme.maps.metrics.UserActionsLogger; import com.mapswithme.util.KeyValue; import com.mapswithme.util.UTM; @@ -125,11 +124,15 @@ public enum BookmarkManager setVisibility(catId, !isVisible); } + @Nullable public Bookmark addNewBookmark(double lat, double lon) { final Bookmark bookmark = nativeAddBookmarkToLastEditedCategory(lat, lon); - UserActionsLogger.logAddToBookmarkEvent(); - Statistics.INSTANCE.trackBookmarkCreated(); + if (bookmark != null) + { + UserActionsLogger.logAddToBookmarkEvent(); + Statistics.INSTANCE.trackBookmarkCreated(); + } return bookmark; } @@ -466,7 +469,7 @@ public enum BookmarkManager nativeUploadToCatalog(rules.ordinal(), category.getId()); } - @NonNull + @Nullable public Bookmark updateBookmarkPlacePage(long bmkId) { return nativeUpdateBookmarkPlacePage(bmkId); @@ -930,7 +933,7 @@ public enum BookmarkManager private native int nativeGetTracksCount(long catId); - @NonNull + @Nullable private native Bookmark nativeUpdateBookmarkPlacePage(long bmkId); @Nullable @@ -974,7 +977,7 @@ public enum BookmarkManager private native void nativeShowBookmarkCategoryOnMap(long catId); - @NonNull + @Nullable private native Bookmark nativeAddBookmarkToLastEditedCategory(double lat, double lon); private native long nativeGetLastEditedCategory(); diff --git a/android/src/com/mapswithme/maps/widget/placepage/BottomSheetPlacePageController.java b/android/src/com/mapswithme/maps/widget/placepage/BottomSheetPlacePageController.java index 1c74686a36..bea214ef02 100644 --- a/android/src/com/mapswithme/maps/widget/placepage/BottomSheetPlacePageController.java +++ b/android/src/com/mapswithme/maps/widget/placepage/BottomSheetPlacePageController.java @@ -9,11 +9,6 @@ import android.graphics.Rect; import android.graphics.drawable.Drawable; import android.location.Location; import android.os.Bundle; -import androidx.annotation.DrawableRes; -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; -import androidx.core.view.GestureDetectorCompat; -import androidx.appcompat.widget.Toolbar; import android.util.Log; import android.view.GestureDetector; import android.view.MotionEvent; @@ -21,6 +16,11 @@ import android.view.View; import android.view.ViewGroup; import android.widget.ImageView; +import androidx.annotation.DrawableRes; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import androidx.appcompat.widget.Toolbar; +import androidx.core.view.GestureDetectorCompat; import com.mapswithme.maps.Framework; import com.mapswithme.maps.R; import com.mapswithme.maps.ads.CompoundNativeAdLoader; @@ -525,6 +525,12 @@ public class BottomSheetPlacePageController implements PlacePageController, Loca if (mPlacePageBehavior.getState() == AnchorBottomSheetBehavior.STATE_HIDDEN) return; + if (!Framework.nativeHasPlacePageInfo()) + { + close(); + return; + } + MapObject object = inState.getParcelable(EXTRA_MAP_OBJECT); if (object == null) return; diff --git a/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java b/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java index abd4e2b91e..0e6db36928 100644 --- a/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java +++ b/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java @@ -2079,7 +2079,11 @@ public class PlacePageView extends NestedScrollView @Override public void onBookmarkSaved(long bookmarkId, boolean movedFromCategory) { - setMapObject(BookmarkManager.INSTANCE.updateBookmarkPlacePage(bookmarkId), null); + Bookmark updatedBookmark = BookmarkManager.INSTANCE.updateBookmarkPlacePage(bookmarkId); + if (updatedBookmark == null) + return; + + setMapObject(updatedBookmark, null); NetworkPolicy policy = NetworkPolicy.newInstance(NetworkPolicy.getCurrentNetworkUsageStatus()); refreshViews(policy); } diff --git a/map/framework.cpp b/map/framework.cpp index 11a31bbfe4..28ed208a70 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -2364,13 +2364,13 @@ void Framework::SetPlacePageListeners(PlacePageEvent::OnOpen const & onOpen, place_page::Info const & Framework::GetCurrentPlacePageInfo() const { - CHECK(IsPlacePageOpened(), ()); + CHECK(HasPlacePageInfo(), ()); return m_currentPlacePageInfo.get(); } place_page::Info & Framework::GetCurrentPlacePageInfo() { - CHECK(IsPlacePageOpened(), ()); + CHECK(HasPlacePageInfo(), ()); return m_currentPlacePageInfo.get(); } diff --git a/map/framework.hpp b/map/framework.hpp index f147e87d77..234194048b 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -408,7 +408,7 @@ public: void SetPlacePageListeners(PlacePageEvent::OnOpen const & onOpen, PlacePageEvent::OnClose const & onClose, PlacePageEvent::OnUpdate const & onUpdate); - bool IsPlacePageOpened() const { return m_currentPlacePageInfo.has_value(); } + bool HasPlacePageInfo() const { return m_currentPlacePageInfo.has_value(); } place_page::Info const & GetCurrentPlacePageInfo() const; place_page::Info & GetCurrentPlacePageInfo(); -- cgit v1.2.3