From 854a4bdbf4bc258b70874d35e2bc96831060e7ba Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Tue, 25 Sep 2018 15:18:59 -0400 Subject: [PATCH 1/4] Redirect stdout/stderr to logcat. --- ports/libsimpleservo/src/jniapi.rs | 95 +++++++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-) diff --git a/ports/libsimpleservo/src/jniapi.rs b/ports/libsimpleservo/src/jniapi.rs index a41dbb61088..5c84b47ea8d 100644 --- a/ports/libsimpleservo/src/jniapi.rs +++ b/ports/libsimpleservo/src/jniapi.rs @@ -13,7 +13,10 @@ use jni::sys::{jboolean, jfloat, jint, jstring, JNI_TRUE}; use log::Level; use std; use std::borrow::Cow; -use std::os::raw::c_void; +use std::ffi::CString; +use std::mem; +use std::os::raw::{c_char, c_int, c_long, c_void}; +use std::ptr; use std::sync::{Arc, Mutex}; struct HostCallbacks { @@ -77,6 +80,7 @@ pub fn Java_com_mozilla_servoview_JNIServo_init( info!("init"); initialize_android_glue(&env, activity); + redirect_stdout_to_logcat(); let args = env.get_string(args) .expect("Couldn't get java string") @@ -434,3 +438,92 @@ fn initialize_android_glue(env: &JNIEnv, activity: JObject) { ANDROID_APP = Box::into_raw(Box::new(app)); } } + + +#[allow(non_camel_case_types)] +type pthread_t = c_long; +#[allow(non_camel_case_types)] +type pthread_mutexattr_t = c_long; +#[allow(non_camel_case_types)] +type pthread_attr_t = c_void; // FIXME: wrong + +extern { + fn pipe(_: *mut c_int) -> c_int; + fn dup2(fildes: c_int, fildes2: c_int) -> c_int; + fn read(fd: c_int, buf: *mut c_void, count: usize) -> isize; + fn pthread_create(_: *mut pthread_t, _: *const pthread_attr_t, + _: extern fn(*mut c_void) -> *mut c_void, _: *mut c_void) -> c_int; + fn pthread_detach(thread: pthread_t) -> c_int; + + pub fn __android_log_write(prio: c_int, tag: *const c_char, text: *const c_char) -> c_int; +} +fn redirect_stdout_to_logcat() { + // The first step is to redirect stdout and stderr to the logs. + unsafe { + // We redirect stdout and stderr to a custom descriptor. + let mut pfd: [c_int; 2] = [0, 0]; + pipe(pfd.as_mut_ptr()); + dup2(pfd[1], 1); + dup2(pfd[1], 2); + + // Then we spawn a thread whose only job is to read from the other side of the + // pipe and redirect to the logs. + extern fn logging_thread(descriptor: *mut c_void) -> *mut c_void { + unsafe { + let descriptor = descriptor as usize as c_int; + let mut buf: Vec = Vec::with_capacity(512); + let mut cursor = 0_usize; + + // TODO: shouldn't use Rust stdlib + let tag = CString::new("simpleservo").unwrap(); + let tag = tag.as_ptr(); + + loop { + let result = read(descriptor, buf.as_mut_ptr().offset(cursor as isize) as *mut _, + buf.capacity() - 1 - cursor); + + let len = if result == 0 { + return ptr::null_mut(); + } else if result < 0 { + return ptr::null_mut(); /* TODO: report problem */ + } else { + result as usize + cursor + }; + + buf.set_len(len); + + if let Some(last_newline_pos) = buf.iter().rposition(|&c| c == b'\n' as c_char) { + buf[last_newline_pos] = b'\0' as c_char; + __android_log_write(3, tag, buf.as_ptr()); + if last_newline_pos < buf.len() - 1 { + let last_newline_pos = last_newline_pos + 1; + cursor = buf.len() - last_newline_pos; + debug_assert!(cursor < buf.capacity()); + for j in 0..cursor as usize { + buf[j] = buf[last_newline_pos + j]; + } + buf[cursor] = b'\0' as c_char; + buf.set_len(cursor + 1); + } else { + cursor = 0; + } + } else { + cursor = buf.len(); + } + if cursor == buf.capacity() - 1 { + __android_log_write(3, tag, buf.as_ptr()); + buf.set_len(0); + cursor = 0; + } + } + } + } + + let mut thread = mem::uninitialized(); + let result = pthread_create(&mut thread, ptr::null(), logging_thread, + pfd[0] as usize as *mut c_void); + assert_eq!(result, 0); + let result = pthread_detach(thread); + assert_eq!(result, 0); + } +} From 38d3f591706afe1dbd9fdfeacb50c8948fbe9925 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Tue, 25 Sep 2018 15:20:23 -0400 Subject: [PATCH 2/4] Add useful logging defaults for debugging web content on android. --- ports/libsimpleservo/src/jniapi.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/ports/libsimpleservo/src/jniapi.rs b/ports/libsimpleservo/src/jniapi.rs index 5c84b47ea8d..8b5c9d7f0ae 100644 --- a/ports/libsimpleservo/src/jniapi.rs +++ b/ports/libsimpleservo/src/jniapi.rs @@ -64,11 +64,19 @@ pub fn Java_com_mozilla_servoview_JNIServo_init( // Note: Android debug logs are stripped from a release build. // debug!() will only show in a debug build. Use info!() if logs // should show up in adb logcat with a release build. - let mut filter = Filter::default() - .with_min_level(Level::Debug) - .with_allowed_module_path("simpleservo::api") - .with_allowed_module_path("simpleservo::jniapi") - .with_allowed_module_path("simpleservo::gl_glue::egl"); + let filters = [ + "simpleservo::api", + "simpleservo::jniapi", + "simpleservo::gl_glue::egl", + // Show JS errors by default. + "script::dom::bindings::error", + // Show GL errors by default. + "canvas::webgl_thread", + ]; + let mut filter = Filter::default().with_min_level(Level::Debug); + for &module in &filters { + filter = filter.with_allowed_module_path(module); + } let log_str = env.get_string(log_str).ok(); let log_str = log_str.as_ref().map_or(Cow::Borrowed(""), |s| s.to_string_lossy()); for module in log_str.split(',') { From f402dd9ecfe34ca98d69edd8a8f571fcc63a35d1 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 26 Sep 2018 10:54:43 -0400 Subject: [PATCH 3/4] Use Rust stdlib threads for android logging. --- ports/libsimpleservo/src/jniapi.rs | 116 ++++++++++++----------------- 1 file changed, 48 insertions(+), 68 deletions(-) diff --git a/ports/libsimpleservo/src/jniapi.rs b/ports/libsimpleservo/src/jniapi.rs index 8b5c9d7f0ae..85b9b1e7952 100644 --- a/ports/libsimpleservo/src/jniapi.rs +++ b/ports/libsimpleservo/src/jniapi.rs @@ -10,14 +10,14 @@ use gl_glue; use jni::{JNIEnv, JavaVM}; use jni::objects::{GlobalRef, JClass, JObject, JString, JValue}; use jni::sys::{jboolean, jfloat, jint, jstring, JNI_TRUE}; +use libc::{pipe, dup2, read}; use log::Level; use std; use std::borrow::Cow; use std::ffi::CString; -use std::mem; -use std::os::raw::{c_char, c_int, c_long, c_void}; -use std::ptr; +use std::os::raw::{c_char, c_int, c_void}; use std::sync::{Arc, Mutex}; +use std::thread; struct HostCallbacks { callbacks: GlobalRef, @@ -447,91 +447,71 @@ fn initialize_android_glue(env: &JNIEnv, activity: JObject) { } } - -#[allow(non_camel_case_types)] -type pthread_t = c_long; -#[allow(non_camel_case_types)] -type pthread_mutexattr_t = c_long; -#[allow(non_camel_case_types)] -type pthread_attr_t = c_void; // FIXME: wrong - extern { - fn pipe(_: *mut c_int) -> c_int; - fn dup2(fildes: c_int, fildes2: c_int) -> c_int; - fn read(fd: c_int, buf: *mut c_void, count: usize) -> isize; - fn pthread_create(_: *mut pthread_t, _: *const pthread_attr_t, - _: extern fn(*mut c_void) -> *mut c_void, _: *mut c_void) -> c_int; - fn pthread_detach(thread: pthread_t) -> c_int; - pub fn __android_log_write(prio: c_int, tag: *const c_char, text: *const c_char) -> c_int; } + fn redirect_stdout_to_logcat() { // The first step is to redirect stdout and stderr to the logs. + // We redirect stdout and stderr to a custom descriptor. + let mut pfd: [c_int; 2] = [0, 0]; unsafe { - // We redirect stdout and stderr to a custom descriptor. - let mut pfd: [c_int; 2] = [0, 0]; pipe(pfd.as_mut_ptr()); dup2(pfd[1], 1); dup2(pfd[1], 2); + } - // Then we spawn a thread whose only job is to read from the other side of the - // pipe and redirect to the logs. - extern fn logging_thread(descriptor: *mut c_void) -> *mut c_void { - unsafe { - let descriptor = descriptor as usize as c_int; - let mut buf: Vec = Vec::with_capacity(512); - let mut cursor = 0_usize; + let descriptor = pfd[0]; - // TODO: shouldn't use Rust stdlib - let tag = CString::new("simpleservo").unwrap(); - let tag = tag.as_ptr(); + // Then we spawn a thread whose only job is to read from the other side of the + // pipe and redirect to the logs. + let _detached = thread::spawn(move || { + unsafe { + let mut buf: Vec = Vec::with_capacity(512); + let mut cursor = 0_usize; - loop { - let result = read(descriptor, buf.as_mut_ptr().offset(cursor as isize) as *mut _, - buf.capacity() - 1 - cursor); + // TODO: shouldn't use Rust stdlib + let tag = CString::new("simpleservo").unwrap(); + let tag = tag.as_ptr(); - let len = if result == 0 { - return ptr::null_mut(); - } else if result < 0 { - return ptr::null_mut(); /* TODO: report problem */ - } else { - result as usize + cursor - }; + loop { + let result = read(descriptor, buf.as_mut_ptr().offset(cursor as isize) as *mut _, + buf.capacity() - 1 - cursor); - buf.set_len(len); + let len = if result == 0 { + return; + } else if result < 0 { + return; /* TODO: report problem */ + } else { + result as usize + cursor + }; - if let Some(last_newline_pos) = buf.iter().rposition(|&c| c == b'\n' as c_char) { - buf[last_newline_pos] = b'\0' as c_char; - __android_log_write(3, tag, buf.as_ptr()); - if last_newline_pos < buf.len() - 1 { - let last_newline_pos = last_newline_pos + 1; - cursor = buf.len() - last_newline_pos; - debug_assert!(cursor < buf.capacity()); - for j in 0..cursor as usize { - buf[j] = buf[last_newline_pos + j]; - } - buf[cursor] = b'\0' as c_char; - buf.set_len(cursor + 1); - } else { - cursor = 0; + buf.set_len(len); + + if let Some(last_newline_pos) = buf.iter().rposition(|&c| c == b'\n' as c_char) { + buf[last_newline_pos] = b'\0' as c_char; + __android_log_write(3, tag, buf.as_ptr()); + if last_newline_pos < buf.len() - 1 { + let last_newline_pos = last_newline_pos + 1; + cursor = buf.len() - last_newline_pos; + debug_assert!(cursor < buf.capacity()); + for j in 0..cursor as usize { + buf[j] = buf[last_newline_pos + j]; } + buf[cursor] = b'\0' as c_char; + buf.set_len(cursor + 1); } else { - cursor = buf.len(); - } - if cursor == buf.capacity() - 1 { - __android_log_write(3, tag, buf.as_ptr()); - buf.set_len(0); cursor = 0; } + } else { + cursor = buf.len(); + } + if cursor == buf.capacity() - 1 { + __android_log_write(3, tag, buf.as_ptr()); + buf.set_len(0); + cursor = 0; } } } - - let mut thread = mem::uninitialized(); - let result = pthread_create(&mut thread, ptr::null(), logging_thread, - pfd[0] as usize as *mut c_void); - assert_eq!(result, 0); - let result = pthread_detach(thread); - assert_eq!(result, 0); - } + }); } From b11f0719763741090f927630671c076b07d68022 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 26 Sep 2018 11:00:11 -0400 Subject: [PATCH 4/4] Rewrite android logging to use checked slices. --- ports/libsimpleservo/src/jniapi.rs | 81 ++++++++++++++++-------------- 1 file changed, 43 insertions(+), 38 deletions(-) diff --git a/ports/libsimpleservo/src/jniapi.rs b/ports/libsimpleservo/src/jniapi.rs index 85b9b1e7952..70a65652727 100644 --- a/ports/libsimpleservo/src/jniapi.rs +++ b/ports/libsimpleservo/src/jniapi.rs @@ -14,7 +14,6 @@ use libc::{pipe, dup2, read}; use log::Level; use std; use std::borrow::Cow; -use std::ffi::CString; use std::os::raw::{c_char, c_int, c_void}; use std::sync::{Arc, Mutex}; use std::thread; @@ -466,51 +465,57 @@ fn redirect_stdout_to_logcat() { // Then we spawn a thread whose only job is to read from the other side of the // pipe and redirect to the logs. let _detached = thread::spawn(move || { - unsafe { - let mut buf: Vec = Vec::with_capacity(512); - let mut cursor = 0_usize; + const BUF_LENGTH: usize = 512; + let mut buf = vec![b'\0' as c_char; BUF_LENGTH]; - // TODO: shouldn't use Rust stdlib - let tag = CString::new("simpleservo").unwrap(); - let tag = tag.as_ptr(); + // Always keep at least one null terminator + const BUF_AVAILABLE: usize = BUF_LENGTH - 1; + let buf = &mut buf[..BUF_AVAILABLE]; - loop { - let result = read(descriptor, buf.as_mut_ptr().offset(cursor as isize) as *mut _, - buf.capacity() - 1 - cursor); + let mut cursor = 0_usize; - let len = if result == 0 { - return; - } else if result < 0 { - return; /* TODO: report problem */ - } else { - result as usize + cursor - }; + let tag = b"simpleservo\0".as_ptr() as _; - buf.set_len(len); - - if let Some(last_newline_pos) = buf.iter().rposition(|&c| c == b'\n' as c_char) { - buf[last_newline_pos] = b'\0' as c_char; - __android_log_write(3, tag, buf.as_ptr()); - if last_newline_pos < buf.len() - 1 { - let last_newline_pos = last_newline_pos + 1; - cursor = buf.len() - last_newline_pos; - debug_assert!(cursor < buf.capacity()); - for j in 0..cursor as usize { - buf[j] = buf[last_newline_pos + j]; - } - buf[cursor] = b'\0' as c_char; - buf.set_len(cursor + 1); - } else { - cursor = 0; - } - } else { - cursor = buf.len(); + loop { + let result = { + let read_into = &mut buf[cursor..]; + unsafe { + read(descriptor, read_into.as_mut_ptr() as *mut _, read_into.len()) } - if cursor == buf.capacity() - 1 { + }; + + let end = if result == 0 { + return; + } else if result < 0 { + return; /* TODO: report problem */ + } else { + result as usize + cursor + }; + + if let Some(last_newline_pos) = buf.iter().rposition(|&c| c == b'\n' as c_char) { + buf[last_newline_pos] = b'\0' as c_char; + unsafe { __android_log_write(3, tag, buf.as_ptr()); - buf.set_len(0); + } + if last_newline_pos < buf.len() - 1 { + let pos_after_newline = last_newline_pos + 1; + let len_not_logged_yet = buf[pos_after_newline..].len(); + for j in 0..len_not_logged_yet as usize { + buf[j] = buf[pos_after_newline + j]; + } + cursor = len_not_logged_yet; + } else { cursor = 0; } + } else if cursor == BUF_AVAILABLE { + // No newline found but the buffer is full, flush it anyway. + // `buf.as_ptr()` is null-terminated by BUF_LENGTH being 1 less than BUF_AVAILABLE. + unsafe { + __android_log_write(3, tag, buf.as_ptr()); + } + cursor = 0; + } else { + cursor = end; } } });