Auto merge of #15609 - fabricedesre:android-cleanup, r=Wafflespeanut

Android cleanup

<!-- Please describe your changes on the following line: -->
As I dive in the Android code, a bit of cleanup:
- jni/main.c was totally useless, and its presence misleading. I left the jni/ directory to please `ndk-build`.
- the logging redirection in servo/main.rs was redondant with the implementation in the injected android glue.

Most other changes are just due to my editor doing a `rustfmt` when saving files.
I also turned on RUST_LOG to `debug` by default, that helps quite a bit to see what's happening.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X ] `./mach build -d` does not report any errors
- [X ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X ] These changes do not require tests because unfortunately we don't have Android tests.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15609)
<!-- Reviewable:end -->

Cargo.lock

@@ -2463,7 +2463,6 @@ dependencies = [
 name = "servo"
 version = "0.0.1"
 dependencies = [
- "android_glue 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "android_injected_glue 0.2.1 (git+https://github.com/mmatyas/android-rs-injected-glue)",
  "backtrace 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "browserhtml 0.1.17 (git+https://github.com/browserhtml/browserhtml?branch=crate)",
@@ -2471,7 +2470,6 @@ dependencies = [
  "gfx_tests 0.0.1",
  "glutin_app 0.0.1",
  "layout_tests 0.0.1",
- "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",
  "libservo 0.0.1",
  "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "net_tests 0.0.1",

ports/servo/Cargo.toml

@@ -46,6 +46,4 @@ libservo = {path = "../../components/servo"}
 sig = "0.1"
 
 [target.'cfg(target_os = "android")'.dependencies]
-libc = "0.2"
-android_glue = "0.2"
 android_injected_glue = {git = "https://github.com/mmatyas/android-rs-injected-glue"}

ports/servo/main.rs

@@ -17,16 +17,11 @@
 
 #![feature(start, core_intrinsics)]
 
-#[cfg(target_os = "android")]
-#[macro_use]
-extern crate android_glue;
 #[cfg(target_os = "android")]
 extern crate android_injected_glue;
 extern crate backtrace;
 // The window backed by glutin
 extern crate glutin_app as app;
-#[cfg(target_os = "android")]
-extern crate libc;
 #[macro_use]
 extern crate log;
 // The Servo engine
@@ -65,7 +60,8 @@ fn install_crash_handler() {
     use std::thread;
 
     fn handler(_sig: i32) {
-        let name = thread::current().name()
+        let name = thread::current()
+            .name()
             .map(|n| format!(" for thread \"{}\"", n))
             .unwrap_or("".to_owned());
         println!("Stack trace{}\n{:?}", name, Backtrace::new());
@@ -81,8 +77,7 @@ fn install_crash_handler() {
 }
 
 #[cfg(target_os = "android")]
-fn install_crash_handler() {
+fn install_crash_handler() {}
-}
 
 fn main() {
     install_crash_handler();
@@ -106,15 +101,21 @@ fn main() {
         warn!("Panic hook called.");
         let msg = match info.payload().downcast_ref::<&'static str>() {
             Some(s) => *s,
-            None => match info.payload().downcast_ref::<String>() {
+            None => {
+                match info.payload().downcast_ref::<String>() {
                     Some(s) => &**s,
                     None => "Box<Any>",
+                }
             },
         };
         let current_thread = thread::current();
         let name = current_thread.name().unwrap_or("<unnamed>");
         if let Some(location) = info.location() {
-            println!("{} (thread {}, at {}:{})", msg, name, location.file(), location.line());
+            println!("{} (thread {}, at {}:{})",
+                     msg,
+                     name,
+                     location.file(),
+                     location.line());
         } else {
             println!("{} (thread {})", msg, name);
         }
@@ -128,7 +129,7 @@ fn main() {
     setup_logging();
 
     if let Some(token) = content_process_token {
-        return servo::run_content_process(token)
+        return servo::run_content_process(token);
     }
 
     if opts::get().is_printing_version {
@@ -155,17 +156,16 @@ fn main() {
     loop {
         let should_continue = browser.browser.handle_events(window.wait_events());
         if !should_continue {
-            break
+            break;
+        }
     }
-    };
 
     unregister_glutin_resize_handler(&window);
 
     platform::deinit()
 }
 
-fn register_glutin_resize_handler(window: &Rc<app::window::Window>,
+fn register_glutin_resize_handler(window: &Rc<app::window::Window>, browser: &mut BrowserWrapper) {
-                                        browser: &mut BrowserWrapper) {
     unsafe {
         window.set_nested_event_loop_listener(browser);
     }
@@ -188,7 +188,7 @@ impl app::NestedEventLoopListener for BrowserWrapper {
             _ => false,
         };
         if !self.browser.handle_events(vec![event]) {
-            return false
+            return false;
         }
         if is_resize {
             self.browser.repaint_synchronously()
@@ -199,12 +199,14 @@ impl app::NestedEventLoopListener for BrowserWrapper {
 
 #[cfg(target_os = "android")]
 fn setup_logging() {
-    android::setup_logging();
+    // Piping logs from stdout/stderr to logcat happens in android_injected_glue.
+    ::std::env::set_var("RUST_LOG", "debug");
+
+    unsafe { android_injected_glue::ffi::app_dummy() };
 }
 
 #[cfg(not(target_os = "android"))]
-fn setup_logging() {
+fn setup_logging() {}
-}
 
 #[cfg(target_os = "android")]
 /// Attempt to read parameters from a file since they are not passed to us in Android environments.
@@ -233,11 +235,10 @@ fn args() -> Vec<String> {
             vec
         },
         Err(e) => {
-            debug!("Failed to open params file '{}': {}", PARAMS_FILE, Error::description(&e));
+            debug!("Failed to open params file '{}': {}",
-            vec![
+                   PARAMS_FILE,
-                "servo".to_owned(),
+                   Error::description(&e));
-                "http://en.wikipedia.org/wiki/Rust".to_owned()
+            vec!["servo".to_owned(), "http://en.wikipedia.org/wiki/Rust".to_owned()]
-            ]
         },
     }
 }
@@ -254,61 +255,5 @@ fn args() -> Vec<String> {
 #[inline(never)]
 #[allow(non_snake_case)]
 pub extern "C" fn android_main(app: *mut ()) {
-    android_injected_glue::android_main2(app as *mut _, move |_, _| { main() });
+    android_injected_glue::android_main2(app as *mut _, move |_, _| main());
-}
-
-
-#[cfg(target_os = "android")]
-mod android {
-    extern crate android_glue;
-    extern crate android_injected_glue;
-    extern crate libc;
-
-    use self::libc::c_int;
-    use std::borrow::ToOwned;
-
-    pub fn setup_logging() {
-        use self::libc::{STDERR_FILENO, STDOUT_FILENO};
-        //use std::env;
-
-        //env::set_var("RUST_LOG", "servo,gfx,msg,util,layers,js,std,rt,extra");
-        redirect_output(STDERR_FILENO);
-        redirect_output(STDOUT_FILENO);
-
-        unsafe { android_injected_glue::ffi::app_dummy() };
-    }
-
-    struct FilePtr(*mut self::libc::FILE);
-
-    unsafe impl Send for FilePtr {}
-
-    fn redirect_output(file_no: c_int) {
-        use self::libc::{pipe, dup2};
-        use self::libc::fdopen;
-        use self::libc::fgets;
-        use std::ffi::CStr;
-        use std::ffi::CString;
-        use std::str::from_utf8;
-        use std::thread;
-
-        unsafe {
-            let mut pipes: [c_int; 2] = [ 0, 0 ];
-            pipe(pipes.as_mut_ptr());
-            dup2(pipes[1], file_no);
-            let mode = CString::new("r").unwrap();
-            let input_file = FilePtr(fdopen(pipes[0], mode.as_ptr()));
-            thread::Builder::new().name("android-logger".to_owned()).spawn(move || {
-                static READ_SIZE: usize = 1024;
-                let mut read_buffer = vec![0; READ_SIZE];
-                let FilePtr(input_file) = input_file;
-                loop {
-                    fgets(read_buffer.as_mut_ptr(), (read_buffer.len() as i32)-1, input_file);
-                    let c_str = CStr::from_ptr(read_buffer.as_ptr());
-                    let slice = from_utf8(c_str.to_bytes()).unwrap();
-                    android_glue::write_log(slice);
-                }
-            });
-        }
-    }
-
 }

support/android/apk/jni/main.c

@@ -1,58 +0,0 @@
-/*
- * Copyright (C) 2010 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- */
-
-//BEGIN_INCLUDE(all)
-#include <jni.h>
-#include <errno.h>
-#include <dlfcn.h>
-#include <stdlib.h>
-
-#include <android/log.h>
-#include <android_native_app_glue.h>
-
-#define LOGI(...) ((void)__android_log_print(ANDROID_LOG_INFO, "servo-wrapper", __VA_ARGS__))
-#define LOGW(...) ((void)__android_log_print(ANDROID_LOG_WARN, "servo-wrapper", __VA_ARGS__))
-
-/**
- * This is the main entry point of a native application that is using
- * android_native_app_glue.  It runs in its own thread, with its own
- * event loop for receiving input events and doing other things.
- */
-void android_main(struct android_app* state) {
-    LOGI("in android_main");
-    void* libservo = dlopen("libservo.so", RTLD_NOW);
-    if (libservo == NULL) {
-        LOGI("failed to load servo lib: %s", dlerror());
-        return;
-    }
-
-    LOGI("loaded libservo.so");
-    void (*android_main)(struct android_app*);
-    *(void**)(&android_main) = dlsym(libservo, "android_main");
-    if (android_main) {
-        LOGI("go into android_main()");
-        (*android_main)(state);
-        return;
-    }
-}
-//END_INCLUDE(all)
-
-int main(int argc, char* argv[])
-{
-    LOGI("WAT");
-    return 0;
-}

support/android/apk/src/com/mozilla/servo/MainActivity.java

@@ -16,10 +16,12 @@ import java.util.zip.ZipFile;
 
 
 public class MainActivity extends android.app.NativeActivity {
-    private static final String LOGTAG="servo_wrapper";
+    private static final String LOGTAG="ServoWrapper";
     static {
         Log.i(LOGTAG, "Loading the NativeActivity");
+        //  libmain.so contains all of Servo native code with the injected glue.
         System.loadLibrary("main");
+        Log.i(LOGTAG, "libmain.so loaded");
     }
 
     private void set_url(String url) {

support/android/build-apk/src/main.rs

@@ -129,8 +129,7 @@ fn main() {
     } else {
         antcmd.arg("release");
     }
-    let antresult = antcmd
+    let antresult = antcmd.stdout(Stdio::inherit())
-        .stdout(Stdio::inherit())
         .stderr(Stdio::inherit())
         .current_dir(directory.clone())
         .status();
@@ -143,8 +142,7 @@ fn main() {
     // Release builds also need to be signed. For now, we use a simple debug
     // signing key.
     if debug {
-        fs::copy(&directory.join("bin").join("Servo-debug.apk"),
+        fs::copy(&directory.join("bin").join("Servo-debug.apk"), &args.output).unwrap();
-                 &args.output).unwrap();
     } else {
         let keystore_dir = env::home_dir().expect("Please have a home directory");
         let keystore_dir = Path::new(&keystore_dir).join(".keystore");
@@ -181,8 +179,7 @@ fn main() {
                 .stderr(Stdio::inherit())
                 .current_dir(directory.clone())
                 .status();
-            if keytoolcreatecmd.is_err() ||
+            if keytoolcreatecmd.is_err() || keytoolcreatecmd.unwrap().code().unwrap() != 0 {
-               keytoolcreatecmd.unwrap().code().unwrap() != 0 {
                 println!("Error while using `keytool` to create the debug keystore.");
                 process::exit(1);
             }
@@ -209,7 +206,8 @@ fn main() {
         }
 
         fs::copy(&directory.join("bin").join("Servo-release-unsigned.apk"),
-                 &args.output).unwrap();
+                 &args.output)
+            .unwrap();
     }
 
 }
@@ -233,16 +231,16 @@ fn parse_arguments() -> (Args, Vec<String>) {
 
     loop {
         let arg = match args.next() {
-            None => return (
+            None => {
-                Args {
+                return (Args {
                     output: result_output.expect("Could not find -o argument"),
                     root_path: result_root_path.expect("Could not find -r enlistment root argument"),
                     target_path: result_target_path.expect("Could not find -t target path argument"),
                     shared_libraries: result_shared_libraries,
                 },
-                result_passthrough
+                        result_passthrough)
-            ),
+            },
-            Some(arg) => arg
+            Some(arg) => arg,
         };
 
         match &*arg {
@@ -250,12 +248,12 @@ fn parse_arguments() -> (Args, Vec<String>) {
                 result_output = Some(PathBuf::from(args.next().expect("-o must be followed by the output name")));
             },
             "-r" => {
-                result_root_path =
+                result_root_path = Some(PathBuf::from(args.next()
-                    Some(PathBuf::from(args.next().expect("-r must be followed by the enlistment root directory")));
+                    .expect("-r must be followed by the enlistment root directory")));
             },
             "-t" => {
-                result_target_path =
+                result_target_path = Some(PathBuf::from(args.next()
-                    Some(PathBuf::from(args.next().expect("-t must be followed by the target output directory")));
+                    .expect("-t must be followed by the target output directory")));
             },
             "-l" => {
                 let name = args.next().expect("-l must be followed by a library name");
@@ -264,13 +262,13 @@ fn parse_arguments() -> (Args, Vec<String>) {
                 // Also pass these through.
                 result_passthrough.push(arg);
                 result_passthrough.push(name);
-            }
+            },
             _ => {
                 if arg.starts_with("-l") {
                     result_shared_libraries.insert(vec!["lib", &arg[2..], ".so"].concat());
                 }
                 result_passthrough.push(arg)
-            }
+            },
         };
     }
 }
@@ -287,15 +285,13 @@ fn find_native_libs(args: &Args) -> HashMap<String, PathBuf> {
             (Some(file_name), Some(extension)) => {
                 let file_name = file_name.to_str().unwrap();
 
-                if file_name.starts_with("lib") &&
+                if file_name.starts_with("lib") && extension == "so" && args.shared_libraries.contains(file_name) {
-                    extension == "so" &&
-                    args.shared_libraries.contains(file_name) {
                     println!("Adding the file {:?}", file_name);
                     native_shared_libs.insert(file_name.to_string(), path.to_path_buf().clone());
                     break;
                 }
-            }
+            },
-            _ => {}
+            _ => {},
         }
     }