Hi Andreas, It's mostly looking good, but some details still need a bit more work. On 5/6/19 10:21 PM, Andreas Maier wrote:
Signed-off-by: Andreas Maier <staubim(a)quantentunnel.de> --- dlls/jscript/enumerator.c | 227 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 216 insertions(+), 11 deletions(-)
diff --git a/dlls/jscript/enumerator.c b/dlls/jscript/enumerator.c index aa7737ac52..a7cad81c8a 100644 --- a/dlls/jscript/enumerator.c +++ b/dlls/jscript/enumerator.c @@ -26,6 +26,11 @@ WINE_DEFAULT_DEBUG_CHANNEL(jscript);
typedef struct { jsdisp_t dispex; + BOOL atend; + /* IEnumVARIANT returned by _NewEnum */ + IEnumVARIANT *enumvar; + /* IEnumVARIANT current item */ + VARIANT* enumitm;
There is no need to allocate enumitm (BTW, maybe call it just 'item'?) separately. You could just store the structure inside EnumeratorInstance. I would also suggest storing it as jsval_t. If you just set it to jsval_undefined() when you're at the end, it could simplify things even more.
} EnumeratorInstance;
static const WCHAR atEndW[] = {'a','t','E','n','d',0}; @@ -33,43 +38,156 @@ static const WCHAR itemW[] = {'i','t','e','m',0}; static const WCHAR moveFirstW[] = {'m','o','v','e','F','i','r','s','t',0}; static const WCHAR moveNextW[] = {'m','o','v','e','N','e','x','t',0};
+static inline EnumeratorInstance *enumerator_from_jsdisp(jsdisp_t *jsdisp) +{ + return CONTAINING_RECORD(jsdisp, EnumeratorInstance, dispex); +} + +static inline EnumeratorInstance *enumerator_from_vdisp(vdisp_t *vdisp) +{ + return enumerator_from_jsdisp(vdisp->u.jsdisp); +} + +static inline HRESULT enumvar_get_next_item( + IEnumVARIANT* enumvar, VARIANT* enumitm, + BOOL* atend, jsval_t *r)
I expect that passing EnumeratorInstance pointer instead of its fields separately would make things easier.
+{ + HRESULT hres; + + /* not at end ... get next item */ + if (!(*atend)) + { + /* Assume valid variant */ + VariantClear(enumitm); + hres = IEnumVARIANT_Next(enumvar, 1, enumitm, NULL); + if (hres != S_OK) + { + VariantClear(enumitm); + *atend = TRUE; + } + } + + if (*atend) + { + if (r) + *r = jsval_undefined(); + return S_OK; + } + + if (r) + return variant_to_jsval(enumitm, r); + + return S_OK; +} + static void Enumerator_destructor(jsdisp_t *dispex) { + EnumeratorInstance *This; + TRACE("Enumerator_destructor\n");
+ This = enumerator_from_jsdisp(dispex); + + if (This->enumitm) + { + VariantClear(This->enumitm); + heap_free(This->enumitm); + } + heap_free(dispex); }
HRESULT Enumerator_atEnd(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - TRACE("Enumerator_atEnd\n"); + EnumeratorInstance *This; + + This = enumerator_from_vdisp(jsthis);
- return E_NOTIMPL; + if (r) + *r = jsval_bool(This->atend); + + TRACE("Enumerator_atEnd %d\n", This->atend);
I missed it in already committed patch, but please don't add function names to debug traces. Wine will add it for you.
+ + return S_OK; }
HRESULT Enumerator_item(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { + EnumeratorInstance *This; + HRESULT hres = E_FAIL; + TRACE("Enumerator_item\n");
- return E_NOTIMPL; + This = enumerator_from_vdisp(jsthis);
You need to make sure that This is actually an enumerator instance. JavaScript allows passing arbitrary values as this with something like Enumerator.prototype.item.call(new Object()); See how it's handled in bool.c for an example (and don't worry about exact error exceptions, FIXME(); return E_NOTIMPL; is fine for the first iteration). Same applies to other functions.
+ + if (This->enumvar) + { + hres = variant_to_jsval(This->enumitm, r); + } + else + { + if (r) + *r = jsval_undefined(); + hres = S_OK; + } + + return hres;
If you store the value as I suggested, it could be just: return r ? jsval_copy(This->item, r) : S_OK;
}
HRESULT Enumerator_moveFirst(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { + EnumeratorInstance *This; + HRESULT hres = E_FAIL; + TRACE("Enumerator_moveFirst\n");
- return E_NOTIMPL; + This = enumerator_from_vdisp(jsthis); + if (This->enumvar) + { + IEnumVARIANT_Reset(This->enumvar);
We should probably check for Reset() error.
+ hres = enumvar_get_next_item(This->enumvar, This->enumitm, &This->atend, r);
Does it really return the item value here? It would need a test, but I'd expect it to return undefined value.
+ } + else + { + if (r) + *r = jsval_undefined(); + hres = S_OK; + } + + return hres; }
HRESULT Enumerator_moveNext(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { + EnumeratorInstance *This; + HRESULT hres = E_FAIL; + TRACE("Enumerator_moveNext\n");
- return E_NOTIMPL; + This = enumerator_from_vdisp(jsthis); + + if (This->atend) + { + if (r) + *r = jsval_undefined(); + return S_OK; + } + if (This->enumvar) + { + hres = enumvar_get_next_item(This->enumvar, This->enumitm, &This->atend, r);
Same here, should it return the value?
@@ -180,11 +301,95 @@ HRESULT create_enumerator(script_ctx_t *ctx, jsval_t *argv, jsdisp_t **ret) { EnumeratorInstance *enumerator; HRESULT hres; + IDispatch *obj; + DISPPARAMS dispparams = {NULL, NULL, 0, 0}; + VARIANT varresult; + + BOOL atend = TRUE; + IEnumVARIANT *enumvar = NULL; + VARIANT *enumitm = NULL; + + memset(&varresult, 0, sizeof(VARIANT)); + VariantInit(&varresult); + + /* new Enumerator() */ + if (argv == NULL) + { + atend = TRUE; + } + else if(is_object_instance(*argv)) + { + obj = get_object(*argv); + + /* Try to get a IEnumVARIANT by _NewEnum */ + hres = IDispatch_Invoke(obj, + DISPID_NEWENUM, &IID_NULL, LOCALE_NEUTRAL, + DISPATCH_METHOD, &dispparams, &varresult, + NULL, NULL); + if (FAILED(hres)) + { + ERR("Enumerator: no DISPID_NEWENUM.\n"); + hres = E_INVALIDARG; + goto cleanuperr; + } + + if (V_VT(&varresult) == VT_DISPATCH) + { + hres = IUnknown_QueryInterface(V_DISPATCH(&varresult), + &IID_IEnumVARIANT, (void**)&enumvar); + } + else if (V_VT(&varresult) == VT_UNKNOWN) + { + hres = IUnknown_QueryInterface(V_UNKNOWN(&varresult), + &IID_IEnumVARIANT, (void**)&enumvar); + }
Note that IDispatch inherits from IUnknown, so you could just handle them as a single case and use V_UNKNOWN. Thanks, Jacek