Improve cleanup of the FlutterView's Mojo service providers (#2644)

sky/services/firebase/src/org/domokit/firebase/FirebaseImpl.java

@@ -22,6 +22,7 @@ import java.util.Map;
 import java.util.HashMap;
 
 import org.chromium.mojo.bindings.ConnectionErrorHandler;
+import org.chromium.mojo.bindings.Interface.Binding;
 import org.chromium.mojo.bindings.InterfaceRequest;
 import org.chromium.mojo.system.Core;
 import org.chromium.mojo.system.MessagePipeHandle;
@@ -46,8 +47,8 @@ public class FirebaseImpl implements org.chromium.mojom.firebase.Firebase {
         mContext = context;
     }
 
-    public static void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-        Firebase.MANAGER.bind(new FirebaseImpl(context), pipe);
+    public static Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
+        return Firebase.MANAGER.bind(new FirebaseImpl(context), pipe);
     }
 
     @Override

sky/shell/platform/android/io/flutter/view/FlutterMain.java

@@ -27,6 +27,7 @@ import org.chromium.base.PathUtils;
 import org.chromium.base.library_loader.LibraryLoader;
 import org.chromium.base.library_loader.LibraryProcessType;
 import org.chromium.base.library_loader.ProcessInitException;
+import org.chromium.mojo.bindings.Interface.Binding;
 import org.chromium.mojo.sensors.SensorServiceImpl;
 import org.chromium.mojo.system.Core;
 import org.chromium.mojo.system.impl.CoreImpl;
@@ -140,64 +141,64 @@ public class FlutterMain {
 
         registry.register(Activity.MANAGER.getName(), new ServiceFactory() {
             @Override
-            public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-                Activity.MANAGER.bind(new ActivityImpl(), pipe);
+            public Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
+                return Activity.MANAGER.bind(new ActivityImpl(), pipe);
             }
         });
 
         registry.register(MediaService.MANAGER.getName(), new ServiceFactory() {
             @Override
-            public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-                MediaService.MANAGER.bind(new MediaServiceImpl(context, core), pipe);
+            public Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
+                return MediaService.MANAGER.bind(new MediaServiceImpl(context, core), pipe);
             }
         });
 
         registry.register(NetworkService.MANAGER.getName(), new ServiceFactory() {
             @Override
-            public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-                NetworkService.MANAGER.bind(new NetworkServiceImpl(context, core), pipe);
+            public Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
+                return NetworkService.MANAGER.bind(new NetworkServiceImpl(context, core), pipe);
             }
         });
 
         registry.register(SensorService.MANAGER.getName(), new ServiceFactory() {
             @Override
-            public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-                SensorService.MANAGER.bind(new SensorServiceImpl(context), pipe);
+            public Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
+                return SensorService.MANAGER.bind(new SensorServiceImpl(context), pipe);
             }
         });
 
         registry.register(VSyncProvider.MANAGER.getName(), new ServiceFactory() {
             @Override
-            public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-                VSyncProvider.MANAGER.bind(new VSyncProviderImpl(pipe), pipe);
+            public Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
+                return VSyncProvider.MANAGER.bind(new VSyncProviderImpl(pipe), pipe);
             }
         });
 
         registry.register(HapticFeedback.MANAGER.getName(), new ServiceFactory() {
             @Override
-            public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-                HapticFeedback.MANAGER.bind(new HapticFeedbackImpl((android.app.Activity) context), pipe);
+            public Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
+                return HapticFeedback.MANAGER.bind(new HapticFeedbackImpl((android.app.Activity) context), pipe);
             }
         });
 
         registry.register(PathProvider.MANAGER.getName(), new ServiceFactory() {
             @Override
-            public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-                PathProvider.MANAGER.bind(new PathProviderImpl(context), pipe);
+            public Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
+                return PathProvider.MANAGER.bind(new PathProviderImpl(context), pipe);
             }
         });
 
         registry.register(SystemChrome.MANAGER.getName(), new ServiceFactory() {
             @Override
-            public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-                SystemChrome.MANAGER.bind(new SystemChromeImpl((android.app.Activity) context), pipe);
+            public Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
+                return SystemChrome.MANAGER.bind(new SystemChromeImpl((android.app.Activity) context), pipe);
             }
         });
 
         registry.register(SystemSound.MANAGER.getName(), new ServiceFactory() {
             @Override
-            public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-                SystemSound.MANAGER.bind(new SystemSoundImpl((android.app.Activity) context), pipe);
+            public Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
+                return SystemSound.MANAGER.bind(new SystemSoundImpl((android.app.Activity) context), pipe);
             }
         });
     }
@@ -239,13 +240,14 @@ public class FlutterMain {
     private static void registerService(ServiceRegistry registry, final String serviceName, final String className) {
         registry.register(serviceName, new ServiceFactory() {
             @Override
-            public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
+            public Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
                 try {
-                    Class.forName(className)
+                    return (Binding) Class.forName(className)
                         .getMethod("connectToService", Context.class, Core.class, MessagePipeHandle.class)
                         .invoke(null, context, core, pipe);
                 } catch(Exception e) {
                     Log.e(TAG, "Failed to register service '" + serviceName + "'", e);
+                    throw new RuntimeException(e);
                 }
             }
         });

sky/shell/platform/android/io/flutter/view/FlutterView.java

@@ -26,6 +26,7 @@ import android.view.inputmethod.EditorInfo;
 import android.view.inputmethod.InputConnection;
 
 import org.chromium.base.JNINamespace;
+import org.chromium.mojo.bindings.Interface.Binding;
 import org.chromium.mojo.bindings.InterfaceRequest;
 import org.chromium.mojo.system.Core;
 import org.chromium.mojo.system.MessagePipeHandle;
@@ -70,7 +71,9 @@ public class FlutterView extends SurfaceView
     private long mNativePlatformView;
     private SkyEngine.Proxy mSkyEngine;
     private ServiceProviderImpl mPlatformServiceProvider;
+    private Binding mPlatformServiceProviderBinding;
     private ServiceProviderImpl mViewServiceProvider;
+    private Binding mViewServiceProviderBinding;
     private ServiceProvider.Proxy mDartServiceProvider;
     private ApplicationMessages.Proxy mFlutterAppMessages;
     private HashMap<String, OnMessageListener> mOnMessageListeners;
@@ -190,6 +193,16 @@ public class FlutterView extends SurfaceView
     }
 
     public void destroy() {
+        if (mPlatformServiceProviderBinding != null) {
+            mPlatformServiceProviderBinding.unbind().close();
+            mPlatformServiceProvider.unbindServices();
+        }
+
+        if (mViewServiceProviderBinding != null) {
+            mViewServiceProviderBinding.unbind().close();
+            mViewServiceProvider.unbindServices();
+        }
+
         getHolder().removeCallback(mSurfaceCallback);
         nativeDetach(mNativePlatformView);
         mNativePlatformView = 0;
@@ -337,22 +350,22 @@ public class FlutterView extends SurfaceView
     private void configureLocalServices(ServiceRegistry registry) {
         registry.register(Keyboard.MANAGER.getName(), new ServiceFactory() {
             @Override
-            public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-                Keyboard.MANAGER.bind(new KeyboardImpl(context, mKeyboardState), pipe);
+            public Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
+                return Keyboard.MANAGER.bind(new KeyboardImpl(context, mKeyboardState), pipe);
             }
         });
 
         registry.register(RawKeyboardService.MANAGER.getName(), new ServiceFactory() {
             @Override
-            public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-                RawKeyboardService.MANAGER.bind(new RawKeyboardServiceImpl(mRawKeyboardState), pipe);
+            public Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
+                return RawKeyboardService.MANAGER.bind(new RawKeyboardServiceImpl(mRawKeyboardState), pipe);
             }
         });
 
         registry.register(ApplicationMessages.MANAGER.getName(), new ServiceFactory() {
             @Override
-            public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-                ApplicationMessages.MANAGER.bind(new ApplicationMessagesImpl(), pipe);
+            public Binding connectToService(Context context, Core core, MessagePipeHandle pipe) {
+                return ApplicationMessages.MANAGER.bind(new ApplicationMessagesImpl(), pipe);
             }
         });
     }
@@ -365,16 +378,14 @@ public class FlutterView extends SurfaceView
         mNativePlatformView = nativeAttach(engine.second.passHandle().releaseNativeHandle());
     }
 
-    private ServiceProvider.Proxy getServiceProviderProxy(Core core, ServiceProviderImpl impl) {
-        Pair<ServiceProvider.Proxy, InterfaceRequest<ServiceProvider>> serviceProvider =
-                ServiceProvider.MANAGER.getInterfaceRequest(core);
-        ServiceProvider.MANAGER.bind(impl, serviceProvider.second);
-        return serviceProvider.first;
-    }
-
     public void runFromBundle(String bundlePath, String snapshotPath) {
-        if (mPlatformServiceProvider != null) {
-            mPlatformServiceProvider.close();
+        if (mPlatformServiceProviderBinding != null) {
+            mPlatformServiceProviderBinding.unbind().close();
+            mPlatformServiceProvider.unbindServices();
+        }
+        if (mViewServiceProviderBinding != null) {
+            mViewServiceProviderBinding.unbind().close();
+            mViewServiceProvider.unbindServices();
         }
         if (mDartServiceProvider != null) {
             mDartServiceProvider.close();
@@ -386,10 +397,20 @@ public class FlutterView extends SurfaceView
                 ServiceProvider.MANAGER.getInterfaceRequest(core);
         mDartServiceProvider = dartServiceProvider.first;
 
+        Pair<ServiceProvider.Proxy, InterfaceRequest<ServiceProvider>> platformServiceProvider =
+                ServiceProvider.MANAGER.getInterfaceRequest(core);
+        mPlatformServiceProviderBinding = ServiceProvider.MANAGER.bind(
+                mPlatformServiceProvider, platformServiceProvider.second);
+
+        Pair<ServiceProvider.Proxy, InterfaceRequest<ServiceProvider>> viewServiceProvider =
+                ServiceProvider.MANAGER.getInterfaceRequest(core);
+        mViewServiceProviderBinding = ServiceProvider.MANAGER.bind(
+                mViewServiceProvider, viewServiceProvider.second);
+
         ServicesData services = new ServicesData();
-        services.incomingServices = getServiceProviderProxy(core, mPlatformServiceProvider);
+        services.incomingServices = platformServiceProvider.first;
         services.outgoingServices = dartServiceProvider.second;
-        services.viewServices = getServiceProviderProxy(core, mViewServiceProvider);
+        services.viewServices = viewServiceProvider.first;
         mSkyEngine.setServices(services);
 
         resetAccessibilityTree();

sky/shell/platform/android/io/flutter/view/ServiceFactory.java

@@ -6,6 +6,7 @@ package io.flutter.view;
 
 import android.content.Context;
 
+import org.chromium.mojo.bindings.Interface.Binding;
 import org.chromium.mojo.system.Core;
 import org.chromium.mojo.system.MessagePipeHandle;
 
@@ -15,5 +16,5 @@ import org.chromium.mojo.system.MessagePipeHandle;
  * clients.
  **/
 interface ServiceFactory {
-    void connectToService(Context context, Core core, MessagePipeHandle pipe);
+    Binding connectToService(Context context, Core core, MessagePipeHandle pipe);
 }

sky/shell/platform/android/io/flutter/view/ServiceProviderImpl.java

@@ -5,7 +5,11 @@
 package io.flutter.view;
 
 import android.content.Context;
+import android.util.Log;
+import java.util.HashSet;
 
+import org.chromium.mojo.bindings.ConnectionErrorHandler;
+import org.chromium.mojo.bindings.Interface.Binding;
 import org.chromium.mojo.system.Core;
 import org.chromium.mojo.system.MessagePipeHandle;
 import org.chromium.mojo.system.MojoException;
@@ -16,9 +20,12 @@ import org.chromium.mojom.mojo.ServiceProvider;
  * A collection of services.
  **/
 class ServiceProviderImpl implements ServiceProvider {
+    private static final String TAG = "ServiceProviderImpl";
+
     private Core mCore;
     private Context mContext;
     private ServiceRegistry mRegistry;
+    private HashSet<Binding> mBindings = new HashSet<Binding>();
 
     ServiceProviderImpl(Core core, Context context, ServiceRegistry registry) {
         assert core != null;
@@ -41,6 +48,21 @@ class ServiceProviderImpl implements ServiceProvider {
             pipe.close();
             return;
         }
-        factory.connectToService(mContext, mCore, pipe);
+        final Binding binding = factory.connectToService(mContext, mCore, pipe);
+        mBindings.add(binding);
+        binding.registerErrorHandler(new ConnectionErrorHandler() {
+            @Override
+            public void onConnectionError(MojoException e) {
+                Log.w(TAG, "Flutter service provider connection error", e);
+                mBindings.remove(binding);
+            }
+        });
+    }
+
+    public void unbindServices() {
+        for (Binding binding : mBindings) {
+            binding.unbind().close();
+        }
+        mBindings.clear();
     }
 }