[android] Fixed crash during access to C++ framework place page objec…

…t 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.
mapsme · Dec 23, 2019 · 38e5320 · 38e5320
1 parent 766818f
commit 38e5320
Show file tree

Hide file tree

Showing 10 changed files with 59 additions and 19 deletions.
diff --git 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<int>(zoom), center);
   frm()->GetSearchAPI().OnViewportChanged(rect);
 }
+
+JNIEXPORT jboolean JNICALL
+Java_com_mapswithme_maps_Framework_nativeHasPlacePageInfo(JNIEnv *, jclass)
+{
+  return static_cast<jboolean>(frm()->HasPlacePageInfo());
+}
 }  // extern "C"
diff --git 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
@@ -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
@@ -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<kml::MarkId>(bmkId);

diff --git a/android/src/com/mapswithme/maps/Framework.java b/android/src/com/mapswithme/maps/Framework.java
@@ -2,14 +2,14 @@
 
 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 static native void nativeSetViewportCenter(double lat, double lon, int zo
 
   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
@@ -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 void toggleCategoryVisibility(long catId)
     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 void uploadToCatalog(@NonNull BookmarkCategory.AccessRules rules, @NonNul
     nativeUploadToCatalog(rules.ordinal(), category.getId());
   }
 
-  @NonNull
+  @Nullable
   public Bookmark updateBookmarkPlacePage(long bmkId)
   {
     return nativeUpdateBookmarkPlacePage(bmkId);
@@ -930,7 +933,7 @@ public void notifyParametersUpdating(@NonNull Bookmark bookmark, @NonNull String
 
   private native int nativeGetTracksCount(long catId);
 
-  @NonNull
+  @Nullable
   private native Bookmark nativeUpdateBookmarkPlacePage(long bmkId);
 
   @Nullable
@@ -974,7 +977,7 @@ public void notifyParametersUpdating(@NonNull Bookmark bookmark, @NonNull String
 
   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
@@ -9,18 +9,18 @@
 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;
 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 void onRestore(@NonNull Bundle inState)
     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
@@ -2079,7 +2079,11 @@ MwmActivity getActivity()
   @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
@@ -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
@@ -408,7 +408,7 @@ class Framework : public PositionProvider,
   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();