[google_maps_flutter] Clean up google_maps_flutter plugin (#3206)

1. A few minor formatting changes and additions of @nullable annotations
2. Removed pass-through of activityHashCode. In the legacy plugin use case, the value is always -1 (which has been inlined). In the new plugin use case, the value should never be used because it uses DefaultLifecycleCallbacks instead of ActivityLifecycleCallbacks.
3. Replaced custom lifecycle state ints with androidx.lifecycle.Lifecycle.State enum. Also simplify paused/stopped states, which don't need their own dedicated states. Paused == started and stopped == created.
4. Fixed a bug where the Lifecycle object was being leaked onDetachFromActivity by nulling out the field.
5. Moved GoogleMapListener to its own file. Declaring multiple top level classes in the same file is bad practice.

Author: math1man

packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md

@@ -1,3 +1,12 @@
+## 1.0.4
+
+* Cleanup of Android code:
+* A few minor formatting changes and additions of `@Nullable` annotations.
+* Removed pass-through of `activityHashCode` to `GoogleMapController`.
+* Replaced custom lifecycle state ints with `androidx.lifecycle.Lifecycle.State` enum.
+* Fixed a bug where the Lifecycle object was being leaked `onDetachFromActivity`, by nulling out the field.
+* Moved GoogleMapListener to its own file. Declaring multiple top level classes in the same file is discouraged.
+
 ## 1.0.3
 
 * Update android compileSdkVersion to 29.

packages/google_maps_flutter/google_maps_flutter/android/build.gradle

@@ -8,7 +8,7 @@ buildscript {
     }
 
     dependencies {
-        classpath 'com.android.tools.build:gradle:3.3.2'
+        classpath 'com.android.tools.build:gradle:3.5.0'
     }
 }
 
@@ -33,6 +33,7 @@ android {
     }
 
     dependencies {
+        implementation "androidx.annotation:annotation:1.1.0"
         implementation 'com.google.android.gms:play-services-maps:17.0.0'
         androidTestImplementation 'androidx.test:runner:1.2.0'
         androidTestImplementation 'androidx.test:rules:1.2.0'

packages/google_maps_flutter/google_maps_flutter/android/src/main/java/io/flutter/plugins/googlemaps/GoogleMapBuilder.java

@@ -7,13 +7,14 @@
 import android.app.Application;
 import android.content.Context;
 import android.graphics.Rect;
+import androidx.annotation.Nullable;
 import androidx.lifecycle.Lifecycle;
+import androidx.lifecycle.Lifecycle.State;
 import com.google.android.gms.maps.GoogleMapOptions;
 import com.google.android.gms.maps.model.CameraPosition;
 import com.google.android.gms.maps.model.LatLngBounds;
 import io.flutter.plugin.common.BinaryMessenger;
 import io.flutter.plugin.common.PluginRegistry;
-import java.util.concurrent.atomic.AtomicInteger;
 
 class GoogleMapBuilder implements GoogleMapOptionsSink {
   private final GoogleMapOptions options = new GoogleMapOptions();
@@ -32,24 +33,15 @@ class GoogleMapBuilder implements GoogleMapOptionsSink {
   GoogleMapController build(
       int id,
       Context context,
-      AtomicInteger state,
+      State lifecycleState,
       BinaryMessenger binaryMessenger,
-      Application application,
-      Lifecycle lifecycle,
-      PluginRegistry.Registrar registrar,
-      int activityHashCode) {
+      @Nullable Application application,
+      @Nullable Lifecycle lifecycle,
+      @Nullable PluginRegistry.Registrar registrar) {
     final GoogleMapController controller =
         new GoogleMapController(
-            id,
-            context,
-            state,
-            binaryMessenger,
-            application,
-            lifecycle,
-            registrar,
-            activityHashCode,
-            options);
-    controller.init();
+            id, context, binaryMessenger, application, lifecycle, registrar, options);
+    controller.init(lifecycleState);
     controller.setMyLocationEnabled(myLocationEnabled);
     controller.setMyLocationButtonEnabled(myLocationButtonEnabled);
     controller.setIndoorEnabled(indoorEnabled);

packages/google_maps_flutter/google_maps_flutter/android/src/main/java/io/flutter/plugins/googlemaps/GoogleMapController.java

@@ -4,13 +4,6 @@
 
 package io.flutter.plugins.googlemaps;
 
-import static io.flutter.plugins.googlemaps.GoogleMapsPlugin.CREATED;
-import static io.flutter.plugins.googlemaps.GoogleMapsPlugin.DESTROYED;
-import static io.flutter.plugins.googlemaps.GoogleMapsPlugin.PAUSED;
-import static io.flutter.plugins.googlemaps.GoogleMapsPlugin.RESUMED;
-import static io.flutter.plugins.googlemaps.GoogleMapsPlugin.STARTED;
-import static io.flutter.plugins.googlemaps.GoogleMapsPlugin.STOPPED;
-
 import android.Manifest;
 import android.annotation.SuppressLint;
 import android.app.Activity;
@@ -26,6 +19,7 @@
 import androidx.annotation.Nullable;
 import androidx.lifecycle.DefaultLifecycleObserver;
 import androidx.lifecycle.Lifecycle;
+import androidx.lifecycle.Lifecycle.State;
 import androidx.lifecycle.LifecycleOwner;
 import com.google.android.gms.maps.CameraUpdate;
 import com.google.android.gms.maps.GoogleMap;
@@ -53,7 +47,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.atomic.AtomicInteger;
 
 /** Controller of a single GoogleMaps MapView instance. */
 final class GoogleMapController
@@ -68,7 +61,6 @@ final class GoogleMapController
 
   private static final String TAG = "GoogleMapController";
   private final int id;
-  private final AtomicInteger activityState;
   private final MethodChannel methodChannel;
   private final GoogleMapOptions options;
   @Nullable private MapView mapView;
@@ -83,13 +75,12 @@ final class GoogleMapController
   private boolean disposed = false;
   private final float density;
   private MethodChannel.Result mapReadyResult;
-  private final int
-      activityHashCode; // Do not use directly, use getActivityHashCode() instead to get correct hashCode for both v1 and v2 embedding.
-  private final Lifecycle lifecycle;
+  @Nullable private final Lifecycle lifecycle;
   private final Context context;
-  private final Application
-      mApplication; // Do not use direclty, use getApplication() instead to get correct application object for both v1 and v2 embedding.
-  private final PluginRegistry.Registrar registrar; // For v1 embedding only.
+  // Do not use directly, use getApplication() instead to get correct application object for both v1
+  // and v2 embedding.
+  @Nullable private final Application mApplication;
+  @Nullable private final PluginRegistry.Registrar registrar; // For v1 embedding only.
   private final MarkersController markersController;
   private final PolygonsController polygonsController;
   private final PolylinesController polylinesController;
@@ -102,16 +93,13 @@ final class GoogleMapController
   GoogleMapController(
       int id,
       Context context,
-      AtomicInteger activityState,
       BinaryMessenger binaryMessenger,
-      Application application,
-      Lifecycle lifecycle,
-      PluginRegistry.Registrar registrar,
-      int registrarActivityHashCode,
+      @Nullable Application application,
+      @Nullable Lifecycle lifecycle,
+      @Nullable PluginRegistry.Registrar registrar,
       GoogleMapOptions options) {
     this.id = id;
     this.context = context;
-    this.activityState = activityState;
     this.options = options;
     this.mapView = new MapView(context, options);
     this.density = context.getResources().getDisplayMetrics().density;
@@ -120,7 +108,6 @@ final class GoogleMapController
     mApplication = application;
     this.lifecycle = lifecycle;
     this.registrar = registrar;
-    this.activityHashCode = registrarActivityHashCode;
     this.markersController = new MarkersController(methodChannel);
     this.polygonsController = new PolygonsController(methodChannel, density);
     this.polylinesController = new PolylinesController(methodChannel, density);
@@ -132,21 +119,8 @@ public View getView() {
     return mapView;
   }
 
-  void init() {
-    switch (activityState.get()) {
-      case STOPPED:
-        mapView.onCreate(null);
-        mapView.onStart();
-        mapView.onResume();
-        mapView.onPause();
-        mapView.onStop();
-        break;
-      case PAUSED:
-        mapView.onCreate(null);
-        mapView.onStart();
-        mapView.onResume();
-        mapView.onPause();
-        break;
+  void init(State lifecycleState) {
+    switch (lifecycleState) {
       case RESUMED:
         mapView.onCreate(null);
         mapView.onStart();
@@ -160,11 +134,9 @@ void init() {
         mapView.onCreate(null);
         break;
       case DESTROYED:
-        // Nothing to do, the activity has been completely destroyed.
+      case INITIALIZED:
+        // Nothing to do, the activity has been completely destroyed or not yet created.
         break;
-      default:
-        throw new IllegalArgumentException(
-            "Cannot interpret " + activityState.get() + " as an activity state");
     }
     if (lifecycle != null) {
       lifecycle.addObserver(this);
@@ -556,14 +528,14 @@ private void setGoogleMapListener(@Nullable GoogleMapListener listener) {
   // does. This will override it when available even with the annotation commented out.
   public void onInputConnectionLocked() {
     // TODO(mklim): Remove this empty override once https://github.com/flutter/flutter/issues/40126 is fixed in stable.
-  };
+  }
 
   // @Override
   // The minimum supported version of Flutter doesn't have this method on the PlatformView interface, but the maximum
   // does. This will override it when available even with the annotation commented out.
   public void onInputConnectionUnlocked() {
     // TODO(mklim): Remove this empty override once https://github.com/flutter/flutter/issues/40126 is fixed in stable.
-  };
+  }
 
   // Application.ActivityLifecycleCallbacks methods
   @Override
@@ -880,7 +852,9 @@ private int getActivityHashCode() {
     if (registrar != null && registrar.activity() != null) {
       return registrar.activity().hashCode();
     } else {
-      return activityHashCode;
+      // TODO(cyanglaz): Remove `getActivityHashCode()` and use a cached hashCode when creating the view for V1 embedding.
+      // https://github.com/flutter/flutter/issues/69128
+      return -1;
     }
   }
 
@@ -916,16 +890,3 @@ public void setBuildingsEnabled(boolean buildingsEnabled) {
     this.buildingsEnabled = buildingsEnabled;
   }
 }
-
-interface GoogleMapListener
-    extends GoogleMap.OnCameraIdleListener,
-        GoogleMap.OnCameraMoveListener,
-        GoogleMap.OnCameraMoveStartedListener,
-        GoogleMap.OnInfoWindowClickListener,
-        GoogleMap.OnMarkerClickListener,
-        GoogleMap.OnPolygonClickListener,
-        GoogleMap.OnPolylineClickListener,
-        GoogleMap.OnCircleClickListener,
-        GoogleMap.OnMapClickListener,
-        GoogleMap.OnMapLongClickListener,
-        GoogleMap.OnMarkerDragListener {}

packages/google_maps_flutter/google_maps_flutter/android/src/main/java/io/flutter/plugins/googlemaps/GoogleMapFactory.java

@@ -6,37 +6,36 @@
 
 import android.app.Application;
 import android.content.Context;
+import androidx.annotation.Nullable;
 import androidx.lifecycle.Lifecycle;
+import androidx.lifecycle.Lifecycle.State;
 import com.google.android.gms.maps.model.CameraPosition;
 import io.flutter.plugin.common.BinaryMessenger;
 import io.flutter.plugin.common.PluginRegistry;
 import io.flutter.plugin.common.StandardMessageCodec;
 import io.flutter.plugin.platform.PlatformView;
 import io.flutter.plugin.platform.PlatformViewFactory;
 import java.util.Map;
-import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
 
 public class GoogleMapFactory extends PlatformViewFactory {
 
-  private final AtomicInteger mActivityState;
+  private final AtomicReference<State> lifecycleState;
   private final BinaryMessenger binaryMessenger;
-  private final Application application;
-  private final int activityHashCode;
-  private final Lifecycle lifecycle;
-  private final PluginRegistry.Registrar registrar; // V1 embedding only.
+  @Nullable private final Application application;
+  @Nullable private final Lifecycle lifecycle;
+  @Nullable private final PluginRegistry.Registrar registrar; // V1 embedding only.
 
   GoogleMapFactory(
-      AtomicInteger state,
+      AtomicReference<State> lifecycleState,
       BinaryMessenger binaryMessenger,
-      Application application,
-      Lifecycle lifecycle,
-      PluginRegistry.Registrar registrar,
-      int activityHashCode) {
+      @Nullable Application application,
+      @Nullable Lifecycle lifecycle,
+      @Nullable PluginRegistry.Registrar registrar) {
     super(StandardMessageCodec.INSTANCE);
-    mActivityState = state;
+    this.lifecycleState = lifecycleState;
     this.binaryMessenger = binaryMessenger;
     this.application = application;
-    this.activityHashCode = activityHashCode;
     this.lifecycle = lifecycle;
     this.registrar = registrar;
   }
@@ -65,13 +64,6 @@ public PlatformView create(Context context, int id, Object args) {
       builder.setInitialCircles(params.get("circlesToAdd"));
     }
     return builder.build(
-        id,
-        context,
-        mActivityState,
-        binaryMessenger,
-        application,
-        lifecycle,
-        registrar,
-        activityHashCode);
+        id, context, lifecycleState.get(), binaryMessenger, application, lifecycle, registrar);
   }
 }

packages/google_maps_flutter/google_maps_flutter/android/src/main/java/io/flutter/plugins/googlemaps/GoogleMapListener.java

@@ -0,0 +1,20 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package io.flutter.plugins.googlemaps;
+
+import com.google.android.gms.maps.GoogleMap;
+
+interface GoogleMapListener
+    extends GoogleMap.OnCameraIdleListener,
+        GoogleMap.OnCameraMoveListener,
+        GoogleMap.OnCameraMoveStartedListener,
+        GoogleMap.OnInfoWindowClickListener,
+        GoogleMap.OnMarkerClickListener,
+        GoogleMap.OnPolygonClickListener,
+        GoogleMap.OnPolylineClickListener,
+        GoogleMap.OnCircleClickListener,
+        GoogleMap.OnMapClickListener,
+        GoogleMap.OnMapLongClickListener,
+        GoogleMap.OnMarkerDragListener {}