views:

66

answers:

3

I am working with a webbrowser host on c++, I managed to sink event and I am running this void on DISPID_DOCUMENTCOMPLETE:

void DocumentComplete(LPDISPATCH pDisp, VARIANT *url)
{
    READYSTATE rState;

    iBrowser->get_ReadyState(&rState);

    if(rState == READYSTATE_COMPLETE)
    {
        HRESULT hr;
        IDispatch *pHtmlDoc = NULL;
        IHTMLDocument2 *pDocument = NULL;
        IHTMLElement *pBody = NULL;
        IHTMLElement *lpParentElm = NULL;
        BSTR bstrHTMLText;

        hr = iBrowser->get_Document(&pHtmlDoc);

        hr = pHtmlDoc->QueryInterface(IID_IHTMLDocument2, (void**)&pDocument);

        if( (FAILED(hr)) || !pDocument)
        {
            MessageBox(NULL, "QueryInterface failed", "WebBrowser", MB_OK);
        }

        hr = pDocument->get_body( &pBody );

        if( (!SUCCEEDED(hr)) || !pBody)
        {
            MessageBox(NULL, "get_body failed", "WebBrowser", MB_OK);
        }

        pBody->get_parentElement(&lpParentElm);

        lpParentElm->get_outerHTML(&bstrHTMLText);

        _bstr_t bstr_t(bstrHTMLText);

        std::string sTemp(bstr_t);

        MessageBox(NULL, sTemp.c_str(), "WebBrowser", MB_OK);
    }
}

I don't much about c++, I built this code from watching other codes in google. Now I know I have to use ->Release, but do I have to use all these?:

    pHtmlDoc->Release();
    pDocument->Release();
    pBody->Release();
    lpParentElm->Release();
    iBrowser->Release();

Because on the examples I used to build my code it was using it only for the IHTMLElement(s).

+4  A: 

Yes, you do have to call Release() on those pointers, otherwise objects will leak. The same goes for BSTRs.

You'll be much better off if you use smart pointers for that - ATL::CComPtr/ATL::CComBSTR or _com_ptr_t/_bstr_t.

sharptooth
You don't want to release `iBrowser`, unless this is the last thing you're going to do with it - `iBrowser` lives outside of the scope of this function.
RichieHindle
@RichieHindle: Yes, you're right.
sharptooth
I need to include atlbase.h to use CComPtr right? Does this makes my compiled app to have some extra dll dependencie or something like that?
jarkam
@jarkam: If you only use `CComPtr` you won't have extra dependencies - the class is templated and so you'll have code statically linked into your executable.
sharptooth
+3  A: 

You should wrap those objects into a CComPtr or one of its variants. That will handle the release for you. It goes with the concept of RAII.

wheaties
+1  A: 

Yes you do. But not on the iBrowser, you didn't acquire that pointer inside this code.

Beware that your error checking isn't sufficient, your code will bomb when get_Document() fails. Same for get_parentElement(). And after the message box is dismissed.

Hans Passant