Hi Jacek,
On 21/02/2020 19:31, Jacek Caban wrote:
> On 21.02.2020 13:55, Gabriel Ivăncescu wrote:
>> Hi Jacek,
>>
>> On 20/02/2020 22:41, Jacek Caban wrote:
>>> Hi Gabriel,
>>>
>>> First of all, this patch breaks mshtml script tests. It's likely that
>>> it's mshtml's fault, but we need to get it working.
>>>
>>
>> Ok I'll take a look into it, didn't know it could.
>
>
> Yeah, mshtml is the main and most complex active script user we have in
> tree. It can host multiple script with in multiple languages, so
> vbscript can call jscript, etc. All script languages share global
> namespace and may call each other. This is done by mshtml window object
> exposing properties of all hosted script engines. To make it a bit more
> complicated, there may be multiple frames (like <frame> and <iframe>
> elements) and scripts in these don't share script engine nor global
> namespace with other frames but may access them using DOM API. The code
> doing that in mshtml could use some work, but I'm not sure if this part
> is responsible for the problem, only testing could tell.
>
>
> On top of that native IE since version 9 rewrote JavaScript engine. In
> recent version, jscript.dll is not used for mshtml. To support new
> features that are present in native jscript.dll, we extended active
> script versioning API to add new flags that control those features. This
> is still work in progress. Quite a few additional APIs are implemented,
> but a lot more are missing. The main known problem is how objects are
> handling because. Making mshtml objects behave more like JavaScript
> objects needs a better mechanism than IDispatchEx. I have some ideas
> about that, but never had time to implement it.
>
>
> Anyway, I'm saying that because what we observe in mshtml is not
> necessarily matching active script variant (because we know that native
> is not limited by active script). We have quite a few tests about how
> active scripts are handled in script.c, it should allow confirming here
> the problem is.
>
>
Thanks, it appears that there's a corner case with
SCRIPTITEM_GLOBALMEMBERS and the script dispatch, and this applies to
both jscript and vbscript, so mshtml doesn't seem to be wrong here. I
will of course add tests to both of them to show this behavior.
While investigating this I also found out another behavior for
SCRIPTITEM_CODEONLY (again for both jscript and vbscript), namely that
the 'this' or 'me' returns the item's disp, and only returns the
script_obj if the flag is set.
I'll do some more testing just to make sure I'm not missing something,
but I have a general idea of how to fix both cleanly.
>>> On 19.02.2020 17:38, Gabriel Ivăncescu wrote:
>>>> -void enter_script(script_ctx_t *ctx, jsexcept_t *ei)
>>>> +HRESULT enter_script(script_ctx_t *ctx, named_item_t *item,
>>>> jsexcept_t *ei)
>>>> {
>>>> + HRESULT hres;
>>>> +
>>>> memset(ei, 0, sizeof(*ei));
>>>> + if(item) {
>>>> + if(!item->script_obj) {
>>>> + hres = create_named_item_script_obj(ctx, item);
>>>> + if(FAILED(hres)) return hres;
>>>> + }
>>>
>>>
>>> Can we have it in exec_source instead? If nothing else, it would
>>> simplify enter_script error handling.
>>>
>>
>> I think it can, yes. Do you mean to also remove it from leave_script,
>> right? (I mean, either set it in exec_source and unset at the end of
>> the function, or find a way to retrieve it otherwise -- currently I
>> use a "stack", but that's probably not necessary)
>
>
> I hope we can just get rid of item_context from script context, which
> would make leave_script change obsolete.
>
Yeah, I got rid of it, and now I obtain it from ctx->ei->named_item.
The reason I need to store it in the ei (along with the "stack" it
creates on each enter_script/leave_script) is because it has to be
accessed from many parts, not just exec_source, but also 'eval',
compile_script, InvokeEx, and we can't pass a parameter from there.
However, it doesn't need any changes to enter_script/leave_script
anymore, so it's much cleaner.
>
>>
>>>
>>>> DWORD flags;
>>>> LPWSTR name;
>>>> @@ -213,6 +215,7 @@ typedef struct named_item_t {
>>>> } named_item_t;
>>>> named_item_t*lookup_named_item(script_ctx_t*,const
>>>> WCHAR*,unsigned) DECLSPEC_HIDDEN;
>>>> +void release_named_item(named_item_t*) DECLSPEC_HIDDEN;
>>>> typedef struct {
>>>> const WCHAR *name;
>>>> @@ -243,6 +246,7 @@ struct jsdisp_t {
>>>> DWORD prop_cnt;
>>>> dispex_prop_t *props;
>>>> script_ctx_t *ctx;
>>>> + named_item_t *named_item;
>>>
>>>
>>> I'd rather avoid having it here. I think that we should almost always
>>> be getting it from bytecode. In some corner cases, jsexcept should be
>>> enough to fill the gap.
>>>
>>
>> The only issue I had when I tried without it is in
>> dispex.c/DispatchEx_InvokeEx. Is there another way to retrieve it, or
>> the bytecode, in there?
>
>
> See below.
>
>
>>>
>>>> jsdisp_t *prototype;
>>>> @@ -433,6 +437,7 @@ struct _script_ctx_t {
>>>> DWORD last_match_length;
>>>> jsdisp_t *global;
>>>> + jsdisp_t *item_context;
>>>
>>>
>>> Same here, do we really need it?
>>>
>>
>> Probably not, but I'm not that familiar with the code. What's the
>> easiest or best way to retrieve it from the bytecode? From call_ctx?
>> Or perhaps from the ei?
>
>
> When you're in interpreter, yes. Otherwise, it depends. Once you get rid
> of variables I mentioned and start using context from bytecode, it
> should become clear what we need to worry about. I sense that the
> problematic areas will be where builtin functions need to parse source.
>
Right, currently I have it so the context from bytecode sets the
ei->named_item in exec_source at the start and also creates the script
dispatch on demand.
The ei->named_item is also set before compiling the script in
ParseScriptText/ParseProcedureText, but it is automatically picked in
'eval' though (which also needs compilation).