Skip to content
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

Android assets not loaded as expected with custom protocol #846

Closed
1 of 2 tasks
lily-mosquitoes opened this issue Jan 15, 2023 · 12 comments · Fixed by #854 or #869
Closed
1 of 2 tasks

Android assets not loaded as expected with custom protocol #846

lily-mosquitoes opened this issue Jan 15, 2023 · 12 comments · Fixed by #854 or #869

Comments

@lily-mosquitoes
Copy link
Contributor

Describe the bug
When using custom protocol to load assets on android, they are not loaded automatically as suggested by documentation.

The with_custom_protocol docs suggests that handling assets on the closure is unnecessary as android handles its own assets path:

On Android, We only handle the scheme name and ignore the closure. (...) Android has assets and resource path finder to locate your files in those directories.

However, using simply wry://assets/index.html, leaving the closure to return an empty response (i.e.: Ok(Response::builder().body(Vec::new().into())?)) does not work, nothing is shown (whereas not using the custom protocol, but instead file:///android_asset/index.html does work and loads the index.html file in the assets folder, however it is my understanding that this has limitations and it is considered an anti-pattern by android docs).

I fully assume I'm doing something wrong, so I'd love if someone could let me know how to configure the project to load assets and I'm willing to contribute to the documentation.

Steps To Reproduce
(Assuming toolchain for android development is installed following the hackmd notes)
Install tauri-mobile v0.1.4
mkdir testapp && cd testapp
cargo mobile init
create a minimal index.html inside the assets folder.
change code from .with_url("https://tauri.app")? to .with_url("wry://assets/index.html")?.
connect android device or run emulator.
run cargo android run.

Expected behavior
Expected to get the contents of index.html to show in webview window. Got completely blank page.

Screenshots
When trying to load assets as suggested by wry documentation:
Screenshot_2023-01-15_17-51-14

When using file:///, not recommended by android documentation:
Screenshot_2023-01-15_17-52-18

Platform and Versions (please complete the following information):
OS: Compiling on Debian GNU/Linux 11 (bullseye).
Targets tested: aarch64-linux-android (device Sony XA2 H4113) and x86_64-linux-android (android emulator android-33).
Rustc: 1.67.0-nightly.

Would you want to assign yourself to resolve this bug?

  • Yes
  • No

Additional context
I noticed that if I set up the closure to return the contents of the index.html file (captured using include_bytes!) it also works, suggesting that the closure is in fact not ignored as suggested by the docs. I assume actually handling loading the files in the closure is the correct way to do it but I was not able to access the assets folder inside the apk from within rust, trying to call anything from ndk_glue there crashes the application.

Although this is probably the wrong approach I have also tried to mess with the kotlin code in the gen folder, to follow the android docs but had no luck. I'd need a push in the right direction to get this working properly and contribute to documentation.

@lily-mosquitoes lily-mosquitoes changed the title Assets not loaded as expected with custom protocol Andorid assets not loaded as expected with custom protocol Jan 15, 2023
@lily-mosquitoes lily-mosquitoes changed the title Andorid assets not loaded as expected with custom protocol Android assets not loaded as expected with custom protocol Jan 15, 2023
@wusyong
Copy link
Member

wusyong commented Jan 16, 2023

I pretty sure I added AssetLoader before when creating webview on Android.
Perhaps it got refactored after moving from tauri-mobile into wry.
@lucasfernog or @amrbashir Could you help explain how custom protocol works now in wry and tauri?

@lily-mosquitoes
Copy link
Contributor Author

lily-mosquitoes commented Jan 16, 2023

Thanks for the response!

I went looking for it on the tauri-mobile repo and indeed until commit 379fb22d953efaf85da484a3491c0cbc57b17ae3 there was code for handling asset loading, and it was removed on commit 1357edcfa1c2805f14807371513767991de3614b. The code in question was in the RustClient class, which resided in the template for MainActivity.kt at the time:

class RustClient(context: Context) : WebViewClient() {
    private val assetLoader = WebViewAssetLoader.Builder()
        .setDomain("tauri.mobile")
        .addPathHandler("/assets/", WebViewAssetLoader.AssetsPathHandler(context))
        .addPathHandler("/res/", WebViewAssetLoader.ResourcesPathHandler(context))
        .build()

    override fun shouldOverrideUrlLoading(view: WebView?, request: WebResourceRequest?): Boolean {
        return false
    }

    override fun onPageFinished(view: WebView?, url: String?) {
        super.onPageFinished(view, url)
        eval()
    }

    override fun shouldInterceptRequest(
        view: WebView,
        request: WebResourceRequest
    ): WebResourceResponse? {
        return handleRequest(request)
    }

    companion object {
        init {
            System.loadLibrary("{{snake-case app.name}}")
        }
    }

    private external fun eval()
    private external fun handleRequest(request: WebResourceRequest): WebResourceResponse?
}

After the commit 1357edcfa1c2805f14807371513767991de3614b it became:

class RustClient: WebViewClient() {
    override fun shouldOverrideUrlLoading(view: WebView?, request: WebResourceRequest?): Boolean {
        return false
    }

    override fun shouldInterceptRequest(
        view: WebView,
        request: WebResourceRequest
    ): WebResourceResponse? {
        return handleRequest(request)
    }

    companion object {
        init {
            System.loadLibrary("{{snake-case app.name}}")
        }
    }

    private external fun handleRequest(request: WebResourceRequest): WebResourceResponse?
}

Now, however, this is all gone, I can see this code lives now in a function called RustWebViewClient, but I can't find the template for that in dev. Ii looks similar but does not have the shouldOverrideUrlLoading function call.

How are the files that live in gen/android/{{app.name}}/app/src/main/kotlin/{{dot-to-slash (reverse-domain app.domain)}}/ being generated? I can only see the handlebars template for MainActivity there now but when doing cargo mobile init I see many files in that location (including the one that holds RustWebViewClient).

I'd be willing to reinstate that code if I have a better understanding of where this generated code is coming from.

[EDIT]
I see now this code resides in the wry repo. May I open a PR for re-introducing this code?

Thanks for the help in sorting this issue!

@lucasfernog
Copy link
Member

I've removed automatic asset loading because Tauri loads assets from memory instead of the assets/ folder. Maybe we should expose a wry method to load an asset instead? I wouldn't do anything automatic though (like having an assets/ path in the custom protocol to load it) as it could conflict with the Tauri implementation.

@wusyong
Copy link
Member

wusyong commented Jan 17, 2023

@lucasfernog Okay I looked into a bit and looks like closure from custom protocol can be used on Android now right?
If so, we think we could just update the documentation to reflect the change.

@lily-mosquitoes
Copy link
Contributor Author

Indeed the closure works, however I could not find a way to access the files in the assets folder from there. I could not find a valid path with fs and if I try to get an instance of AssetManager from ndk via ndk_glue the program crashes, presumably because wry is already calling ndk_glue internally.

I guess we could expose an instance of AssetManager from wry, given that it should be already calling ndk_glue and NativeActivity. What do you think? I could look into it during the weekend and provide a possible implementation.

@lucasfernog
Copy link
Member

I like your idea @lily-mosquitoes. What do you think @wusyong ?

@wusyong
Copy link
Member

wusyong commented Jan 19, 2023

Oh sorry for late reply. I'm busy with lunar new year lately. I think it's fine to expose the instance. But consider it will be a platform-specific trait method. Maybe we can add it as an attribute extension like with_asset_manager or something.

Expose raw instance and require users to call JNI FFI might be difficult for them. But still I'm fine with both methods.

@lily-mosquitoes
Copy link
Contributor Author

So far, using WebViewAssetLoader on the Kotlin code worked as expected, but trying to use instead an instance of AssetManager has been resulting on a crash when I call the method open, no matter how I get the instance... I'm not too sure why as the error message is quite cryptic for me.

One of the simplest methods I've tried:

pub fn get_asset(path: &str) -> Option<Vec<u8>> {
  let activity = ndk_glue::native_activity();
  let asset_manager = activity.asset_manager();
  let path_cstring = std::ffi::CString::new(path).ok()?;
  let mut asset = asset_manager.open(&path_cstring)?;
  Some(asset.get_buffer().ok()?.to_vec())
}

Error I get:

01-22 16:24:05.129  4437  4437 W linker64: type=1400 audit(0.0:17): avc: denied { search } for name="tests" dev="dm-33" ino=65540 scontext=u:r:untrusted_app:s0:c160,c256,c512,c768 tcontext=u:object_r:shell_test_data_file:s0 tclass=dir permissive=0 app=com.example.testapp

Anyone knows what might be the cause?
The same thing happens even if I try to get it directly from the JNIEnv, piggybacking from the setup function like so:

  let asset_manager = env
    .call_method(
      activity.as_obj(),
      "getAssets",
      "()Landroid/content/res/AssetManager;",
      &[],
    )
    .unwrap()
    .l()
    .unwrap();
  let asset_manager = env.new_global_ref(asset_manager).unwrap();
  ASSET_MANGER.get_or_init(move || asset_manager);

Where ASSET_MANAGER is a OnceCell<GlobalRef>, then I tried to get the AssetManager from:

  if let Some(am) = ASSET_MANGER.get() {
    let am_ptr = am.as_obj().into_raw() as *mut _;
    let non_null_am_ptr = core::ptr::NonNull::new(am_ptr)?;
    let asset_manager = unsafe { AssetManager::from_ptr(non_null_am_ptr) };
    let path_cstring = std::ffi::CString::new(path).ok()?;
    let mut asset = asset_manager.open(&path_cstring)?;
    Some(asset.get_buffer().ok()?.to_vec())
} else { None }

On the meantime I suppose I'll try to implement an extension such as with_asset_manager as suggested by @wusyong and see if that fairs better.

@lily-mosquitoes
Copy link
Contributor Author

I was actually trying one last idea of how to get the asset by only using the JNI, but I'm struggling to find out how I can convert a Java byteArray to a rust Vec... Could someone help?

  if let Some(am) = ASSET_MANGER.get() {
    let ctx = ndk_context::android_context();
    let vm = unsafe { jni::JavaVM::from_raw(ctx.vm().cast()) }.expect("vm failed");
    let env = vm.attach_current_thread().expect("attach failed");
    let asset_manager = am.as_obj();
    let asset = env
      .call_method(
        asset_manager,
        "open",
        "(Ljava/lang/String;)Ljava/io/InputStream;",
        &[env.new_string(path).expect("string failed").into()],
      )
      .expect("env failed")
      .l()
      .expect("l failed");
    let bytes = env
      .call_method(asset, "readAllBytes", "()[B", &[])
      .expect("bytes env failed")
      .l()
      .expect("bytes l failed");
    let elements = env
      .get_byte_array_elements(bytes.into(), jni::objects::ReleaseMode::CopyBack)
      .expect("elements env failed");

    Some(\* ??? -> how to make elements into a Vec<u8>? *\))
  } else {
    None
  }

@wusyong
Copy link
Member

wusyong commented Jan 23, 2023

@lily-mosquitoes We didn't use native activity as our main activity. Native activity is very different from other activities and it also has a totally different API implementation in C. It cannot access many android API since most of them are implemented in Java.

The possible way I could think of is a config to to determine whether to use WebViewAssetLoader in shouldInterceptRequest. I'm still away from my keyboard, but my current thought is roughly like following:

  • Add WebViewAssetLoader in RustWebViewClient.
  • Define a FFI function in binding.rs like fn has_asset_loader() -> bool. It will check a static like pub static HAS_ASSET_LOADER: OnceCell<bool> = OnceCell::new(); which is set from the extension trait method.
  • Finally when calling shouldInterceptRequest in RustWebViewClient.kt, we call has_asset_loader() first to determine if we need to call WebViewAssetLoader.shouldInterceptRequest before calling handleRequest.

@lily-mosquitoes
Copy link
Contributor Author

Thank you for the direction @wusyong, I was able to finish the changes using the method you delineated. Here are a few things that I'd like to double check:

  • I have created the trait WebViewBuilderExtAndroid to build the WebView with a function with_asset_loader which takes a String representing the protocol, similar to with_custom_protocol. This is done so that the user does not need to call with_custom_protocol at all when wanting just android support, especially because it would require boilerplate for the closure, which, when wanting to use the WebViewAssetLoader will be unused. The user is given the chance to assign a String to it (instead of fixing the string to something) in order to preserve the possibility of multi-platform code using the same protocols. Let me know if this solution is acceptable.
  • For passing the domain name from rust to the WebViewAssetLoader, I've used env.new_string(domain).unwrap().into_raw(), at first I tried to avoid the unwrap but it seems to me that this function cannot actually return an Error type, only Ok. Please let me know if I'm missing something or if this is acceptable.
  • In the android_binding macro I could not find a way to construct a function with android_fn without extra arguments, I tried to look at the documentation but it seems to me that it just expects extra arguments to be present. Calling it with , [], None or any other token I've tried for the args parameter always results in compile errors:
code error
Screenshot_2023-01-27_21-37-10 Screenshot_2023-01-27_21-37-27

For now I'm giving it a useless [jboolean], which I have to ignore in the function, but it compiles normally. Is there a way to pass an empty args parameter? There is no need for any parameters in withAssetLoader, it just returns true or false.

I believe that's all the questions I have, I'll be creating a PR to accompany this issue so people can take a look at the code and documentation, and see if it is acceptable.

lily-mosquitoes added a commit to lily-mosquitoes/wry that referenced this issue Jan 28, 2023
Implements WebViewAssetLoader to load assets from the asset folder in Android when `with_asset_loader` is called in the builder. The function also sets the desired protocol for use in `with_url`, although the url must always be `<protocol>://assets/<path>`.

Refs: tauri-apps#846
@lily-mosquitoes
Copy link
Contributor Author

I can also propose an update for the template in tauri-mobile if you all believe this fix is acceptable 🚀.

wusyong added a commit that referenced this issue Feb 7, 2023
…) (#854)

* fix(android): restore asset loading functionality to android

Implements WebViewAssetLoader to load assets from the asset folder in Android when `with_asset_loader` is called in the builder. The function also sets the desired protocol for use in `with_url`, although the url must always be `<protocol>://assets/<path>`.

Refs: #846

* docs(changes): document changes for android's fix-asset-loading patch

* Refactor to prevent additional allocation

* Fix merge conflict

* Disable default target to android

* Pass zero parameter to android fns

---------

Co-authored-by: Wu Wayne <yuweiwu@pm.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants