Auto merge of #23300 - jdm:helpful-ml, r=asajeffrey

Magic Leap developer ergonomic improvements

These changes allow passing in command line arguments for Servo via a SERVO_ARGS from the mldb terminal when launching the activity, as well as passing in a launch URL as an argument. It also redirects stdout to the ML log, which allows observing the built-in profiler output.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #23041
- [x] These changes do not require tests because no magic leap testing.

<!-- 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/23300)
<!-- Reviewable:end -->

Cargo.lock

@@ -2437,6 +2437,7 @@ dependencies = [
 name = "libmlservo"
 version = "0.0.1"
 dependencies = [
+ "libc 0.2.44 (registry+https://github.com/rust-lang/crates.io-index)",
  "libservo 0.0.1",
  "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "servo-egl 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -4178,7 +4179,6 @@ dependencies = [
  "libloading 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "libservo 0.0.1",
  "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
- "serde_json 1.0.13 (registry+https://github.com/rust-lang/crates.io-index)",
  "winapi 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
@@ -4202,6 +4202,7 @@ dependencies = [
  "jni 0.10.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.44 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
+ "serde_json 1.0.13 (registry+https://github.com/rust-lang/crates.io-index)",
  "simpleservo 0.0.1",
 ]
 

ports/libmlservo/Cargo.toml

@@ -15,6 +15,7 @@ bench = false
 [dependencies]
 libservo = { path = "../../components/servo", features = ["no_static_freetype"] }
 simpleservo = { path = "../libsimpleservo/api", features = ["no_static_freetype"] }
+libc = "0.2"
 log = "0.4"
 servo-egl = "0.2"
 smallvec = "0.6"

ports/libmlservo/src/lib.rs

@@ -7,6 +7,7 @@ use egl::egl::EGLDisplay;
 use egl::egl::EGLSurface;
 use egl::egl::MakeCurrent;
 use egl::egl::SwapBuffers;
+use libc::{dup2, pipe, read};
 use log::info;
 use log::warn;
 use servo::euclid::TypedScale;
@@ -25,6 +26,7 @@ use std::ffi::CStr;
 use std::ffi::CString;
 use std::io::Write;
 use std::os::raw::c_char;
+use std::os::raw::c_int;
 use std::os::raw::c_void;
 use std::rc::Rc;
 use std::thread;
@@ -67,6 +69,7 @@ pub enum MLKeyType {
 }
 
 #[repr(transparent)]
+#[derive(Clone, Copy)]
 pub struct MLLogger(extern "C" fn(MLLogLevel, *const c_char));
 
 #[repr(transparent)]
@@ -105,10 +108,12 @@ pub unsafe extern "C" fn init_servo(
     url_update: MLURLUpdate,
     keyboard: MLKeyboard,
     url: *const c_char,
+    args: *const c_char,
     width: u32,
     height: u32,
     hidpi: f32,
 ) -> *mut ServoInstance {
+    redirect_stdout_to_log(logger);
     let _ = log::set_boxed_logger(Box::new(logger));
     log::set_max_level(LOG_LEVEL);
 
@@ -123,8 +128,20 @@ pub unsafe extern "C" fn init_servo(
         width as i32,
         height as i32,
     );
+    let args = if args.is_null() {
+        vec![]
+    } else {
+        CStr::from_ptr(args)
+            .to_str()
+            .unwrap_or("")
+            .split(' ')
+            .map(|s| s.to_owned())
+            .collect()
+    };
+    info!("got args: {:?}", args);
+
     let opts = InitOptions {
-        args: None,
+        args,
         url: Some(url.to_string()),
         density: hidpi,
         enable_subpixel_text_antialiasing: false,
@@ -382,3 +399,79 @@ impl log::Log for MLLogger {
 
     fn flush(&self) {}
 }
+
+fn redirect_stdout_to_log(logger: MLLogger) {
+    // 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 {
+        pipe(pfd.as_mut_ptr());
+        dup2(pfd[1], 1);
+        dup2(pfd[1], 2);
+    }
+
+    let descriptor = pfd[0];
+
+    // 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 || {
+        const BUF_LENGTH: usize = 512;
+        let mut buf = vec![b'\0' as c_char; BUF_LENGTH];
+
+        // Always keep at least one null terminator
+        const BUF_AVAILABLE: usize = BUF_LENGTH - 1;
+        let buf = &mut buf[..BUF_AVAILABLE];
+
+        let mut cursor = 0_usize;
+
+        loop {
+            let result = {
+                let read_into = &mut buf[cursor..];
+                unsafe {
+                    read(
+                        descriptor,
+                        read_into.as_mut_ptr() as *mut _,
+                        read_into.len(),
+                    )
+                }
+            };
+
+            let end = if result == 0 {
+                return;
+            } else if result < 0 {
+                (logger.0)(
+                    MLLogLevel::Error,
+                    b"error in log thread; closing\0".as_ptr() as *const _,
+                );
+                return;
+            } else {
+                result as usize + cursor
+            };
+
+            // Only modify the portion of the buffer that contains real data.
+            let buf = &mut buf[0..end];
+
+            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;
+                (logger.0)(MLLogLevel::Info, buf.as_ptr());
+                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 end == 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.
+                (logger.0)(MLLogLevel::Info, buf.as_ptr());
+                cursor = 0;
+            } else {
+                cursor = end;
+            }
+        }
+    });
+}

ports/libsimpleservo/api/Cargo.toml

@@ -9,7 +9,6 @@ publish = false
 [dependencies]
 libservo = { path = "../../../components/servo" }
 log = "0.4"
-serde_json = "1.0"
 
 [target.'cfg(not(target_os = "macos"))'.dependencies]
 libc = "0.2"

ports/libsimpleservo/api/src/lib.rs

@@ -41,7 +41,7 @@ thread_local! {
 pub use servo::embedder_traits::EventLoopWaker;
 
 pub struct InitOptions {
-    pub args: Option<String>,
+    pub args: Vec<String>,
     pub url: Option<String>,
     pub coordinates: Coordinates,
     pub density: f32,
@@ -135,16 +135,15 @@ pub fn servo_version() -> String {
 /// Initialize Servo. At that point, we need a valid GL context.
 /// In the future, this will be done in multiple steps.
 pub fn init(
-    init_opts: InitOptions,
+    mut init_opts: InitOptions,
     gl: Rc<dyn gl::Gl>,
     waker: Box<dyn EventLoopWaker>,
     callbacks: Box<dyn HostTrait>,
 ) -> Result<(), &'static str> {
     resources::set(Box::new(ResourceReaderInstance::new()));
 
-    if let Some(args) = init_opts.args {
-        let mut args: Vec<String> = serde_json::from_str(&args)
-            .map_err(|_| "Invalid arguments. Servo arguments must be formatted as a JSON array")?;
+    let mut args = mem::replace(&mut init_opts.args, vec![]);
+    if !args.is_empty() {
         // opts::from_cmdline_args expects the first argument to be the binary name.
         args.insert(0, "servo".to_string());
 

ports/libsimpleservo/capi/src/lib.rs

@@ -76,7 +76,12 @@ fn init(
     crate::env_logger::init();
 
     let args = unsafe { CStr::from_ptr(opts.args) };
-    let args = args.to_str().map(|s| s.to_string()).ok();
+    let args = args
+        .to_str()
+        .unwrap_or("")
+        .split(' ')
+        .map(|s| s.to_owned())
+        .collect();
 
     let url = unsafe { CStr::from_ptr(opts.url) };
     let url = url.to_str().map(|s| s.to_string()).ok();

ports/libsimpleservo/jniapi/Cargo.toml

@@ -17,9 +17,10 @@ bench = false
 android_injected_glue = "0.2"
 android_logger = "0.7"
 jni = "0.10.2"
-log = "0.4"
-simpleservo = { path = "../api" }
 libc = "0.2"
+log = "0.4"
+serde_json = "1.0"
+simpleservo = { path = "../api" }
 
 [build-dependencies]
 cc = "1.0"

ports/libsimpleservo/jniapi/src/lib.rs

@@ -683,8 +683,15 @@ fn get_options(env: &JNIEnv, opts: JObject) -> Result<(InitOptions, bool, Option
     .l()
     .map_err(|_| "coordinates is not an object")?;
     let coordinates = jni_coords_to_rust_coords(&env, coordinates)?;
+
+    let args = match args {
+        Some(args) => serde_json::from_str(&args)
+            .map_err(|_| "Invalid arguments. Servo arguments must be formatted as a JSON array")?,
+        None => None,
+    };
+
     let opts = InitOptions {
-        args,
+        args: args.unwrap_or(vec![]),
         url,
         coordinates,
         density,

support/magicleap/Servo2D/code/inc/Servo2D.h

@@ -29,7 +29,7 @@ public:
   /**
    * Constructs the Landscape Application.
    */
-  Servo2D();
+  Servo2D(const char* uri, const char* args);
 
   /**
    * Destroys the Landscape Application.
@@ -144,4 +144,6 @@ private:
   glm::quat controller_orientation_; // The last recorded orientation of the controller (in world coords)
   bool controller_trigger_down_ = false; // Is the controller trigger currently down?
   ServoInstance* servo_ = nullptr; // the servo instance we're embedding
+  const char* uri_ = nullptr;
+  const char* args_ = nullptr;
 };

support/magicleap/Servo2D/code/src/Servo2D.cpp

@@ -68,7 +68,7 @@ void keyboard(Servo2D* app, bool visible) {
 // The functions Servo provides for hooking up to the ML.
 extern "C" ServoInstance* init_servo(EGLContext, EGLSurface, EGLDisplay,
                                      Servo2D*, MLLogger, MLHistoryUpdate, MLURLUpdate, MLKeyboard,
-                                     const char* url, int width, int height, float hidpi);
+                                     const char* url, const char* args, int width, int height, float hidpi);
 extern "C" void heartbeat_servo(ServoInstance*);
 extern "C" void keyboard_servo(ServoInstance*, char32_t code, lumin::ui::KeyType keyType);
 extern "C" void trigger_servo(ServoInstance*, float x, float y, bool down);
@@ -78,7 +78,10 @@ extern "C" void navigate_servo(ServoInstance*, const char* text);
 extern "C" void discard_servo(ServoInstance*);
 
 // Create a Servo2D instance
-Servo2D::Servo2D() {
+Servo2D::Servo2D(const char* uri, const char* args)
+: uri_(uri ? uri : HOME_PAGE)
+, args_(args)
+{
   ML_LOG(Debug, "Servo2D Constructor.");
 }
 
@@ -171,7 +174,7 @@ int Servo2D::init() {
   EGLDisplay dpy = eglGetDisplay(EGL_DEFAULT_DISPLAY);
 
   // Hook into servo
-  servo_ = init_servo(ctx, surf, dpy, this, logger, history, url, keyboard, HOME_PAGE, VIEWPORT_W, VIEWPORT_H, HIDPI);
+  servo_ = init_servo(ctx, surf, dpy, this, logger, history, url, keyboard, uri_, args_, VIEWPORT_W, VIEWPORT_H, HIDPI);
   if (!servo_) {
     ML_LOG(Error, "Servo2D Failed to init servo instance");
     abort();

support/magicleap/Servo2D/code/src/main.cpp

@@ -3,11 +3,34 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 #include <Servo2D.h>
+#include <ml_lifecycle.h>
 #include <ml_logging.h>
 
 int main(int argc, char **argv)
 {
   ML_LOG(Debug, "Servo2D Starting.");
-  Servo2D myApp;
-  return myApp.run();
+
+  // Handle optional initialization string passed via 'mldb launch'
+  MLLifecycleInitArgList* list = NULL;
+  MLLifecycleGetInitArgList(&list);
+  const char* uri = NULL;
+  if (nullptr != list) {
+    int64_t list_length = 0;
+    MLLifecycleGetInitArgListLength(list, &list_length);
+    if (list_length > 0) {
+      const MLLifecycleInitArg* iarg = NULL;
+      MLLifecycleGetInitArgByIndex(list, 0, &iarg);
+      if (nullptr != iarg) {
+        MLLifecycleGetInitArgUri(iarg, &uri);
+      }
+    }
+  }
+
+  const char* args = getenv("SERVO_ARGS");
+
+  Servo2D myApp(uri, args);
+  int rv = myApp.run();
+
+  MLLifecycleFreeInitArgList(&list);
+  return rv;
 }