Re: urlmon: Fix a few failing tests in win2k3

[Jacek Caban]

James Hawkins wrote:
> Hi,
>
> Changelog:
> - Fix a few failing tests in win2k3.

You've fixed tests on IE7, not only win2k3. It would be better to change 
Wine to behave like IE7. Also your patch lefts some variables in wrong 
state. I will send patches to fix it.

Jacek

urlmon: Fix a few failing tests in win2k3

[James Hawkins]

On Fri, Jun 6, 2008 at 1:25 PM, Jacek Caban  wrote:
> James Hawkins wrote:
> > Hi,
> >
> > Changelog:
> > - Fix a few failing tests in win2k3.
>
> You've fixed tests on IE7, not only win2k3. It would be better to change
> Wine to behave like IE7. Also your patch lefts some variables in wrong
> state. I will send patches to fix it.

Can you take a look at the urlmon:url tests as well?  I sent in a
patch for one of the failures recently, but failed to see that it
makes the tests crash in wine.

http://test.winehq.org/data/b483b680136944bdb59be89c27960906278c52b2/2003_jh...

Thanks,
James Hawkins

urlmon: Fix a few failing tests in win2k3

[Jacek Caban]

James Hawkins wrote:
> On Fri, Jun 6, 2008 at 1:25 PM, Jacek Caban  wrote:
> > James Hawkins wrote:
> > > Hi,
> > >
> > > Changelog:
> > > - Fix a few failing tests in win2k3.
> >
> > You've fixed tests on IE7, not only win2k3. It would be better to change
> > Wine to behave like IE7. Also your patch lefts some variables in wrong
> > state. I will send patches to fix it.
>
> Can you take a look at the urlmon:url tests as well?  I sent in a
> patch for one of the failures recently, but failed to see that it
> makes the tests crash in wine.
>
> http://test.winehq.org/data/b483b680136944bdb59be89c27960906278c52b2/2003_jh...

The way you tried to fix it looks good, but it has a serious problem:
>  }else todo_wine {
>      ok(IMoniker_Release(mon) == 0, "mon should be destroyed here\n");
> -
>    ok(IBindCtx_Release(bctx) == 0, "bctx should be destroyed here\n");
> -
> -
>    if(bindf & BINDF_ASYNCHRONOUS) {
> -
>        ok(IBindCtx_Release(bctx) == 1, "bctx should not be destroyed here\n");

It would be better to check if ref count is not 0 IMO.
> -
>        IBindCtx_Release(bctx); /* actually destroy it */

You can't do it! The remaining reference is not owned by us so we can't do anything with it (unless it's a leak on tests, but it's probably not). I guess urlmon will release this object after completing some async operations.
> -
>    }
> -
>    else
> -
>        ok(IBindCtx_Release(bctx) == 0, "bctx should be destroyed here\n");
> }

Jacek

urlmon: Fix a few failing tests in win2k3

[James Hawkins]

On Fri, Jun 6, 2008 at 2:08 PM, Jacek Caban  wrote:
> James Hawkins wrote:
> > On Fri, Jun 6, 2008 at 1:25 PM, Jacek Caban  wrote:
> > > James Hawkins wrote:
> > > > Hi,
> > > >
> > > > Changelog:
> > > > - Fix a few failing tests in win2k3.
> > >
> > > You've fixed tests on IE7, not only win2k3. It would be better to change
> > > Wine to behave like IE7. Also your patch lefts some variables in wrong
> > > state. I will send patches to fix it.
> >
> > Can you take a look at the urlmon:url tests as well?  I sent in a
> > patch for one of the failures recently, but failed to see that it
> > makes the tests crash in wine.
> >
> > http://test.winehq.org/data/b483b680136944bdb59be89c27960906278c52b2/2003_jh...
>
> The way you tried to fix it looks good, but it has a serious problem:
> > }else todo_wine {
> >     ok(IMoniker_Release(mon) == 0, "mon should be destroyed here\n");
> > -
> >    ok(IBindCtx_Release(bctx) == 0, "bctx should be destroyed
> >
> > here\n");
> > -
> > -
> >    if(bindf & BINDF_ASYNCHRONOUS) {
> > -
> >        ok(IBindCtx_Release(bctx) == 1, "bctx should not be destroyed
> >
> > here\n");
>
> It would be better to check if ref count is not 0 IMO.
> > -
> >        IBindCtx_Release(bctx); /* actually destroy it */
>
> You can't do it! The remaining reference is not owned by us so we can't do
> anything with it (unless it's a leak on tests, but it's probably not). I
> guess urlmon will release this object after completing some async
> operations.
> > -
> >    }
> > -
> >    else
> > -
> >        ok(IBindCtx_Release(bctx) == 0, "bctx should be destroyed
> >
> > here\n");
> >     }

Do you want to send a better patch or should I resend?  There are some
other failures as well that I think you would know how to fix more
correctly.

urlmon: Fix a few failing tests in win2k3

[Jacek Caban]

James Hawkins wrote:
> Do you want to send a better patch or should I resend?

Please resend it.
> There are some other failures as well that I think you would know how to fix more correctly.

Most failures are due to differences between IE6 and IE7. I occasionally 
fix them and change Wine to follow changes in IE. I will try to spend 
some time on tests soon.

Jacek