Re: [PATCH 1/3] browseui: Add IOleWindow to IProgressDialog

[Nikolay Sivov]

On 4/15/2012 23:08, Detlef Riekenberg wrote:
> Used by apps to adjust the dialog position
> or remove the cancel button before vista
>
> --
> By by ... Detlef
>
> dlls/browseui/progressdlg.c |   69 ++++++++++++++++++++++++++++++++++++++++++-
>   1 files changed, 68 insertions(+), 1 deletions(-)
>
> diff --git a/dlls/browseui/progressdlg.c b/dlls/browseui/progressdlg.c
> index 9b970e2..2807622 100644
> --- a/dlls/browseui/progressdlg.c
> +++ b/dlls/browseui/progressdlg.c
> @@ -60,6 +60,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(browseui);
>
> typedef struct tagProgressDialog {
>       IProgressDialog IProgressDialog_iface;
> - IOleWindow IOleWindow_iface;
>  LONG refCount;
>  CRITICAL_SECTION cs;
>  HWND hwnd;
>
> @@ -79,6 +80,11 @@ static inline ProgressDialog *impl_from_IProgressDialog(IProgressDialog *iface)
>       return CONTAINING_RECORD(iface, ProgressDialog, IProgressDialog_iface);
>   }
>
> +static inline ProgressDialog *impl_from_IOleWindow(IOleWindow *iface)
> +{
> - return CONTAINING_RECORD(iface, ProgressDialog, IOleWindow_iface);
>
> +}
> - static void set_buffer(LPWSTR *buffer, LPCWSTR string)
> {
>     static const WCHAR empty_string[] = {0};
>
> @@ -270,9 +276,20 @@ static HRESULT WINAPI ProgressDialog_QueryInterface(IProgressDialog *iface, REFI
>       ProgressDialog *This = impl_from_IProgressDialog(iface);
>       *ppvOut = NULL;
> - if (IsEqualIID(iid,&IID_IUnknown) || IsEqualIID(iid,&IID_IProgressDialog))
> - if (IsEqualIID(iid,&IID_IUnknown))
>  {
>      *ppvOut = This;
> -
>    TRACE("(%p, IID_IUnknown, %p) ->  %p\n", This, ppvOut, This);
> - }
> - else if (IsEqualIID(iid,&IID_IProgressDialog))
> - {
> -
>    *ppvOut = This;
> -
>    TRACE("(%p, IID_IProgressDialog, %p) ->  %p\n", This, ppvOut, This);
> - }
> - else if (IsEqualIID(iid,&IID_IOleWindow))
> - {
> -
>    *ppvOut =&This->IOleWindow_iface;
> -
>    TRACE("(%p, IID_IOleWindow, %p) ->  %p\n", This, ppvOut, *ppvOut);
> }

What's a point in different traces? A single one is enough, with printed 
riid of course.
Also please return iface instead of This, like *ppvOut = iface;, or if 
you prefer instance pointer add a pointer to
interface like - &This->...
>   if (*ppvOut)
>
> @@ -495,6 +512,55 @@ static const IProgressDialogVtbl ProgressDialogVtbl =
>       ProgressDialog_Timer
>   };
>
> +static HRESULT WINAPI OleWindow_QueryInterface(IOleWindow *iface, REFIID iid, LPVOID *ppvOut)
> +{
> - ProgressDialog *This = impl_from_IOleWindow(iface);
> -
> - TRACE("(%p, %s, %p)\n", iface, debugstr_guid(iid), ppvOut);
> - return ProgressDialog_QueryInterface(&This->IProgressDialog_iface, iid, ppvOut);
>
> +}
> -

This trace should go to ProgressDialog_QueryInterface(), no need to 
place it in every interface.