On Thu Jun 6 15:55:43 2024 +0000, Gabriel Ivăncescu wrote:
> Actually you don't even need to query IID anymore. Just look up the
> function by dispid, and then check if it's same func_info_t, that's enough.
BTW, is there a reason you renamed `IWineJSDispatchHost` to just `IJSDispatchHost` when it's Wine's own interface? I thought it's better to avoid ambiguity with a native interface, especially if we're to extend it (obviously we can't touch native interfaces), makes it a lot more obvious.
And to add to what I said above, what I meant with "retrieving a potentially different function" is mostly theoretical, because as you said current mshtml likely has only one struct layout per interface. **But** if we were to assume this is not the case (addressing the original concern), then two objects could have same IID but different struct layouts and different functions. In such case, instead of failing, it would call the other function that was looked up, since it matches in IID and DISPID. It won't crash, but I don't think it would be quite correct, either. And it still makes the code simpler anyway.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_72601
--
v3: winegstreamer: Request the new transform output format explicitly.
winegstreamer: Use a caps to store the desired output format.
winegstreamer: Only report format changes when frontend supports it.
winegstreamer: Rename allow_size_change to allow_format_change.
winegstreamer: Enforce default stride presence in the video processor.
winegstreamer: Enforce default stride value in the video decoder.
winegstreamer: Allow to clear video decoder input/output types.
https://gitlab.winehq.org/wine/wine/-/merge_requests/5732
This reduce the chance of buffer allocating issue for Kirikiri2 games(eg. Super Naughty Maid) when the game is trying to play videos.
--
v3: qasf/dmowrapper: Sync Stop() and Receive() for dmo wrapper filter.
qasf/dmowrapper: Return VFW_E_WRONG_STATE in dmo_wrapper_sink_Receive.
qasf/dmowrapper: Allocate output samples before calling ProcessInput().
qasf/dmowrapper: Return failure in dmo_wrapper_sink_Receive if samples allocating fails.
qasf/dmowrapper: Introduce release_output_samples.
qasf/dmowrapper: Introduce get_output_samples.
qasf/tests: Add more tests for dmo_wrapper_sink_Receive.
https://gitlab.winehq.org/wine/wine/-/merge_requests/5717
On Thu Jun 6 15:51:53 2024 +0000, Gabriel Ivăncescu wrote:
> > But then you assume that all objects implementing the interface will
> use the same struct layout for that. I can't think of an example inside
> MSHTML where it's currently not the case, but that's not generally a
> promise that QI gives.
> In theory, there's a similar concern when looking up the function via
> IID and DISPID, specifically, that it might find a different function.
> In practice as you said it probably doesn't happen but…
> > So what's the point of it then? Avoiding one argument in a call?
> Continuing the above, IMO the following would make it the most robust:
> we look it up as mentioned from the func_info_t's dispid and IID, and
> then compare to see if it's the same `func_info_t` pointer. If not,
> return E_UNEXPECTED since it's the wrong function. And yes, one less
> argument to keep track of, but also one less thing to keep track of (in
> the ProxyFunction for instance, and everywhere else).
> > I think that you misinterprets your observations. Here is a test
> proving that currentStyle should have a setter that throws an error
> (unless used for modifiable style object):
> Okay, thanks for the test, I guess there's more at play here indeed.
Actually you don't even need to query IID anymore. Just look up the function by dispid, and then check if it's same func_info_t, that's enough.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_72595
Didn't notice that it was failing until I did the same for winemac with https://gitlab.winehq.org/wine/wine/-/merge_requests/5798. It was silently falling back to copies while winemac can't do that.
--
v3: gdi.exe16: Fix some incorrect usage of NtGdiDdDDICreateDCFromMemory.
winex11: Fix some incorrect usage of NtGdiDdDDICreateDCFromMemory.
winex11: Wrap more window surface formats with NtGdiDdDDICreateDCFromMemory.
https://gitlab.winehq.org/wine/wine/-/merge_requests/5799
> But then you assume that all objects implementing the interface will use the same struct layout for that. I can't think of an example inside MSHTML where it's currently not the case, but that's not generally a promise that QI gives.
In theory, there's a similar concern when looking up the function via IID and DISPID, specifically, that it might find a different function. In practice as you said it probably doesn't happen but…
> So what's the point of it then? Avoiding one argument in a call?
Continuing the above, IMO the following would make it the most robust: we look it up as mentioned from the func_info_t's dispid and IID, and then compare to see if it's the same `func_info_t` pointer. If not, return E_UNEXPECTED since it's the wrong function. And yes, one less argument to keep track of, but also one less thing to keep track of (in the ProxyFunction for instance, and everywhere else).
> I think that you misinterprets your observations. Here is a test proving that currentStyle should have a setter that throws an error (unless used for modifiable style object):
Okay, thanks for the test, I guess there's more at play here indeed.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_72591
> Yeah, I forgot to validate it in my branch. In BuiltinFuncCall, something like this would work:
But then you assume that all objects implementing the interface will use the same struct layout for that. I can't think of an example inside MSHTML where it's currently not the case, but that's not generally a promise that QI gives.
> Alternatively, we can pass around `func_info_t` instead of dispid and tid, but still look it up like in your branch.
So what's the point of it then? Avoiding one argument in a call?
> I mean the test will simply start failing if it picks `IHTMLCSSStyleDeclaration` because the assignment does not throw exception anymore (when it should be!).
I think that you misinterprets your observations. Here is a test proving that currentStyle should have a setter that throws an error (unless used for modifiable style object):
```
sync_test("currentStyle", function() {
if (document.documentMode < 9)
return;
var s = document.body.currentStyle;
var d = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(Object.getPrototypeOf(s)), "zoom");
ok(typeof(d.get) === "function", "d.get = " + d.get);
ok(typeof(d.set) === "function", "d.set = " + d.set);
d.set.call(document.body.style, "1");
try {
d.set.call(s, "1");
ok(false, "exception expected");
}catch(e) { trace("MSG: " + e.message); }
});
```
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_72588
This MR adds support for creating file mapping objects backed by large pages on Linux, by making the following changes:
## wineserver
* On Linux, `create_temp_file` will first attempt to use memfds as the backing fd. If it fails, it'll return to the current codepath, creating a temporary file in either the server or config directory.
* The created memfd will be sealed against writes, if the caller requesting the appropriate page protection flags.
* This removes the requirement that FDs be only created on filesystems/directories that aren't `noexec`.
* In the server method `create_mapping` , if large pages have been requested by the caller, hold that the calling thread's token holds `SeLockMemoryPrivilege` .
* Additionally, add `SeLockMemoryPrivilege` to the list of privileges enabled for the Administrator.
## `ntdll`
* Add `virtual_get_min_large_page_size` and its exported wrapper `wine_unix_get_min_large_page_size`.
* On Linux, the minimum page size is determined by going through `/sys/kernel/mm/hugepages`. If hugepage support was not detected, `STATUS_NOT_SUPPORTED` is returned instead. On other platforms, the older hard-coded value of 2\*1024\*1024 is returned instead.
* `NtCreateSection` will validate certain parameters if large pages are requested. Specifically, it will return STATUS_INVALID_PARAMETER if the requested mapping is not anonymous/unnamed, or the size is not a multiple of the minimum supported page size.
## `kernelbase`
* `GetLargePageMinimum` will use `wine_unix_get_min_large_page_size`.
## `kernel32/tests`
* Add new test test_large_page_file_mapping, which validates privilege enforcements and parameter validation while creating large pages backed file mapping obejcts. The tests are skipped if `GetLargePageMinimum` returns 0.
--
v13: kernel32: Add tests for large page mapping support.
ntdll: Validate parameters to NtCreateSection if large pages are requested.
server: Use memfd to back anonymous mappings on Linux.
kernelbase: Implement GetLargePageMinimum by returning the value of LargePageMinimum in _KUSER_SHARED_DATA.
server: Set LargePageMinimum in _KUSER_SHARED_DATA on Linux.
server: Require SeLockMemoryPrivilege to create large page mappings.
https://gitlab.winehq.org/wine/wine/-/merge_requests/5769