Skip to content

Commit e049890

Browse files
hungerogoffart
andauthored
compiler(rust): Make popup window opening and closing more robust (#9687)
* tests: Add a test for another popup-induced panic * compiler(rust): Make popup window opening and closing more robust Do not make the generated rust code trigger a panic when the parent no longer exists. Fixes: #9722 (rust only) Co-authored-by: Olivier Goffart <[email protected]>
1 parent dcd0945 commit e049890

File tree

3 files changed

+92
-11
lines changed

3 files changed

+92
-11
lines changed

internal/compiler/generator/rust.rs

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2869,11 +2869,18 @@ fn compile_builtin_function_call(
28692869
arguments
28702870
{
28712871
let mut parent_ctx = ctx;
2872-
let mut component_access_tokens = quote!(_self);
2872+
let mut component_access_tokens = MemberAccess::Direct(quote!(_self));
28732873
if let llr::PropertyReference::InParent { level, .. } = parent_ref {
28742874
for _ in 0..level.get() {
2875-
component_access_tokens =
2876-
quote!(#component_access_tokens.parent.upgrade().unwrap().as_pin_ref());
2875+
component_access_tokens = match component_access_tokens {
2876+
MemberAccess::Option(token_stream) => MemberAccess::Option(
2877+
quote!(#token_stream.and_then(|a| a.as_pin_ref().parent.upgrade())),
2878+
),
2879+
MemberAccess::Direct(token_stream) => {
2880+
MemberAccess::Option(quote!(#token_stream.parent.upgrade()))
2881+
}
2882+
_ => unreachable!(),
2883+
};
28772884
parent_ctx = parent_ctx.parent.as_ref().unwrap().ctx;
28782885
}
28792886
}
@@ -2894,7 +2901,7 @@ fn compile_builtin_function_call(
28942901
let close_policy = compile_expression(close_policy, ctx);
28952902
let window_adapter_tokens = access_window_adapter_field(ctx);
28962903
let popup_id_name = internal_popup_id(*popup_index as usize);
2897-
quote!({
2904+
component_access_tokens.then(|component_access_tokens| quote!({
28982905
let popup_instance = #popup_window_id::new(#component_access_tokens.self_weak.get().unwrap().clone()).unwrap();
28992906
let popup_instance_vrc = sp::VRc::map(popup_instance.clone(), |x| x);
29002907
let position = { let _self = popup_instance_vrc.as_pin_ref(); #position };
@@ -2911,7 +2918,7 @@ fn compile_builtin_function_call(
29112918
))
29122919
);
29132920
#popup_window_id::user_init(popup_instance_vrc.clone());
2914-
})
2921+
}))
29152922
} else {
29162923
panic!("internal error: invalid args to ShowPopupWindow {arguments:?}")
29172924
}
@@ -2921,18 +2928,34 @@ fn compile_builtin_function_call(
29212928
arguments
29222929
{
29232930
let mut parent_ctx = ctx;
2924-
let mut component_access_tokens = quote!(_self);
2931+
let mut component_access_tokens = MemberAccess::Direct(quote!(_self));
29252932
if let llr::PropertyReference::InParent { level, .. } = parent_ref {
29262933
for _ in 0..level.get() {
2927-
component_access_tokens =
2928-
quote!(#component_access_tokens.parent.upgrade().unwrap().as_pin_ref());
2934+
component_access_tokens = match component_access_tokens {
2935+
MemberAccess::Option(token_stream) => MemberAccess::Option(
2936+
quote!(#token_stream.and_then(|a| a.parent.upgrade())),
2937+
),
2938+
MemberAccess::Direct(token_stream) => {
2939+
MemberAccess::Option(quote!(#token_stream.parent.upgrade()))
2940+
}
2941+
_ => unreachable!(),
2942+
};
29292943
parent_ctx = parent_ctx.parent.as_ref().unwrap().ctx;
29302944
}
29312945
}
29322946
let window_adapter_tokens = access_window_adapter_field(ctx);
29332947
let popup_id_name = internal_popup_id(*popup_index as usize);
2948+
let current_id_tokens = match component_access_tokens {
2949+
MemberAccess::Option(token_stream) => quote!(
2950+
#token_stream.and_then(|a| a.as_pin_ref().#popup_id_name.take())
2951+
),
2952+
MemberAccess::Direct(token_stream) => {
2953+
quote!(#token_stream.as_ref().#popup_id_name.take())
2954+
}
2955+
_ => unreachable!(),
2956+
};
29342957
quote!(
2935-
if let Some(current_id) = #component_access_tokens.#popup_id_name.take() {
2958+
if let Some(current_id) = #current_id_tokens {
29362959
sp::WindowInner::from_pub(#window_adapter_tokens.window()).close_popup(current_id);
29372960
}
29382961
)
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Copyright © SixtyFPS GmbH <[email protected]>
2+
// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
3+
4+
// FIXME: Skip test on C++ and NodeJS since this was only fixed in rust so far
5+
//ignore: cpp,js
6+
7+
export component TestCase inherits Window {
8+
property <bool> condition: true;
9+
in property <bool> condition-2: true;
10+
11+
TouchArea {
12+
clicked => {
13+
popup.show();
14+
}
15+
popup := PopupWindow {
16+
width: root.width;
17+
height: root.height;
18+
h := HorizontalLayout {
19+
if condition: Rectangle {
20+
background: yellow;
21+
if condition-2 : TouchArea {
22+
clicked => {
23+
condition = false;
24+
debug(h.preferred-width);
25+
popup.close();
26+
}
27+
}
28+
}
29+
}
30+
}
31+
}
32+
}
33+
34+
/*
35+
```rust
36+
let instance = TestCase::new().unwrap();
37+
slint_testing::send_mouse_click(&instance, 10., 16.);
38+
slint_testing::send_mouse_click(&instance, 10., 16.);
39+
slint_testing::send_mouse_click(&instance, 10., 16.);
40+
```
41+
42+
```cpp
43+
auto handle = TestCase::create();
44+
const TestCase &instance = *handle;
45+
slint_testing::send_mouse_click(&instance, 10., 16.);
46+
slint_testing::send_mouse_click(&instance, 10., 16.);
47+
slint_testing::send_mouse_click(&instance, 10., 16.);
48+
```
49+
50+
```js
51+
let instance = new slint.TestCase({});
52+
slintlib.private_api.send_mouse_click(instance, 10., 16.);
53+
slintlib.private_api.send_mouse_click(instance, 10., 16.);
54+
slintlib.private_api.send_mouse_click(instance, 10., 16.);
55+
```
56+
*/

tests/driver/rust/build.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@ fn main() -> std::io::Result<()> {
2525
&& source.contains("//bundle-translations")
2626
{
2727
"#[ignore = \"translation bundle not working with the macro\"]"
28-
} else if live_preview && source.contains("ComponentContainer") {
29-
"#[ignore = \"ComponentContainer doesn't work with the interpreter\"]"
28+
} else if live_preview && testcase.is_ignored("js") {
29+
"#[ignore = \"Ignored JS testcases ignored in live-preview mode\"]"
30+
} else if live_preview && testcase.is_ignored("live-preview") {
31+
"#[ignore = \"testcase ignored in live-preview mode\"]"
3032
} else if live_preview && source.contains("#3464") {
3133
"#[ignore = \"issue #3464 not fixed with the interpreter\"]"
3234
} else if live_preview && module_name.contains("widgets_menubar") {

0 commit comments

Comments
 (0)