Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing call to removeActivityResultListener #156

Closed
fmatosqg opened this issue May 5, 2020 · 8 comments
Closed

Missing call to removeActivityResultListener #156

fmatosqg opened this issue May 5, 2020 · 8 comments
Labels
bug Something isn't working

Comments

@fmatosqg
Copy link
Contributor

fmatosqg commented May 5, 2020

I believe onDetachedFromActivity and onDetachedFromActivityForConfigChanges should call removeActivityResultListener as shown in this official flutter plugin: https://github.com/flutter/plugins/blob/master/packages/image_picker/image_picker/android/src/main/java/io/flutter/plugins/imagepicker/ImagePickerPlugin.java

I believe this may be behind a crash I was facing in an app I'm developing, but need further investigation to find out if/how they're related.

My successfull flow is like following:

  • pick image from gallery or take a new pic with camera
  • send it to image cropper

My crash happens if:

  • I open my app
  • from a specific button, I open another app. Then I click something in there that triggers a deeplink back to my 1st app
  • second app seems to work as usual
  • pick image from gallery or take pic with camera
  • try to open image in image cropper
  • crash
@hnvn hnvn added the bug Something isn't working label May 5, 2020
@fmatosqg
Copy link
Contributor Author

fmatosqg commented May 5, 2020

@hnvn I can send a PR for this and retest my app. Or else let me know if you're already started working on it, and I'll wait for the commit for my test.

@hnvn
Copy link
Owner

hnvn commented May 5, 2020

You're welcome to PR

@mc10sw
Copy link

mc10sw commented May 6, 2020

@fmatosqg @hnvn I solved problem with your link.
I just copy and paste the code that the LifeCycleObserver class and the override methods from 'image_picker' plug-in.
I don't know about flutter plug-in well but, i've just checked it's working.
so, please check this code is collect or not. thank you

+@
you need to import the dependency of 'flutter_plugin_android_lifecycle: ^1.0.7'
in your plug-in to use FlutterLifecycleAdapter class.

package vn.hunghd.flutter.plugins.imagecropper;

import android.util.Log;
import android.app.Activity;
import android.app.Application;
import android.os.Bundle;
import android.os.Environment;
import android.os.Handler;
import android.os.Looper;
import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;
import androidx.lifecycle.DefaultLifecycleObserver;
import androidx.lifecycle.Lifecycle;
import androidx.lifecycle.LifecycleOwner;
import io.flutter.embedding.engine.plugins.FlutterPlugin;
import io.flutter.embedding.engine.plugins.activity.ActivityAware;
import io.flutter.embedding.engine.plugins.activity.ActivityPluginBinding;
import io.flutter.embedding.engine.plugins.lifecycle.FlutterLifecycleAdapter;
import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;
import io.flutter.plugin.common.BinaryMessenger;
import io.flutter.plugin.common.MethodCall;
import io.flutter.plugin.common.MethodChannel;
import io.flutter.plugin.common.MethodChannel.MethodCallHandler;
import io.flutter.plugin.common.MethodChannel.Result;
import io.flutter.plugin.common.PluginRegistry;
import io.flutter.plugin.common.PluginRegistry.Registrar;

import androidx.appcompat.app.AppCompatDelegate;

/** ImageCropperPlugin */
public class ImageCropperPlugin implements MethodCallHandler, FlutterPlugin, ActivityAware {

private class LifeCycleObserver
implements Application.ActivityLifecycleCallbacks, DefaultLifecycleObserver {
private final Activity thisActivity;

LifeCycleObserver(Activity activity) {
  this.thisActivity = activity;
}

@Override
public void onCreate(@NonNull LifecycleOwner owner) {}

@Override
public void onStart(@NonNull LifecycleOwner owner) {}

@Override
public void onResume(@NonNull LifecycleOwner owner) {}

@Override
public void onPause(@NonNull LifecycleOwner owner) {}

@Override
public void onStop(@NonNull LifecycleOwner owner) {
  onActivityStopped(thisActivity);
}

@Override
public void onDestroy(@NonNull LifecycleOwner owner) {
  onActivityDestroyed(thisActivity);
}

@Override
public void onActivityCreated(Activity activity, Bundle savedInstanceState) {}

@Override
public void onActivityStarted(Activity activity) {}

@Override
public void onActivityResumed(Activity activity) {}

@Override
public void onActivityPaused(Activity activity) {}

@Override
public void onActivitySaveInstanceState(Activity activity, Bundle outState) {}

@Override
public void onActivityDestroyed(Activity activity) {
  if (thisActivity == activity && activity.getApplicationContext() != null) {
    ((Application) activity.getApplicationContext())
            .unregisterActivityLifecycleCallbacks(
                    this); // Use getApplicationContext() to avoid casting failures
  }
}

@Override
public void onActivityStopped(Activity activity) {
  if (thisActivity == activity) {
    //delegate.saveStateBeforeResult();
  }
}

}

static
{
AppCompatDelegate.setCompatVectorFromResourcesEnabled(true);
}

private static final String CHANNEL = "plugins.hunghd.vn/image_cropper";

// private static ImageCropperPlugin instance;
// private static MethodChannel channel;
// private static ImageCropperDelegate delegate;
// private ActivityPluginBinding activityBinding;
// private Activity activity;

private MethodChannel channel;
private ImageCropperDelegate delegate;
private FlutterPluginBinding pluginBinding;
private ActivityPluginBinding activityBinding;
private Application application;
private Activity activity;
// This is null when not using v2 embedding;
private Lifecycle lifecycle;
private LifeCycleObserver observer;

private final Object initializationLock = new Object();

/** Plugin registration. */
public static void registerWith(Registrar registrar) {
if (registrar.activity() == null) {
return;
}
Activity activity = registrar.activity();
Application application = null;
if (registrar.context() != null) {
application = (Application) (registrar.context().getApplicationContext());
}
ImageCropperPlugin plugin = new ImageCropperPlugin();
plugin.setup(registrar.messenger(), application, activity, registrar, null);
}

@OverRide
public void onMethodCall(MethodCall call, Result result) {
if (activity == null || delegate == null) {
result.error("no_activity", "image_cropper plugin requires a foreground activity.", null);
return;
}
if (call.method.equals("cropImage")) {
delegate.startCrop(call, result);
}
}

public ImageCropperPlugin() {}

@VisibleForTesting
ImageCropperPlugin(final ImageCropperDelegate delegate, final Activity activity) {
this.delegate = delegate;
this.activity = activity;
}

@OverRide
public void onAttachedToEngine(FlutterPluginBinding binding) {
pluginBinding = binding;
}

@OverRide
public void onDetachedFromEngine(FlutterPluginBinding binding) {
pluginBinding = null;
}

@OverRide
public void onAttachedToActivity(ActivityPluginBinding binding) {
activityBinding = binding;
setup(
pluginBinding.getBinaryMessenger(),
(Application) pluginBinding.getApplicationContext(),
activityBinding.getActivity(),
null,
activityBinding);
}

@OverRide
public void onDetachedFromActivity() {
tearDown();
}

@OverRide
public void onDetachedFromActivityForConfigChanges() {
onDetachedFromActivity();
}

@OverRide
public void onReattachedToActivityForConfigChanges(ActivityPluginBinding binding) {
onAttachedToActivity(binding);
}

private void setup(
final BinaryMessenger messenger,
final Application application,
final Activity activity,
final PluginRegistry.Registrar registrar,
final ActivityPluginBinding activityBinding) {
this.activity = activity;
this.application = application;
this.delegate = constructDelegate(activity);
channel = new MethodChannel(messenger, CHANNEL);
channel.setMethodCallHandler(this);
observer = new LifeCycleObserver(activity);
if (registrar != null) {
// V1 embedding setup for activity listeners.
application.registerActivityLifecycleCallbacks(observer);
registrar.addActivityResultListener(delegate);
registrar.addRequestPermissionsResultListener(delegate);
} else {
// V2 embedding setup for activity listeners.
activityBinding.addActivityResultListener(delegate);
activityBinding.addRequestPermissionsResultListener(delegate);
lifecycle = FlutterLifecycleAdapter.getActivityLifecycle(activityBinding);
lifecycle.addObserver(observer);
}
}

private void tearDown() {
activityBinding.removeActivityResultListener(delegate);
activityBinding.removeRequestPermissionsResultListener(delegate);
activityBinding = null;
lifecycle.removeObserver(observer);
lifecycle = null;
delegate = null;
channel.setMethodCallHandler(null);
channel = null;
application.unregisterActivityLifecycleCallbacks(observer);
application = null;
}

private final ImageCropperDelegate constructDelegate(final Activity setupActivity) {
return new ImageCropperDelegate(setupActivity);
}
}

@fmatosqg
Copy link
Contributor Author

fmatosqg commented May 12, 2020

@mc10sw I'm sorry I can't read such long code in a github comment. Pls consider making a PR so we can diff your changes and understand them better using a proper editor. That said, at first glance it looks pretty much like another plugin I was reading for tracking down my problems, but I think in this case we can get away without using a lifecycle and observer since we have a simple use case.

@hnvn I cleaned the whole file and have an implementation that works with my project. Seems like the problem was with the static instance which should get updated every once in a while, that's why I got a bad method call: it was going somehow to an instance that was no longer valid. But my understanding of plugins is quite superficial, I can be wrong. It's still a messy code that works, I hope I don't find more surprises.

Do you have any objections against kotlin? My suggestion would be to rewrite ImageCropperPlugin.java in kotlin, however the code is short enough that I'm not sure it would justify my effort to set up kotlin - which is usually not big either.

@hnvn
Copy link
Owner

hnvn commented May 12, 2020

Thanks @fmatosqg , I don't have any objections against kotlin. I choose Java for this plugin in the first place because I feel more confident with my Java experience rather than to kotlin. You are welcome to refactor the plugin with kotlin.

@fmatosqg
Copy link
Contributor Author

fmatosqg commented Jun 8, 2020

hey @hnvn would you have a look at #167? It's the refactor to solve the problem that started my investigation. As you'll see there, it deals with the original problems related to static objects that wouldn't get renewed at the right time.

@hnvn
Copy link
Owner

hnvn commented Jun 8, 2020

Ah, Sorry @fmatosqg , I miss your PR. I will have a look at it.

@hnvn
Copy link
Owner

hnvn commented Jun 8, 2020

Merged and released as v1.2.3

@hnvn hnvn closed this as completed Jun 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants