[PATCH 2/7] ieframe: Implement IOleObject::Advise().

[Dmitry Timoshkov]

Based on ole32/defaulthandler.c implementation.

Signed-off-by: Dmitry Timoshkov 
---
 dlls/ieframe/ieframe.h    |  2 ++
 dlls/ieframe/oleobject.c  | 15 ++++++++++++---
 dlls/ieframe/webbrowser.c |  3 +++
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/dlls/ieframe/ieframe.h b/dlls/ieframe/ieframe.h
index 8d238863be..329cea2f7c 100644
--- a/dlls/ieframe/ieframe.h
+++ b/dlls/ieframe/ieframe.h
@@ -199,6 +199,8 @@ struct WebBrowser {
     DWORD sink_aspects;
     DWORD sink_flags;

+    IOleAdviseHolder *advise_holder;
+
     /* window context */

HWND frame_hwnd;
diff --git a/dlls/ieframe/oleobject.c b/dlls/ieframe/oleobject.c
index 28ead0f687..39acd43028 100644
--- a/dlls/ieframe/oleobject.c
+++ b/dlls/ieframe/oleobject.c
@@ -760,11 +760,20 @@ static HRESULT WINAPI OleObject_GetExtent(IOleObject *iface, DWORD dwDrawAspect,
 }

static HRESULT WINAPI OleObject_Advise(IOleObject *iface, IAdviseSink *pAdvSink,
-        DWORD* pdwConnection)
+        DWORD *pdwConnection)
 {
     WebBrowser *This = impl_from_IOleObject(iface);
-    FIXME("(%p)->(%p, %p)\n", This, pAdvSink, pdwConnection);
-    return E_NOTIMPL;
+    HRESULT hr = S_OK;
+
+    TRACE("(%p)->(%p, %p)\n", This, pAdvSink, pdwConnection);
+
+    if(!This->advise_holder)
+        hr = CreateOleAdviseHolder(&This->advise_holder);
+
+    if(hr == S_OK)
+        hr = IOleAdviseHolder_Advise(This->advise_holder, pAdvSink, pdwConnection);
+
+    return hr;
 }

static HRESULT WINAPI OleObject_Unadvise(IOleObject *iface, DWORD dwConnection)
diff --git a/dlls/ieframe/webbrowser.c b/dlls/ieframe/webbrowser.c
index 356b0a2833..87b74b8e01 100644
--- a/dlls/ieframe/webbrowser.c
+++ b/dlls/ieframe/webbrowser.c
@@ -170,6 +170,9 @@ static ULONG WINAPI WebBrowser_Release(IUnknown *iface)
         if(This->sink)
             IAdviseSink_Release(This->sink);

+        if(This->advise_holder)
+            IOleAdviseHolder_Release(This->advise_holder);
+
         if(This->doc_host.document)
             IUnknown_Release(This->doc_host.document);

[Jacek Caban]

Hi Dmitry,

On 27.07.2020 12:32, Dmitry Timoshkov wrote:
> Based on ole32/defaulthandler.c implementation.
> Signed-off-by: Dmitry Timoshkov
>
> dlls/ieframe/ieframe.h    |  2 ++
>   dlls/ieframe/oleobject.c  | 15 ++++++++++++---
>   dlls/ieframe/webbrowser.c |  3 +++
>   3 files changed, 17 insertions(+), 3 deletions(-)

Please add some test cases. For this series, at least some basic calls 
should be easy to test. However, are those callbacks supposed to be 
called? If yes, then this is rather a "semi-stub" implementation and 
would be good to mark it as such. Ideally, we'd have the object advised 
in something like test_WebBrowser to see if/how are those callbacks used.

Thanks,

Jacek

[Dmitry Timoshkov]

Hi Jacek,

Jacek Caban  wrote:
> Please add some test cases. For this series, at least some basic calls 
> should be easy to test. However, are those callbacks supposed to be 
> called? If yes, then this is rather a "semi-stub" implementation and 
> would be good to mark it as such. Ideally, we'd have the object advised 
> in something like test_WebBrowser to see if/how are those callbacks used.

These patches implement appropriate methods similar to what ole32/defaulthandler.c
and mshtml do, and in that regard these implementations are not semi-stubs
but rather pretty complete. What is missing - is actual notifications of
the sink about particular events, and that's where the tests could be
helpful. I'll certainly look at the WebBrowser tests and see what kind
of notifications and at which events they are supposed to get, however
I hope that the patches could be accepted as is, without the tests since
they follow existing ole32/defaulthandler.c and mshtml implementations.

[Jacek Caban]

Hi Dmitry,

On 28.07.2020 17:51, Dmitry Timoshkov wrote:
> I hope that the patches could be accepted as is, without the tests since
> they follow existing ole32/defaulthandler.c and mshtml implementations.

There is no guarantee that they should follow existing implementation. 
For example I could imagine that native could return E_NOTIMPL here.

Thanks,

Jacek

[Dmitry Timoshkov]

Jacek Caban  wrote:
> On 28.07.2020 17:51, Dmitry Timoshkov wrote:
> > I hope that the patches could be accepted as is, without the tests since
> > they follow existing ole32/defaulthandler.c and mshtml implementations.
>
> There is no guarantee that they should follow existing implementation. 
> For example I could imagine that native could return E_NOTIMPL here.

I forgot to mention that I did preliminary minimal testing before writing
the code and sending it to wine-devel, and under Windows 10 implemented
methods return S_OK, otherwise I wouldn't bother spending the time writing
them in the first place.