Android: proper shutdown mechanism

7 files changed, 145 insertions, 17 deletions

diff --git a/ports/libsimpleservo/src/api.rs b/ports/libsimpleservo/src/api.rs
index f946b0681cc..8b63aca4169 100644
--- a/ports/libsimpleservo/src/api.rs
+++ b/ports/libsimpleservo/src/api.rs
@@ -77,6 +77,8 @@ pub trait HostTrait {
     /// has events for Servo, or Servo has woken up the embedder event loop via
     /// EventLoopWaker).
     fn on_animating_changed(&self, animating: bool);
+    /// Servo finished shutting down.
+    fn on_shutdown_complete(&self);
 }
 
 pub struct ServoGlue {
@@ -169,6 +171,12 @@ pub fn init(
     Ok(())
 }
 
+pub fn deinit() {
+    SERVO.with(|s| {
+        s.replace(None).unwrap().deinit()
+    });
+}
+
 impl ServoGlue {
     fn get_browser_id(&self) -> Result<BrowserId, &'static str> {
         let browser_id = match self.browser_id {
@@ -177,6 +185,17 @@ impl ServoGlue {
         };
         Ok(browser_id)
     }
+
+    /// Request shutdown. Will call on_shutdown_complete.
+    pub fn request_shutdown(&mut self) -> Result<(), &'static str> {
+        self.process_event(WindowEvent::Quit)
+    }
+
+    /// Call after on_shutdown_complete
+    pub fn deinit(self) {
+        self.servo.deinit();
+    }
+
     /// This is the Servo heartbeat. This needs to be called
     /// everytime wakeup is called or when embedder wants Servo
     /// to act on its pending events.
@@ -404,6 +423,9 @@ impl ServoGlue {
                         self.events.push(WindowEvent::Quit);
                     }
                 },
+                EmbedderMsg::Shutdown => {
+                    self.callbacks.host_callbacks.on_shutdown_complete();
+                },
                 EmbedderMsg::Status(..) |
                 EmbedderMsg::SelectFiles(..) |
                 EmbedderMsg::MoveTo(..) |
@@ -415,7 +437,6 @@ impl ServoGlue {
                 EmbedderMsg::SetFullscreenState(..) |
                 EmbedderMsg::ShowIME(..) |
                 EmbedderMsg::HideIME |
-                EmbedderMsg::Shutdown |
                 EmbedderMsg::Panic(..) => {},
             }
         }
diff --git a/ports/libsimpleservo/src/capi.rs b/ports/libsimpleservo/src/capi.rs
index 28c036cff56..7b6e401311d 100644
--- a/ports/libsimpleservo/src/capi.rs
+++ b/ports/libsimpleservo/src/capi.rs
@@ -35,6 +35,7 @@ pub struct CHostCallbacks {
     pub on_url_changed: extern fn(url: *const c_char),
     pub on_history_changed: extern fn(can_go_back: bool, can_go_forward: bool),
     pub on_animating_changed: extern fn(animating: bool),
+    pub on_shutdown_complete: extern fn(),
 }
 
 /// Servo options
@@ -109,6 +110,18 @@ pub extern "C" fn init_with_gl(
 }
 
 #[no_mangle]
+pub extern "C" fn deinit() {
+    debug!("deinit");
+    api::deinit();
+}
+
+#[no_mangle]
+pub extern "C" fn request_shutdown() {
+    debug!("request_shutdown");
+    call(|s| s.request_shutdown());
+}
+
+#[no_mangle]
 pub extern "C" fn set_batch_mode(batch: bool) {
     debug!("set_batch_mode");
     call(|s| s.set_batch_mode(batch));
@@ -296,4 +309,9 @@ impl HostTrait for HostCallbacks {
         debug!("on_animating_changed");
         (self.0.on_animating_changed)(animating);
     }
+
+    fn on_shutdown_complete(&self) {
+        debug!("on_shutdown_complete");
+        (self.0.on_shutdown_complete)();
+    }
 }
diff --git a/ports/libsimpleservo/src/jniapi.rs b/ports/libsimpleservo/src/jniapi.rs
index ad027ec946a..bb499cedf48 100644
--- a/ports/libsimpleservo/src/jniapi.rs
+++ b/ports/libsimpleservo/src/jniapi.rs
@@ -70,6 +70,8 @@ pub fn Java_org_mozilla_servoview_JNIServo_init(
             "script::dom::bindings::error",
             // Show GL errors by default.
             "canvas::webgl_thread",
+            "compositing::compositor",
+            "constellation::constellation",
         ];
         let mut filter = Filter::default().with_min_level(Level::Debug);
         for &module in &filters {
@@ -118,6 +120,18 @@ pub fn Java_org_mozilla_servoview_JNIServo_setBatchMode(
 }
 
 #[no_mangle]
+pub fn Java_org_mozilla_servoview_JNIServo_requestShutdown(env: JNIEnv, _class: JClass) {
+    debug!("requestShutdown");
+    call(&env, |s| s.request_shutdown());
+}
+
+#[no_mangle]
+pub fn Java_org_mozilla_servoview_JNIServo_deinit(_env: JNIEnv, _class: JClass) {
+    debug!("deinit");
+    api::deinit();
+}
+
+#[no_mangle]
 pub fn Java_org_mozilla_servoview_JNIServo_resize(
     env: JNIEnv,
     _: JClass,
@@ -357,6 +371,13 @@ impl HostTrait for HostCallbacks {
             .unwrap();
     }
 
+    fn on_shutdown_complete(&self) {
+        debug!("on_shutdown_complete");
+        let env = self.jvm.get_env().unwrap();
+        env.call_method(self.callbacks.as_obj(), "onShutdownComplete", "()V", &[])
+            .unwrap();
+    }
+
     fn on_title_changed(&self, title: String) {
         debug!("on_title_changed");
         let env = self.jvm.get_env().unwrap();
diff --git a/support/android/apk/servoview/src/main/java/org/mozilla/servoview/JNIServo.java b/support/android/apk/servoview/src/main/java/org/mozilla/servoview/JNIServo.java
index 004b91888a5..c500630d93b 100644
--- a/support/android/apk/servoview/src/main/java/org/mozilla/servoview/JNIServo.java
+++ b/support/android/apk/servoview/src/main/java/org/mozilla/servoview/JNIServo.java
@@ -22,6 +22,10 @@ public class JNIServo {
 
     public native void init(Activity activity, ServoOptions options, Callbacks callbacks);
 
+    public native void deinit();
+
+    public native void requestShutdown();
+
     public native void setBatchMode(boolean mode);
 
     public native void performUpdates();
@@ -84,6 +88,8 @@ public class JNIServo {
 
         void onHistoryChanged(boolean canGoBack, boolean canGoForward);
 
+        void onShutdownComplete();
+
         byte[] readfile(String file);
     }
 }
diff --git a/support/android/apk/servoview/src/main/java/org/mozilla/servoview/Servo.java b/support/android/apk/servoview/src/main/java/org/mozilla/servoview/Servo.java
index 2f708552218..9a707004c85 100644
--- a/support/android/apk/servoview/src/main/java/org/mozilla/servoview/Servo.java
+++ b/support/android/apk/servoview/src/main/java/org/mozilla/servoview/Servo.java
@@ -47,6 +47,14 @@ public class Servo {
         }
     }
 
+    public void requestShutdown() {
+        mRunCallback.inGLThread(() -> mJNI.requestShutdown());
+    }
+
+    public void deinit() {
+        mRunCallback.inGLThread(() -> mJNI.deinit());
+    }
+
     public String version() {
         return mJNI.version();
     }
@@ -137,6 +145,8 @@ public class Servo {
         void inGLThread(Runnable f);
 
         void inUIThread(Runnable f);
+
+        void finalizeShutdown();
     }
 
     public interface GfxCallbacks {
@@ -173,6 +183,10 @@ public class Servo {
             mGfxCb.makeCurrent();
         }
 
+        public void onShutdownComplete() {
+            mRunCallback.finalizeShutdown();
+        }
+
         public void onAnimatingChanged(boolean animating) {
             mRunCallback.inGLThread(() -> mGfxCb.animationStateChanged(animating));
         }
diff --git a/support/android/apk/servoview/src/main/java/org/mozilla/servoview/ServoSurface.java b/support/android/apk/servoview/src/main/java/org/mozilla/servoview/ServoSurface.java
index bd419472a0f..41918628f69 100644
--- a/support/android/apk/servoview/src/main/java/org/mozilla/servoview/ServoSurface.java
+++ b/support/android/apk/servoview/src/main/java/org/mozilla/servoview/ServoSurface.java
@@ -5,7 +5,6 @@
 
 package org.mozilla.servoview;
 
-import android.annotation.SuppressLint;
 import android.app.Activity;
 import android.net.Uri;
 import android.opengl.EGL14;
@@ -16,7 +15,6 @@ import android.opengl.EGLSurface;
 import android.opengl.GLUtils;
 import android.os.Handler;
 import android.os.Looper;
-import android.os.Message;
 import android.util.Log;
 import android.view.Surface;
 
@@ -26,6 +24,8 @@ import org.mozilla.servoview.Servo.GfxCallbacks;
 import org.mozilla.servoview.Servo.RunCallback;
 
 import static android.opengl.EGL14.EGL_CONTEXT_CLIENT_VERSION;
+import static android.opengl.EGL14.EGL_NO_CONTEXT;
+import static android.opengl.EGL14.EGL_NO_SURFACE;
 import static android.opengl.EGL14.EGL_OPENGL_ES2_BIT;
 
 public class ServoSurface {
@@ -68,6 +68,17 @@ public class ServoSurface {
         mGLThread.start();
     }
 
+    public void shutdown() {
+        Log.d(LOGTAG, "shutdown");
+        mServo.requestShutdown();
+        try {
+            Log.d(LOGTAG, "Waiting for GL thread to shutdown");
+            mGLThread.join();
+        } catch (InterruptedException e) {
+            e.printStackTrace();
+        }
+    }
+
     public void reload() {
         mServo.reload();
     }
@@ -121,12 +132,20 @@ public class ServoSurface {
         private EGLDisplay mEglDisplay;
         private EGLContext mEglContext;
         private EGLSurface mEglSurface;
+        
+        void throwGLError(String function) {
+            throwGLError(function, EGL14.eglGetError());
+        }
+
+        void throwGLError(String function, int error) {
+            throw new RuntimeException("Error: " + function + "() Failed " + GLUtils.getEGLErrorString(error));
+        }
 
         GLSurface(Surface surface) {
             mEglDisplay = EGL14.eglGetDisplay(EGL14.EGL_DEFAULT_DISPLAY);
             int[] version = new int[2];
             if (!EGL14.eglInitialize(mEglDisplay, version, 0, version, 1)) {
-                throw new RuntimeException("Error: eglInitialize() Failed " + GLUtils.getEGLErrorString(EGL14.eglGetError()));
+                throwGLError("eglInitialize");
             }
             mEGLConfigs = new EGLConfig[1];
             int[] configsCount = new int[1];
@@ -141,7 +160,7 @@ public class ServoSurface {
                     EGL14.EGL_NONE
             };
             if ((!EGL14.eglChooseConfig(mEglDisplay, configSpec, 0, mEGLConfigs, 0, 1, configsCount, 0)) || (configsCount[0] == 0)) {
-                throw new IllegalArgumentException("Error: eglChooseConfig() Failed " + GLUtils.getEGLErrorString(EGL14.eglGetError()));
+                throwGLError("eglChooseConfig");
             }
             if (mEGLConfigs[0] == null) {
                 throw new RuntimeException("Error: eglConfig() not Initialized");
@@ -150,7 +169,7 @@ public class ServoSurface {
             mEglContext = EGL14.eglCreateContext(mEglDisplay, mEGLConfigs[0], EGL14.EGL_NO_CONTEXT, attrib_list, 0);
             int glError = EGL14.eglGetError();
             if (glError != EGL14.EGL_SUCCESS) {
-                throw new RuntimeException("Error: eglCreateContext() Failed " + GLUtils.getEGLErrorString(glError));
+                throwGLError("eglCreateContext", glError);
             }
             mEglSurface = EGL14.eglCreateWindowSurface(mEglDisplay, mEGLConfigs[0], surface, new int[]{EGL14.EGL_NONE}, 0);
             if (mEglSurface == null || mEglSurface == EGL14.EGL_NO_SURFACE) {
@@ -159,7 +178,7 @@ public class ServoSurface {
                     Log.e(LOGTAG, "Error: createWindowSurface() Returned EGL_BAD_NATIVE_WINDOW.");
                     return;
                 }
-                throw new RuntimeException("Error: createWindowSurface() Failed " + GLUtils.getEGLErrorString(glError));
+                throwGLError("createWindowSurface", glError);
             }
 
             makeCurrent();
@@ -168,7 +187,7 @@ public class ServoSurface {
 
         public void makeCurrent() {
             if (!EGL14.eglMakeCurrent(mEglDisplay, mEglSurface, mEglSurface, mEglContext)) {
-                throw new RuntimeException("Error: eglMakeCurrent() Failed " + GLUtils.getEGLErrorString(EGL14.eglGetError()));
+                throwGLError("eglMakeCurrent");
             }
         }
 
@@ -180,9 +199,26 @@ public class ServoSurface {
             // FIXME
         }
 
+        void destroy() {
+            Log.d(LOGTAG, "Destroying surface");
+            if (!EGL14.eglMakeCurrent(mEglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT)) {
+                throwGLError("eglMakeCurrent");
+            }
+            if (!EGL14.eglDestroyContext(mEglDisplay, mEglContext)) {
+                throwGLError("eglDestroyContext");
+            }
+            if (!EGL14.eglDestroySurface(mEglDisplay, mEglSurface)) {
+                throwGLError("eglDestroySurface");
+            }
+            if (!EGL14.eglTerminate(mEglDisplay)) {
+                throwGLError("eglTerminate");
+            }
+        }
+
     }
 
     class GLThread extends Thread implements RunCallback {
+        private GLSurface mSurface;
 
         public void inGLThread(Runnable r) {
             mGLLooperHandler.post(r);
@@ -192,17 +228,19 @@ public class ServoSurface {
             mMainLooperHandler.post(r);
         }
 
-        // FIXME: HandlerLeak
-        @SuppressLint("HandlerLeak")
+        public void finalizeShutdown() {
+            Log.d(LOGTAG, "finalizeShutdown");
+            mServo.deinit();
+            mSurface.destroy();
+            mGLLooperHandler.getLooper().quitSafely();
+        }
+
         public void run() {
             Looper.prepare();
 
-            GLSurface surface = new GLSurface(mASurface);
+            mSurface = new GLSurface(mASurface);
 
-            mGLLooperHandler = new Handler() {
-                public void handleMessage(Message msg) {
-                }
-            };
+            mGLLooperHandler = new Handler();
 
             inUIThread(() -> {
               ServoOptions options = new ServoOptions();
@@ -210,12 +248,12 @@ public class ServoSurface {
               options.width = mWidth;
               options.height = mHeight;
               options.density = 1;
-              options.url = mInitialUri == null ? null : mInitialUri;
+              options.url = mInitialUri;
               options.logStr = mServoLog;
               options.enableLogs = true;
               options.enableSubpixelTextAntialiasing = false;
 
-              mServo = new Servo(options, this, surface, mClient, mActivity);
+              mServo = new Servo(options, this, mSurface, mClient, mActivity);
             });
 
             Looper.loop();
diff --git a/support/android/apk/servoview/src/main/java/org/mozilla/servoview/ServoView.java b/support/android/apk/servoview/src/main/java/org/mozilla/servoview/ServoView.java
index fcf3f9f4c93..45ca4073c88 100644
--- a/support/android/apk/servoview/src/main/java/org/mozilla/servoview/ServoView.java
+++ b/support/android/apk/servoview/src/main/java/org/mozilla/servoview/ServoView.java
@@ -70,6 +70,11 @@ public class ServoView extends GLSurfaceView
         init(context);
     }
 
+    protected void onDetachedFromWindow() {
+        mServo.requestShutdown();
+        super.onDetachedFromWindow();
+    }
+
     private void init(Context context) {
         mActivity = (Activity) context;
         setFocusable(true);
@@ -136,6 +141,11 @@ public class ServoView extends GLSurfaceView
     public void makeCurrent() {
     }
 
+    public void finalizeShutdown() {
+        // shutdown has been requested and completed.
+        mServo.deinit();
+    }
+
     public void inGLThread(Runnable f) {
         queueEvent(f);
     }