Re: [PATCH v2 0/12] MR2691: mshtml: More ref leak fixes.

[Jacek Caban (＠jacek)]

Jacek Caban (@jacek) commented about dlls/mshtml/nsembed.c:
> - nsIThreadManager *threadmgr;
> - nsIThread *thread;
> - nsresult nsres;
> - cpp_bool b;
> -
> - nsres = nsIServiceManager_GetServiceByContractID(pServMgr, NS_THREADMANAGER_CONTRACTID, &IID_nsIThreadManager, (void**)&threadmgr);
> - if(NS_FAILED(nsres))
> -
>    return;
> -
> - nsres = nsIThreadManager_GetCurrentThread(threadmgr, &thread);
> - nsIThreadManager_Release(threadmgr);
> - if(NS_FAILED(nsres))
> -
>    return;
> -
> - do {
> -
>    nsres = nsIThread_ProcessNextEvent(thread, FALSE, &b);

This looks hackish. For example, releasing HTML document may happen while processing other document's script. If that's the case, you'd flush other document's events, which does not seem desired.

More generally, I'm not sure what you're trying to fix. We try to force CC run when releasing a document is released because it's nice if it's able to collect objects at that point. But if CC fails to clean up things right away, it's also fine as long as it will clean up things on later run. The next two commits seem similarly controversial.