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.
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.
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.
Thanks,
Jacek