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

feat(windows): add sharedbuffer implement and example #1115

Closed
wants to merge 1 commit into from

Conversation

defims
Copy link

@defims defims commented Dec 5, 2023

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes
  • No

Checklist

Other information

Two methods, webview.create_shared_buffer() and webview.post_shared_buffer_to_script(), were added to the WebViewExtWindows trait, Since these are two Windows-specific methods.
Because manual memory management is risky, the new methods do not provide any additional helper functions to manage memory. Users are responsible for managing memory themselves.

@defims defims requested a review from a team as a code owner December 5, 2023 09:00
@defims defims changed the title feat: add shared_buffer implement and example feat: add sharedbuffer implement and example Dec 5, 2023
@defims defims changed the title feat: add sharedbuffer implement and example feat(windows): add sharedbuffer implement and example Dec 5, 2023
@defims defims force-pushed the feat/windows/sharedbuffer branch from b8c73f7 to 5c99ea1 Compare December 7, 2023 08:57
@dklassic
Copy link
Contributor

Hey @defims thanks for contributing! Though we have enforced commit signing so I'll need you to sign your commit before we can merge it.

@defims defims force-pushed the feat/windows/sharedbuffer branch from 9f95014 to 4b63d82 Compare December 11, 2023 14:00
@defims
Copy link
Author

defims commented Dec 11, 2023

Hey @defims thanks for contributing! Though we have enforced commit signing so I'll need you to sign your commit before we can merge it.

@dklassic My pleasure. I have resubmitted it and signed the commit. Please check it again.😊

@amrbashir
Copy link
Member

Same as #1109 this PR is only exposing APIs that can be accessed through webview.controller and I don't see any value in adding it here tbh.

@defims
Copy link
Author

defims commented Dec 12, 2023

Same as #1109 this PR is only exposing APIs that can be accessed through webview.controller and I don't see any value in adding it here tbh.

@amrbashir Thank you for your review.
SharedBuffer is a zero-copy IPC method that has the following benefits:

  • Reduces memory usage
  • Lowers I/O overhead
  • Improves communication performance
  • Facilitates communication of large amounts of data (e.g., printing)

However, as you are concerned, it is a high-cost method with few applicable scenarios. It is currently only available on Windows.

For these reasons, I have not wrapped related methods too much. Instead, I only expose basic APIs on WebViewExtWindows. This allows us to cover a small number of applicable scenarios with the lowest maintenance cost. We do not need to consider the burden of use too much, as users can manage the burden themselves.

WKWebView and WebKitGTK currently do not have similar APIs. The cost of simulating them with existing interfaces is too high, so they have not been added yet. If the relevant API documentation is provided, I will be happy to add them and encapsulate them into a unified interface.

@amrbashir
Copy link
Member

amrbashir commented Dec 12, 2023

I meant that these newly exposed APIs can just be accessed by doing the following without adding it explicitly in wry:

let controller = webview.controller();
let webview2 = controller.CoreWebview2().unwrap().cast::<ICoreWebView2_19>().unwrap();
let environment = webview2.Environment().unwrap().cast::<ICoreWebView2Environment12>().unwrap();

// use webview2 & environment to create and post the shared buffer

Same goes for #1109

@defims
Copy link
Author

defims commented Dec 12, 2023

I meant that these newly exposed APIs can just be accessed by doing the following without adding it explicitly in wry:

let controller = webview.controller();
let webview2 = controller.CoreWebview2().unwrap().cast::<ICoreWebView2_19>().unwrap();
let environment = webview2.Environment().unwrap().cast::<ICoreWebView2Environment12>().unwrap();

// use webview2 & environment to create and post the shared buffer

Same goes for #1109

Wow! That's great. I didn't notice this method before. This method can get the webview2 and environment, which means we can call the relevant webview2 methods. This pull request can be merged now. Thanks!

I tried it, and it worked. 🎉 Here is the test code:

use windows::ComInterface;
let controller = webview.controller();
let webview2 = unsafe { controller.CoreWebView2() }.unwrap().cast::<ICoreWebView2_19>().unwrap();
let environment = unsafe { webview2.Environment() }.unwrap().cast::<ICoreWebView2Environment12>().unwrap();
let _ = unsafe { webview2.AddHostObjectToScript(w!("i32"), &mut i32_variant.0) };
let shared_buffer = unsafe { webview.create_shared_buffer(1024) }.unwrap());
let _ = unsafe { environment.PostSharedBufferToScript(sharedbuffer, access, additionaldataasjson) };

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add PostSharedBufferToScript for making streaming possible
3 participants