Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[google_maps_flutter] Overhaul lifecycle management in GoogleMapsPlugin #3213

Merged
merged 14 commits into from
Oct 29, 2020
Merged

[google_maps_flutter] Overhaul lifecycle management in GoogleMapsPlugin #3213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c977679
GoogleMapController is now uniformly driven by implementing DefaultLi…
math1man Oct 27, 2020
e934a18
Update CHANGELOG.md and version
math1man Oct 27, 2020
9bdc386
Merge remote-tracking branch 'origin/master'
math1man Oct 27, 2020
96cf26b
Fix formatting issues
math1man Oct 28, 2020
62476dc
Update GoogleMapControllerTest
math1man Oct 28, 2020
a8e02a4
Merge remote-tracking branch 'origin/master'
math1man Oct 28, 2020
b920973
Remove extraneous import
math1man Oct 28, 2020
ea016b8
Update GoogleMapsPlugin.ProxyLifecycleProvider to use handleLifecycle…
math1man Oct 28, 2020
d6c9014
Merge remote-tracking branch 'origin/master'
math1man Oct 28, 2020
5f479f0
Update version from 1.1.0 to 1.0.5
math1man Oct 28, 2020
2c182a9
Merge remote-tracking branch 'origin/master'
math1man Oct 28, 2020
039e3ce
Add javadoc for ProxyLifecycleProvider and move initial ActivityLifec…
math1man Oct 28, 2020
85e592e
Merge remote-tracking branch 'origin/master'
math1man Oct 28, 2020
df2345d
Remove LifecycleProvider as an interface of GoogleMapsPlugin and inst…
math1man Oct 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
## 1.0.5

Overhaul lifecycle management in GoogleMapsPlugin.

GoogleMapController is now uniformly driven by implementing `DefaultLifecycleObserver`. That observer is registered to a lifecycle from one of three sources:

1. For v2 plugin registration, `GoogleMapsPlugin` obtains the lifecycle via `ActivityAware` methods.
2. For v1 plugin registration, if the activity implements `LifecycleOwner`, it's lifecycle is used directly.
3. For v1 plugin registration, if the activity does not implement `LifecycleOwner`, a proxy lifecycle is created and driven via `ActivityLifecycleCallbacks`.

## 1.0.4

* Cleanup of Android code:
Expand Down
15 changes: 3 additions & 12 deletions ...le_maps_flutter/android/src/main/java/io/flutter/plugins/googlemaps/GoogleMapBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,12 @@

package io.flutter.plugins.googlemaps;

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;

class GoogleMapBuilder implements GoogleMapOptionsSink {
private final GoogleMapOptions options = new GoogleMapOptions();
Expand All @@ -33,15 +28,11 @@ class GoogleMapBuilder implements GoogleMapOptionsSink {
GoogleMapController build(
int id,
Context context,
State lifecycleState,
BinaryMessenger binaryMessenger,
@Nullable Application application,
@Nullable Lifecycle lifecycle,
@Nullable PluginRegistry.Registrar registrar) {
LifecycleProvider lifecycleProvider) {
final GoogleMapController controller =
new GoogleMapController(
id, context, binaryMessenger, application, lifecycle, registrar, options);
controller.init(lifecycleState);
new GoogleMapController(id, context, binaryMessenger, lifecycleProvider, options);
controller.init();
controller.setMyLocationEnabled(myLocationEnabled);
controller.setMyLocationButtonEnabled(myLocationButtonEnabled);
controller.setIndoorEnabled(indoorEnabled);
Expand Down
130 changes: 12 additions & 118 deletions ...maps_flutter/android/src/main/java/io/flutter/plugins/googlemaps/GoogleMapController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@

import android.Manifest;
import android.annotation.SuppressLint;
import android.app.Activity;
import android.app.Application;
import android.content.Context;
import android.content.pm.PackageManager;
import android.graphics.Bitmap;
Expand All @@ -19,7 +17,6 @@
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;
Expand All @@ -39,7 +36,6 @@
import io.flutter.plugin.common.BinaryMessenger;
import io.flutter.plugin.common.MethodCall;
import io.flutter.plugin.common.MethodChannel;
import io.flutter.plugin.common.PluginRegistry;
import io.flutter.plugin.platform.PlatformView;
import java.io.ByteArrayOutputStream;
import java.util.ArrayList;
Expand All @@ -50,8 +46,7 @@

/** Controller of a single GoogleMaps MapView instance. */
final class GoogleMapController
implements Application.ActivityLifecycleCallbacks,
DefaultLifecycleObserver,
implements DefaultLifecycleObserver,
ActivityPluginBinding.OnSaveInstanceStateListener,
GoogleMapOptionsSink,
MethodChannel.MethodCallHandler,
Expand All @@ -75,12 +70,8 @@ final class GoogleMapController
private boolean disposed = false;
private final float density;
private MethodChannel.Result mapReadyResult;
@Nullable private final Lifecycle lifecycle;
private final Context context;
// 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 LifecycleProvider lifecycleProvider;
private final MarkersController markersController;
private final PolygonsController polygonsController;
private final PolylinesController polylinesController;
Expand All @@ -94,9 +85,7 @@ final class GoogleMapController
int id,
Context context,
BinaryMessenger binaryMessenger,
@Nullable Application application,
@Nullable Lifecycle lifecycle,
@Nullable PluginRegistry.Registrar registrar,
LifecycleProvider lifecycleProvider,
GoogleMapOptions options) {
this.id = id;
this.context = context;
Expand All @@ -105,9 +94,7 @@ final class GoogleMapController
this.density = context.getResources().getDisplayMetrics().density;
methodChannel = new MethodChannel(binaryMessenger, "plugins.flutter.io/google_maps_" + id);
methodChannel.setMethodCallHandler(this);
mApplication = application;
this.lifecycle = lifecycle;
this.registrar = registrar;
this.lifecycleProvider = lifecycleProvider;
this.markersController = new MarkersController(methodChannel);
this.polygonsController = new PolygonsController(methodChannel, density);
this.polylinesController = new PolylinesController(methodChannel, density);
Expand All @@ -119,30 +106,8 @@ public View getView() {
return mapView;
}

void init(State lifecycleState) {
switch (lifecycleState) {
case RESUMED:
mapView.onCreate(null);
mapView.onStart();
mapView.onResume();
break;
case STARTED:
mapView.onCreate(null);
mapView.onStart();
break;
case CREATED:
mapView.onCreate(null);
break;
case DESTROYED:
case INITIALIZED:
// Nothing to do, the activity has been completely destroyed or not yet created.
break;
}
if (lifecycle != null) {
lifecycle.addObserver(this);
} else {
getApplication().registerActivityLifecycleCallbacks(this);
}
void init() {
lifecycleProvider.getLifecycle().addObserver(this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure so double checking - will we get the initial notification for the current state?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, from addObserver's documentation:

The given observer will be brought to the current state of the LifecycleOwner. For example, if the LifecycleOwner is in State#STARTED state, the given observer will receive Event#ON_CREATE, Event#ON_START events

mapView.getMapAsync(this);
}

Expand Down Expand Up @@ -507,7 +472,10 @@ public void dispose() {
methodChannel.setMethodCallHandler(null);
setGoogleMapListener(null);
destroyMapViewIfNecessary();
getApplication().unregisterActivityLifecycleCallbacks(this);
Lifecycle lifecycle = lifecycleProvider.getLifecycle();
if (lifecycle != null) {
lifecycle.removeObserver(this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not sure) do we need to protect against calling removeObserver twice? (here and from onDestroy)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, operation is idempotent

}
}

private void setGoogleMapListener(@Nullable GoogleMapListener listener) {
Expand Down Expand Up @@ -537,64 +505,7 @@ 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
public void onActivityCreated(Activity activity, Bundle savedInstanceState) {
if (disposed || activity.hashCode() != getActivityHashCode()) {
return;
}
mapView.onCreate(savedInstanceState);
}

@Override
public void onActivityStarted(Activity activity) {
if (disposed || activity.hashCode() != getActivityHashCode()) {
return;
}
mapView.onStart();
}

@Override
public void onActivityResumed(Activity activity) {
if (disposed || activity.hashCode() != getActivityHashCode()) {
return;
}
mapView.onResume();
}

@Override
public void onActivityPaused(Activity activity) {
if (disposed || activity.hashCode() != getActivityHashCode()) {
return;
}
mapView.onPause();
}

@Override
public void onActivityStopped(Activity activity) {
if (disposed || activity.hashCode() != getActivityHashCode()) {
return;
}
mapView.onStop();
}

@Override
public void onActivitySaveInstanceState(Activity activity, Bundle outState) {
if (disposed || activity.hashCode() != getActivityHashCode()) {
return;
}
mapView.onSaveInstanceState(outState);
}

@Override
public void onActivityDestroyed(Activity activity) {
if (disposed || activity.hashCode() != getActivityHashCode()) {
return;
}
destroyMapViewIfNecessary();
}

// DefaultLifecycleObserver and OnSaveInstanceStateListener
// DefaultLifecycleObserver

@Override
public void onCreate(@NonNull LifecycleOwner owner) {
Expand Down Expand Up @@ -638,6 +549,7 @@ public void onStop(@NonNull LifecycleOwner owner) {

@Override
public void onDestroy(@NonNull LifecycleOwner owner) {
owner.getLifecycle().removeObserver(this);
if (disposed) {
return;
}
Expand Down Expand Up @@ -848,24 +760,6 @@ private int checkSelfPermission(String permission) {
permission, android.os.Process.myPid(), android.os.Process.myUid());
}

private int getActivityHashCode() {
if (registrar != null && registrar.activity() != null) {
return registrar.activity().hashCode();
} else {
// 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;
}
}

private Application getApplication() {
if (registrar != null && registrar.activity() != null) {
return registrar.activity().getApplication();
} else {
return mApplication;
}
}

private void destroyMapViewIfNecessary() {
if (mapView == null) {
return;
Expand Down
26 changes: 4 additions & 22 deletions ...le_maps_flutter/android/src/main/java/io/flutter/plugins/googlemaps/GoogleMapFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,23 @@

package io.flutter.plugins.googlemaps;

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.AtomicReference;

public class GoogleMapFactory extends PlatformViewFactory {

private final AtomicReference<State> lifecycleState;
private final BinaryMessenger binaryMessenger;
@Nullable private final Application application;
@Nullable private final Lifecycle lifecycle;
@Nullable private final PluginRegistry.Registrar registrar; // V1 embedding only.
private final LifecycleProvider lifecycleProvider;

GoogleMapFactory(
AtomicReference<State> lifecycleState,
BinaryMessenger binaryMessenger,
@Nullable Application application,
@Nullable Lifecycle lifecycle,
@Nullable PluginRegistry.Registrar registrar) {
GoogleMapFactory(BinaryMessenger binaryMessenger, LifecycleProvider lifecycleProvider) {
super(StandardMessageCodec.INSTANCE);
this.lifecycleState = lifecycleState;
this.binaryMessenger = binaryMessenger;
this.application = application;
this.lifecycle = lifecycle;
this.registrar = registrar;
this.lifecycleProvider = lifecycleProvider;
}

@SuppressWarnings("unchecked")
Expand All @@ -63,7 +46,6 @@ public PlatformView create(Context context, int id, Object args) {
if (params.containsKey("circlesToAdd")) {
builder.setInitialCircles(params.get("circlesToAdd"));
}
return builder.build(
id, context, lifecycleState.get(), binaryMessenger, application, lifecycle, registrar);
return builder.build(id, context, binaryMessenger, lifecycleProvider);
}
}
Loading