Feat: Carry ConvertPointToScreen in a row and trace Chromium's own bubble anchor

rivet/platform/windows/src/bin/chrome-builds.rs

@@ -160,7 +160,12 @@ fn port(
     let wanted = row_port::wanted_names(&named);
     drop(old);
     let mut new = std::collections::HashMap::<String, Vec<u32>>::new();
+    // `views::View::ConvertPointToScreen` is found by its own name, for a prior row without it.
+    let mut screen_origin = Vec::new();
     symbols::publics(new_pdb, &mut |address, name| {
+        if row_port::is_screen_origin_name(name) {
+            screen_origin.push(address);
+        }
         let name = row_port::stable_name(name);
         if wanted.contains(name) {
             new.entry(name.to_owned()).or_default().push(address);
@@ -178,7 +183,7 @@ fn port(
             },
         )
         .collect();
-    let (row, mut report) = row_port::port_row(
+    let (mut row, mut report) = row_port::port_row(
         &prior,
         &named,
         &NewBuild {
@@ -188,6 +193,7 @@ fn port(
             private: &private,
         },
     );
+    row_port::seed_screen_origin(&mut row, &screen_origin, &pe, &mut report);
     report.review.extend(row_port::hand_only_review(&prior));
     let text = |value: &dyn erased::Json| value.pretty();
     std::fs::write(row_path, text(&row)).map_err(|error| error.to_string())?;

rivet/platform/windows/src/row_port.rs

@@ -62,6 +62,8 @@ pub struct PortReport {
     pub derived: Vec<String>,
     /// Values that are not addresses and have no recipe, so they were carried over.
     pub carried: Vec<String>,
+    /// Optional parts the new build could not supply. The runtime does without them.
+    pub left_out: Vec<String>,
 }
 
 fn is_address(value: u64) -> bool {
@@ -1348,6 +1350,61 @@ fn check_frame_slot(frame: &mut Value, pe: &PeFile, report: &mut PortReport) {
     }
 }
 
+/// The start of the public name of `views::View::ConvertPointToScreen(const View*, gfx::Point*)`:
+/// a static function returning nothing. The other overload returns the point and is named `SA?AV`.
+pub const SCREEN_ORIGIN_NAME: &str = "?ConvertPointToScreen@View@views@@SAX";
+
+/// How many bytes of that function's start the runtime compares before it ever calls it.
+const SCREEN_ORIGIN_BYTES: usize = 12;
+
+/// Whether a public name is the one `screen_origin` is found by.
+pub fn is_screen_origin_name(name: &str) -> bool {
+    stable_name(name).starts_with(SCREEN_ORIGIN_NAME)
+}
+
+/// The first whole instructions of a function, at least `SCREEN_ORIGIN_BYTES` long.
+fn function_start(pe: &PeFile, address: u32) -> Option<Vec<u8>> {
+    if !pe.is_code(address) {
+        return None;
+    }
+    let window = pe.read(address, 32)?;
+    let (ends, _) = x64::boundaries(window, SCREEN_ORIGIN_BYTES);
+    let end = *ends.iter().find(|end| **end >= SCREEN_ORIGIN_BYTES)?;
+    Some(window[..end].to_vec())
+}
+
+/// Adds `screen_origin`, `[address, first bytes]` of `views::View::ConvertPointToScreen`, to a row
+/// whose prior row had none, by the function's public name in the new build. A prior row that had
+/// it was ported with every other address. A build with no single place for the name goes without
+/// it, and the runtime keeps placing bubbles from the window's monitor, so the row is not stopped.
+pub fn seed_screen_origin(row: &mut Value, places: &[u32], pe: &PeFile, report: &mut PortReport) {
+    if row["build"]["screen_origin"].is_array() {
+        return;
+    }
+    let mut places = places.to_vec();
+    places.sort_unstable();
+    places.dedup();
+    let found = match places.as_slice() {
+        [only] => function_start(pe, *only).map(|bytes| (*only, bytes)),
+        _ => None,
+    };
+    match found {
+        Some((address, bytes)) => {
+            row["build"]["screen_origin"] =
+                json!([write_address(u64::from(address)), hex_text(&bytes)]);
+            report.derived.push(format!(
+                "build.screen_origin: {SCREEN_ORIGIN_NAME} at {address:#x}"
+            ));
+        }
+        None => {
+            row["build"]["screen_origin"] = Value::Null;
+            report.left_out.push(format!(
+                "build.screen_origin: no single readable function named {SCREEN_ORIGIN_NAME}: {places:x?}"
+            ));
+        }
+    }
+}
+
 /// The review items for the parts of the prior row that only a person writes today. A row made
 /// without them opens Rivet's frame with those features off, so it goes to a person instead of
 /// the table. The runtime names them in the prior row's `hand_only` list.
@@ -1489,6 +1546,44 @@ mod tests {
             .collect()
     }
 
+    #[test]
+    fn nfr3_screen_origin_is_found_by_its_public_name_in_the_new_build() {
+        // push rsi; push rdi; sub rsp, 0x28; mov rsi, rdx; mov rdi, rcx; call.
+        let start = hex("56574883ec284889d64889cfe800000000").unwrap();
+        let bytes = module(&[(0x101400, start.as_slice())]);
+        let pe = PeFile::parse(&bytes).unwrap();
+        let mut row = prior();
+        let mut report = PortReport::default();
+        seed_screen_origin(&mut row, &[0x101400, 0x101400], &pe, &mut report);
+        assert_eq!(
+            row["build"]["screen_origin"],
+            json!(["0x101400", "56574883ec284889d64889cf"])
+        );
+        assert!(report.review.is_empty() && report.left_out.is_empty());
+        // A row that already has it keeps the place the port gave it.
+        seed_screen_origin(&mut row, &[0x101500], &pe, &mut report);
+        assert_eq!(row["build"]["screen_origin"][0], "0x101400");
+        // No name, or two places for it, leaves the part out without stopping the row.
+        for places in [&[][..], &[0x101400, 0x101500][..]] {
+            let mut row = prior();
+            let mut report = PortReport::default();
+            seed_screen_origin(&mut row, places, &pe, &mut report);
+            assert!(row["build"]["screen_origin"].is_null());
+            assert_eq!(report.left_out.len(), 1);
+            assert!(report.review.is_empty());
+        }
+        // Only the in-place overload, which returns nothing, is the one.
+        assert!(is_screen_origin_name(
+            "?ConvertPointToScreen@View@views@@SAXPEBV12@PEAVPoint@gfx@@@Z"
+        ));
+        assert!(!is_screen_origin_name(
+            "?ConvertPointToScreen@View@views@@SA?AVPoint@gfx@@PEBV12@AEBV34@@Z"
+        ));
+        assert!(!is_screen_origin_name(
+            "?ConvertRectToScreen@View@views@@SAXPEBV12@PEAVRect@gfx@@@Z"
+        ));
+    }
+
     #[test]
     fn nfr3_a_new_row_takes_addresses_by_name_and_bytes_from_the_new_module() {
         let code = new_code();

rivet/runtime/windows/src/builds/brave.rs

@@ -43,6 +43,7 @@ pub static BRAVE: Build = Build {
     omnibox: None,
     page_tools: None,
     bubble_anchor: None,
+    screen_origin: None,
     app_window: None,
     browser: "Brave",
     module: "chrome.dll",
@@ -309,6 +310,7 @@ pub static BRAVE_63: Build = Build {
     omnibox: Some(super::omnibox_rows::OMNIBOX_BRAVE_63),
     page_tools: None,
     bubble_anchor: None,
+    screen_origin: None,
     app_window: Some(AppWindow {
         feature_impl: Some(0x00107B60),
         feature_impl_prologue: &[
@@ -710,6 +712,7 @@ pub static BRAVE_157_55: Build = Build {
     omnibox: Some(super::omnibox_rows::OMNIBOX_BRAVE_157_55),
     page_tools: None,
     bubble_anchor: None,
+    screen_origin: None,
     app_window: Some(AppWindow {
         feature_impl: Some(0x0017C0B0),
         feature_impl_prologue: &[

rivet/runtime/windows/src/builds/chrome.rs

@@ -52,6 +52,7 @@ pub static CHROME: Build = Build {
     omnibox: None,
     page_tools: None,
     bubble_anchor: None,
+    screen_origin: None,
     app_window: None,
     browser: "Google Chrome",
     module: "chrome.dll",
@@ -391,6 +392,7 @@ pub static CHROME_156_74: Build = Build {
     omnibox: Some(super::omnibox_rows::OMNIBOX_CHROME_156_74),
     page_tools: None,
     bubble_anchor: None,
+    screen_origin: None,
     app_window: Some(AppWindow {
         feature_impl: Some(0x001BE6D0),
         feature_impl_prologue: &[
@@ -810,6 +812,7 @@ pub static CHROME_156_80: Build = Build {
     omnibox: Some(super::omnibox_rows::OMNIBOX_CHROME_156_80),
     page_tools: None,
     bubble_anchor: None,
+    screen_origin: None,
     app_window: Some(AppWindow {
         feature_impl: Some(0x001E54E0),
         feature_impl_prologue: &[
@@ -1257,6 +1260,7 @@ pub static CHROME_157_38: Build = Build {
     omnibox: Some(super::omnibox_rows::OMNIBOX_CHROME_157_38),
     page_tools: None,
     bubble_anchor: None,
+    screen_origin: None,
     app_window: Some(AppWindow {
         feature_impl: Some(0x0012D300),
         feature_impl_prologue: &[
@@ -1661,6 +1665,7 @@ pub static CHROME_150: Build = Build {
     omnibox: None,
     page_tools: None,
     bubble_anchor: None,
+    screen_origin: None,
     app_window: None,
     browser: "Google Chrome",
     module: "chrome.dll",

rivet/runtime/windows/src/builds/edge.rs

@@ -51,6 +51,7 @@ pub static EDGE: Build = Build {
     omnibox: None,
     page_tools: None,
     bubble_anchor: None,
+    screen_origin: None,
     app_window: None,
     browser: "Microsoft Edge",
     module: "msedge.dll",

rivet/runtime/windows/src/builds/mod.rs

@@ -55,6 +55,10 @@ pub struct Build {
     /// `BubbleDialogDelegate::GetAnchorRect` and its first bytes, when the row carries them. A
     /// built-in row keeps it in the bubble anchor's own list instead.
     pub bubble_anchor: Option<(usize, &'static [u8])>,
+    /// `views::View::ConvertPointToScreen(const View*, gfx::Point*)` and its first bytes, when the
+    /// row carries them: where Chromium itself puts a view in screen DIPs. Without it, bubbles are
+    /// placed from the window's monitor.
+    pub screen_origin: Option<(usize, &'static [u8])>,
 }
 }
 

rivet/runtime/windows/src/builds/samsung.rs

@@ -9,6 +9,7 @@ pub static SAMSUNG: Build = Build {
     omnibox: Some(super::omnibox_rows::OMNIBOX_SAMSUNG_29),
     page_tools: None,
     bubble_anchor: None,
+    screen_origin: None,
     app_window: Some(AppWindow {
         feature_impl: Some(0x001FBB10),
         feature_impl_prologue: &[

rivet/runtime/windows/src/builds/tests.rs

@@ -1619,4 +1619,16 @@ fn nfr6_a_table_row_carries_the_page_tools_and_bubble_anchor_of_its_prior_row()
         crate::builds::bubble_anchor_of(build),
         crate::builds::bubble_anchor_of(&CHROME_157_38)
     );
+    // No built-in row has `ConvertPointToScreen`; a table row found by its name brings it.
+    assert!(chrome["build"]["screen_origin"].is_null());
+    assert_eq!(build.screen_origin, None);
+    row["screen_origin"] = serde_json::json!(["0x1234560", "56574883ec284889d64889cf"]);
+    let build = Build::from_row(&row).unwrap();
+    assert_eq!(
+        build.screen_origin,
+        Some((
+            0x1234560,
+            &[0x56, 0x57, 0x48, 0x83, 0xEC, 0x28, 0x48, 0x89, 0xD6, 0x48, 0x89, 0xCF][..]
+        ))
+    );
 }

rivet/runtime/windows/src/builds/whale.rs

@@ -19,6 +19,7 @@ pub static WHALE: Build = Build {
     omnibox: Some(super::omnibox_rows::OMNIBOX_WHALE_57),
     page_tools: None,
     bubble_anchor: None,
+    screen_origin: None,
     app_window: Some(AppWindow {
         // `WindowFeatureController::SupportsWindowFeatureImpl`, found statically and unverified: the
         // function `BrowserView::ShouldDrawTabStrip` asks through the thunk at 0x8104DF0

rivet/runtime/windows/src/chromium/mod.rs

@@ -1,4 +1,5 @@
 pub(crate) mod image;
 pub(crate) mod memory;
 pub(crate) mod reflect;
+pub(crate) mod screen;
 pub(crate) mod view;

rivet/runtime/windows/src/chromium/screen.rs

@@ -0,0 +1,69 @@
+//! Where Chromium itself puts a view on the screen.
+//!
+//! `views::View::ConvertPointToScreen(const View*, gfx::Point*)` is the conversion Chromium's own
+//! bubbles use for their anchor, so a place found through it is the browser's own, in screen DIPs,
+//! even for a window whose scale differs from that of the monitor it is on. A row may carry the
+//! function with its first bytes. The bytes are checked against the loaded module once, and the
+//! function is called only after they matched.
+
+use std::sync::atomic::{AtomicUsize, Ordering};
+
+static CONVERT: AtomicUsize = AtomicUsize::new(0);
+type ConvertPointToScreen = unsafe extern "system" fn(*const core::ffi::c_void, *mut [i32; 2]);
+
+/// The fewest bytes that tell a function start apart.
+const LEAST_PROLOGUE: usize = 8;
+
+/// Whether the bytes found at the recorded place are the ones the row records.
+fn accepts(found: Option<&[u8]>, recorded: &[u8]) -> bool {
+    recorded.len() >= LEAST_PROLOGUE && found == Some(recorded)
+}
+
+/// Checks the row's `ConvertPointToScreen` against the loaded module, and says what it found.
+pub(crate) fn install() -> &'static str {
+    let Some((at, prologue)) = crate::builds::current().and_then(|build| build.screen_origin)
+    else {
+        return "the row has no ConvertPointToScreen";
+    };
+    let Some(image) = crate::chromium::image::recorded() else {
+        return "no recorded module";
+    };
+    if !accepts(unsafe { image.bytes(at, prologue.len()) }, prologue) {
+        return "ConvertPointToScreen is not the recorded one";
+    }
+    CONVERT.store(image.base + at, Ordering::Release);
+    "ConvertPointToScreen checked"
+}
+
+/// The top left corner of `view` in screen DIPs, as Chromium puts it. `None` when the row has no
+/// checked `ConvertPointToScreen`.
+///
+/// # Safety
+/// `view` must be a live `views::View` of the browser, used on the browser's UI thread.
+pub(crate) unsafe fn origin(view: usize) -> Option<(i32, i32)> {
+    let at = CONVERT.load(Ordering::Acquire);
+    if at == 0 || view == 0 {
+        return None;
+    }
+    let convert: ConvertPointToScreen = std::mem::transmute(at);
+    let mut point = [0i32; 2];
+    convert(view as *const core::ffi::c_void, &mut point);
+    Some((point[0], point[1]))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn fr12_screen_origin_is_called_only_after_its_bytes_matched() {
+        let recorded: &[u8] = &[0x56, 0x57, 0x48, 0x83, 0xEC, 0x28, 0x48, 0x89, 0xD6];
+        assert!(accepts(Some(recorded), recorded));
+        assert!(!accepts(Some(&recorded[1..]), recorded));
+        assert!(!accepts(None, recorded));
+        // Too few bytes say nothing about which function lies there.
+        assert!(!accepts(Some(&recorded[..4]), &recorded[..4]));
+        // Without a checked function there is no origin, and nothing is called.
+        assert_eq!(unsafe { origin(0x1000) }, None);
+    }
+}