[PATCH 5/5] jscript: Clear the CODEONLY flag for persistent items when the script is reinitialized.

[Gabriel Ivăncescu]

On 24/03/2020 15:59, Gabriel Ivăncescu wrote:
> On 24/03/2020 02:34, Jacek Caban wrote:
>> Hi Gabriel,
>>
>> On 23.03.2020 14:53, Gabriel Ivăncescu wrote:
>>> diff --git a/dlls/jscript/jscript.c b/dlls/jscript/jscript.c
>>> index 3fcaab0..eb328d2 100644
>>> --- a/dlls/jscript/jscript.c
>>> +++ b/dlls/jscript/jscript.c
>>> @@ -757,6 +757,9 @@ static HRESULT WINAPI 
>>> JScript_SetScriptSite(IActiveScript *iface,
>>>               hres = retrieve_named_item_disp(pass, item);
>>>               if(FAILED(hres)) return hres;
>>>           }
>>> +
>>> +        /* For some reason, Windows clears the CODEONLY flag */
>>> +        item->flags &= ~SCRIPTITEM_CODEONLY;
>>
>>
>> This clearly shows that we're still not interpreting those flags 
>> correctly and this patch just tries to hide the problem. I think we 
>> simply shouldn't depend on the flag but rather on item->disp being 
>> set. See the attached patch (on top of your patches). It solves some 
>> of test failures that you observed. The remaining problem is that we 
>> still store IDispatch for visible items. The attached test shows that 
>> we probably shouldn't.
>>
>>
> 
> I'm not sure if that's going to work. We have tests covering a lot of 
> stuff already; if we change some of it, then some of the tests will 
> fail. If we don't store disp for visible items, so we can pass the new 
> test, then the test at line 1360 will fail.
> 
> There's also the test at line 1383. Which is same as line 1360, except 
> that it also has the CODEONLY flag (its "disp" was still retrieved 
> earlier, though). In that case, the disp is not used.
> 
> Both of them have the VISIBLE flag -- and yet only the former one 
> accesses its disp. I don't know how to solve this situation without 
> checking the flags.

So, I will end up resending the last patch as-is, with changes to the 
other tests as suggested.

The reason is that I can't figure out a way to solve this without 
actually using the flag. I tried changes to visible items (release old 
one in lookup, move it after it's looked up instead of holding another 
reference, etc) and they each made several other tests fail.

I'm assuming that, if Windows does clear the flag, it probably resets it 
for some reason (could be a bug, but vbscript also does it).


If you still think it's a bad idea, then feel free to ignore/reject just 
the last patch in the series. The other patches should work fine without 
it, with the only thing being a todo_wine added:

todo_wine CHECK_CALLED(GetIDsOfNames_persistent);

which is not that big of a deal if it's there...

And so, if the last patch is rejected, when we figure out how to solve 
this without clearing the flag, we can get rid of the todo_wine then.

Thanks,
Gabriel