-
Notifications
You must be signed in to change notification settings - Fork 357
WASM Improvements #5838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WASM Improvements #5838
Changes from all commits
463589e
b87c179
a964f09
fd93a0f
2c701e7
74ad9c9
5b40dd9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -928,7 +928,6 @@ impl Client { | |
| } | ||
| } | ||
|
|
||
| #[cfg(not(target_family = "wasm"))] | ||
| #[matrix_sdk_ffi_macros::export] | ||
| impl Client { | ||
| /// Retrieves a media file from the media source | ||
|
|
@@ -942,22 +941,60 @@ impl Client { | |
| use_cache: bool, | ||
| temp_dir: Option<String>, | ||
| ) -> Result<Arc<MediaFileHandle>, ClientError> { | ||
| let source = (*media_source).clone(); | ||
| let mime_type: mime::Mime = mime_type.parse()?; | ||
| #[cfg(not(target_family = "wasm"))] | ||
| { | ||
| let source = (*media_source).clone(); | ||
| let mime_type: mime::Mime = mime_type.parse()?; | ||
|
|
||
| let handle = self | ||
| .inner | ||
| .media() | ||
| .get_media_file( | ||
| &MediaRequestParameters { source: source.media_source, format: MediaFormat::File }, | ||
| filename, | ||
| &mime_type, | ||
| use_cache, | ||
| temp_dir, | ||
| ) | ||
| .await?; | ||
| let handle = self | ||
| .inner | ||
| .media() | ||
| .get_media_file( | ||
| &MediaRequestParameters { | ||
| source: source.media_source, | ||
| format: MediaFormat::File, | ||
| }, | ||
| filename, | ||
| &mime_type, | ||
| use_cache, | ||
| temp_dir, | ||
| ) | ||
| .await?; | ||
|
|
||
| Ok(Arc::new(MediaFileHandle::new(handle))) | ||
| } | ||
|
|
||
| /// MediaFileHandle uses SdkMediaFileHandle which requires an | ||
| /// intermediate TempFile which is not available on wasm | ||
| /// platforms due to lack of an accessible file system. | ||
| #[cfg(target_family = "wasm")] | ||
| Err(ClientError::Generic { | ||
| msg: "get_media_file is not supported on wasm platforms".to_owned(), | ||
| details: None, | ||
| }) | ||
| } | ||
|
|
||
| pub async fn set_display_name(&self, name: String) -> Result<(), ClientError> { | ||
| #[cfg(not(target_family = "wasm"))] | ||
| { | ||
| self.inner | ||
| .account() | ||
| .set_display_name(Some(name.as_str())) | ||
| .await | ||
| .context("Unable to set display name")?; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't we want to just get rid of the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The context call still works for non wasm platforms. So what it now does is use map_err on wasm and context everywhere else. That way it should be (almost) the same for both cases in behaviour. Of course map_err isn't identical to context (especially due to just using the to_string() message on wasm) but it still generally ensures the information is the same on both. But I can also just remove it in general I guess. I just thought it might make sense to preserve the context on existing bindings basically.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also its not disbled on wasm anymore. It does actually execute the action on wasm now. It just does different error handling.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, sorry I misread this part. Thanks for pointing this out. |
||
| } | ||
|
|
||
| #[cfg(target_family = "wasm")] | ||
| { | ||
| self.inner.account().set_display_name(Some(name.as_str())).await.map_err(|e| { | ||
| ClientError::Generic { | ||
| msg: "Unable to set display name".to_owned(), | ||
| details: Some(e.to_string()), | ||
| } | ||
| })?; | ||
| } | ||
|
|
||
| Ok(Arc::new(MediaFileHandle::new(handle))) | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1093,15 +1130,6 @@ impl Client { | |
| Ok(display_name) | ||
| } | ||
|
|
||
| pub async fn set_display_name(&self, name: String) -> Result<(), ClientError> { | ||
| self.inner | ||
| .account() | ||
| .set_display_name(Some(name.as_str())) | ||
| .await | ||
| .context("Unable to set display name")?; | ||
| Ok(()) | ||
| } | ||
|
|
||
| pub async fn upload_avatar(&self, mime_type: String, data: Vec<u8>) -> Result<(), ClientError> { | ||
| let mime: Mime = mime_type.parse()?; | ||
| self.inner.account().upload_avatar(&mime, data).await?; | ||
|
|
@@ -2453,25 +2481,25 @@ fn gen_transaction_id() -> String { | |
|
|
||
| /// A file handle that takes ownership of a media file on disk. When the handle | ||
| /// is dropped, the file will be removed from the disk. | ||
| #[cfg(not(target_family = "wasm"))] | ||
| #[derive(uniffi::Object)] | ||
| pub struct MediaFileHandle { | ||
| #[cfg(not(target_family = "wasm"))] | ||
| inner: std::sync::RwLock<Option<SdkMediaFileHandle>>, | ||
| } | ||
|
|
||
| #[cfg(not(target_family = "wasm"))] | ||
| impl MediaFileHandle { | ||
| #[cfg(not(target_family = "wasm"))] | ||
| fn new(handle: SdkMediaFileHandle) -> Self { | ||
| Self { inner: std::sync::RwLock::new(Some(handle)) } | ||
| } | ||
| } | ||
|
|
||
| #[cfg(not(target_family = "wasm"))] | ||
| #[matrix_sdk_ffi_macros::export] | ||
| impl MediaFileHandle { | ||
| /// Get the media file's path. | ||
| pub fn path(&self) -> Result<String, ClientError> { | ||
| Ok(self | ||
| #[cfg(not(target_family = "wasm"))] | ||
| return Ok(self | ||
| .inner | ||
| .read() | ||
| .unwrap() | ||
|
|
@@ -2480,24 +2508,37 @@ impl MediaFileHandle { | |
| .path() | ||
| .to_str() | ||
| .unwrap() | ||
| .to_owned()) | ||
| .to_owned()); | ||
| #[cfg(target_family = "wasm")] | ||
| Err(ClientError::Generic { | ||
| msg: "MediaFileHandle.path() is not supported on WASM targets".to_string(), | ||
| details: None, | ||
| }) | ||
| } | ||
|
|
||
| pub fn persist(&self, path: String) -> Result<bool, ClientError> { | ||
| let mut guard = self.inner.write().unwrap(); | ||
| Ok( | ||
| match guard | ||
| .take() | ||
| .context("MediaFileHandle was already persisted")? | ||
| .persist(path.as_ref()) | ||
| { | ||
| Ok(_) => true, | ||
| Err(e) => { | ||
| *guard = Some(e.file); | ||
| false | ||
| } | ||
| }, | ||
| ) | ||
| #[cfg(not(target_family = "wasm"))] | ||
| { | ||
| let mut guard = self.inner.write().unwrap(); | ||
| Ok( | ||
| match guard | ||
| .take() | ||
| .context("MediaFileHandle was already persisted")? | ||
| .persist(path.as_ref()) | ||
| { | ||
| Ok(_) => true, | ||
| Err(e) => { | ||
| *guard = Some(e.file); | ||
| false | ||
| } | ||
| }, | ||
| ) | ||
| } | ||
| #[cfg(target_family = "wasm")] | ||
| Err(ClientError::Generic { | ||
| msg: "MediaFileHandle.persist() is not supported on WASM targets".to_string(), | ||
| details: None, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.