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

[google_maps_flutter] Objective-C code clean up #5780

Merged
fluttergithubbot merged 8 commits into from
Jun 1, 2022
Merged

[google_maps_flutter] Objective-C code clean up #5780

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b376693
draft
Apr 26, 2022
fcd3c69
update version
May 24, 2022
367466b
fix crashes
May 24, 2022
fb856e6
review
May 25, 2022
459555d
method channels to strong
May 27, 2022
f23fcbe
renaming
May 31, 2022
fccc746
Merge branch 'google_map_clean_up' of github.com:cyanglaz/plugins int…
May 31, 2022
da044f1
format
May 31, 2022
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
Prev Previous commit
Next Next commit
method channels to strong
  • Loading branch information
Chris Yang committed May 27, 2022
commit 459555d5a8d832d946c374a9c69777979a4ae3ed
4 changes: 2 additions & 2 deletions ...s/google_maps_flutter/google_maps_flutter/ios/Classes/FLTGoogleMapTileOverlayController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ - (void)interpretTileOverlayOptions:(NSDictionary *)data {

@interface FLTTileProviderController ()

@property(weak, nonatomic) FlutterMethodChannel *methodChannel;
Copy link
Member

@jmagman jmagman May 27, 2022

Choose a reason for hiding this comment

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

Do you remember why you made this weak in the first place?
https://github.com/flutter/plugins/pull/3434/files#diff-7013871fe3ee02101b28aad71693b5ef3d5f48414c744ddef944e57fe886c5f4R100

Copy link
Contributor Author

@cyanglaz cyanglaz May 27, 2022

Choose a reason for hiding this comment

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

The same reason I made all method channels to be weak in this PR. The owner of the methodchannel is FLTGoogleMapController. And FLTGoogleMapController owns and out lives all the sub controllers, so I thought it is not necessary for the sub controllers to strongly own methodChannel.

Copy link
Member

@jmagman jmagman May 31, 2022

Choose a reason for hiding this comment

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

Then did you mean to make it strong in this PR?

Copy link
Contributor Author

@cyanglaz cyanglaz May 31, 2022

Choose a reason for hiding this comment

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

Yep, to match other controllers.

Copy link
Member

@jmagman jmagman May 31, 2022

Choose a reason for hiding this comment

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

So should they all be weak as you had it before? I'm not trying to argue one way or the other.

Copy link
Contributor Author

@cyanglaz cyanglaz May 31, 2022

Choose a reason for hiding this comment

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

I think Strong is better. There won't be a retain cycle as method channel is not going to have a strong reference to the controllers.
Having them as weak makes the methodChannel's life cycle confusing. One needs to figure out who strongly owns the methodChannel (GoogleMapsController) to understand its life cycle.

@property(strong, nonatomic) FlutterMethodChannel *methodChannel;

@end

Expand Down Expand Up @@ -160,7 +160,7 @@ - (void)requestTileForX:(NSUInteger)x
@interface FLTTileOverlaysController ()

@property(strong, nonatomic) NSMutableDictionary *tileOverlayIdentifierToController;
@property(weak, nonatomic) FlutterMethodChannel *methodChannel;
@property(strong, nonatomic) FlutterMethodChannel *methodChannel;
@property(weak, nonatomic) GMSMapView *mapView;

@end
Expand Down
2 changes: 1 addition & 1 deletion packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapCircleController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ - (void)interpretCircleOptions:(NSDictionary *)data {

@interface FLTCirclesController ()

@property(weak, nonatomic) FlutterMethodChannel *methodChannel;
@property(strong, nonatomic) FlutterMethodChannel *methodChannel;
@property(weak, nonatomic) GMSMapView *mapView;
@property(strong, nonatomic) NSMutableDictionary *circleIdToController;

Expand Down
2 changes: 1 addition & 1 deletion packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapMarkerController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ - (UIImage *)scaleImage:(UIImage *)image by:(id)scaleParam {
@interface FLTMarkersController ()

@property(strong, nonatomic) NSMutableDictionary *markerIdentifierToController;
@property(weak, nonatomic) FlutterMethodChannel *methodChannel;
@property(strong, nonatomic) FlutterMethodChannel *methodChannel;
@property(weak, nonatomic) NSObject<FlutterPluginRegistrar> *registrar;
@property(weak, nonatomic) GMSMapView *mapView;

Expand Down
2 changes: 1 addition & 1 deletion packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapPolygonController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ - (void)interpretPolygonOptions:(NSDictionary *)data
@interface FLTPolygonsController ()

@property(strong, nonatomic) NSMutableDictionary *polygonIdentifierToController;
@property(weak, nonatomic) FlutterMethodChannel *methodChannel;
@property(strong, nonatomic) FlutterMethodChannel *methodChannel;
@property(weak, nonatomic) NSObject<FlutterPluginRegistrar> *registrar;
@property(weak, nonatomic) GMSMapView *mapView;

Expand Down
2 changes: 1 addition & 1 deletion packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapPolylineController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ - (void)interpretPolylineOptions:(NSDictionary *)data
@interface FLTPolylinesController ()

@property(strong, nonatomic) NSMutableDictionary *polylineIdentifierToController;
@property(weak, nonatomic) FlutterMethodChannel *methodChannel;
@property(strong, nonatomic) FlutterMethodChannel *methodChannel;
@property(weak, nonatomic) NSObject<FlutterPluginRegistrar> *registrar;
@property(weak, nonatomic) GMSMapView *mapView;

Expand Down