WidgetDriver: Use matrix_sdk_common::executor::spawn instead of tokio::spawn to make it wasm compatible.

[toger5]

Using the executer::spawn will select tokio::spawn or wasm_bindgen_futures::spawn_local based on what platform we are on.

They behave differently in that tokio actually starts the async block inside the spawn but the wasm spawn wrapper will only start the part that is not inside the async block and the join handle needs to be awaited for it to work.

• Public API changes documented in changelogs (optional)

Signed-off-by:

[toger5]

I think I need help on this since this is not the correct solution. When running on a non wasm platform the await will block.
So there seems to be an issue with

#[cfg(target_arch = "wasm32")]
pub fn spawn<F, T>(future: F) -> JoinHandle<T>
where
    F: Future<Output = T> + 'static,
{
    let (future, remote_handle) = future.remote_handle();
    let (abort_handle, abort_registration) = AbortHandle::new_pair();
    let future = Abortable::new(future, abort_registration);

    wasm_bindgen_futures::spawn_local(async {
        // Poll the future, and ignore the result (either it's `Ok(())`, or it's
        // `Err(Aborted)`).

        let _ = future.await;
    });
    let j = JoinHandle { remote_handle: remote_handle, abort_handle };
    j.
}

Where we need to await the JoinHandle before the future is actually started on WASM.

[jplatte]

Adding the await there is definitely the wrong way to go. I don't see ny reason why it would be required on the wasm side, the docs for wasm_bindgen_futures state quite clearly that it's scheduled to run in the background, so does not need to be awaited. And the abstraction in mtrix-sdk-common would be worse than useless if this was not the case.

[toger5]

I have done a simple experiment in which this is failing however.

matrix_sdk_common::executor::spawn({
                info!("WIDGET_DRIVER a v1");
                async {
                    info!("WIDGET_DRIVER b v1");
                }
            });
    
            matrix_sdk_common::executor::spawn(async {
                info!("WIDGET_DRIVER b v2");
            });
    
            let c = "test".to_string();
            matrix_sdk_common::executor::spawn({
                let c = c.clone();
                info!("WIDGET_DRIVER a v3, {}", c);
                async move {
                    info!("WIDGET_DRIVER b v3 {}", c);
                }
            });

which results in the following outpu:

INFO src/main.rs:32 WIDGET_DRIVER a v1
INFO src/main.rs:45 WIDGET_DRIVER a v3, test

adding await to the returned handle (from spawn) makes it work.

[jplatte]

I wonder whether the AbortHandle aborts on drop? Maybe copy-paste bits of the spawn function and see if you can make it work without awaiting.

[toger5]

Maybe copy-paste bits of the spawn function and see if you can make it work without awaiting.

I tried this already. only using

    wasm_bindgen_futures::spawn_local(async {
        // Poll the future, and ignore the result (either it's `Ok(())`, or it's
        // `Err(Aborted)`).

          future.await;
    });

without calling remote_handle works as expected.
Checking if dropping the remove handle cancels the future is worth a try.

[codecov]

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.89%. Comparing base (bdf5fad) to head (3f544f1).
Report is 81 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/matrix-sdk/src/widget/mod.rs	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4707      +/-   ##
==========================================
- Coverage   85.90%   85.89%   -0.01%     
==========================================
  Files         292      292              
  Lines       33850    33851       +1     
==========================================
- Hits        29078    29076       -2     
- Misses       4772     4775       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.