Add better error checking and simplify wxWebViewIE::DoSetPage().

Don't ignore errors (this resulted in warnings in optimized builds because
variable "hr" containing the error code to was assigned but never used) and
don't leak memory in the (admittedly unlikely) case an error really occurs.

Also don't duplicate the code for creating a one element SAFEARRAY<VARIANT>,
extract it into a helper function.

src/msw/webview_ie.cpp

@@ -26,6 +26,8 @@
 #include "wx/msw/missing.h"
 #include "wx/filesys.h"
 #include "wx/dynlib.h"
+#include "wx/scopeguard.h"
+
 #include <initguid.h>
 #include <wininet.h>
 
@@ -141,18 +143,89 @@ void wxWebViewIE::LoadURL(const wxString& url)
     m_ie.CallMethod("Navigate", wxConvertStringToOle(url));
 }
 
-void wxWebViewIE::DoSetPage(const wxString& html, const wxString& baseUrl)
+namespace
 {
-    BSTR bstr = SysAllocString(OLESTR(""));
+
-    SAFEARRAY *psaStrings = SafeArrayCreateVector(VT_VARIANT, 0, 1);
+// Simple RAII wrapper for accessing SAFEARRAY<VARIANT>. This class is not specific
-    if (psaStrings != NULL)
+// to wxWebView at all and could be extended to work for any type and extracted
+// into a header later if it is ever needed elsewhere, but for now keep it here
+// as it is only used in MakeOneElementVariantSafeArray() which, itself, is
+// only used by DoSetPage() below.
+class wxSafeArrayVariantAccessor
+{
+public:
+    explicit wxSafeArrayVariantAccessor(SAFEARRAY* sa)
+        : m_sa(sa),
+          m_data(DoAccessData(sa))
+    {
+    }
+
+    VARIANT* GetData() const
+    {
+        return m_data;
+    }
+
+    ~wxSafeArrayVariantAccessor()
+    {
+        if ( m_data )
+            SafeArrayUnaccessData(m_sa);
+    }
+
+private:
+    static VARIANT* DoAccessData(SAFEARRAY* sa)
     {
         VARIANT *param;
-        HRESULT hr = SafeArrayAccessData(psaStrings, (LPVOID*)&param);
+        HRESULT hr = SafeArrayAccessData(sa, (LPVOID*)&param);
-        param->vt = VT_BSTR;
+        if ( FAILED(hr) )
-        param->bstrVal = bstr;
+        {
+            wxLogLastError(wxT("SafeArrayAccessData"));
+            return NULL;
+        }
 
-        hr = SafeArrayUnaccessData(psaStrings);
+        return param;
+    }
+
+    SAFEARRAY* const m_sa;
+    VARIANT* const m_data;
+
+    wxDECLARE_NO_COPY_CLASS(wxSafeArrayVariantAccessor);
+};
+
+// Helper function: wrap the given string in a SAFEARRAY<VARIANT> of size 1.
+SAFEARRAY* MakeOneElementVariantSafeArray(const wxString& str)
+{
+    SAFEARRAY* const sa = SafeArrayCreateVector(VT_VARIANT, 0, 1);
+    if ( !sa )
+    {
+        wxLogLastError(wxT("SafeArrayCreateVector"));
+        return NULL;
+    }
+
+    wxSafeArrayVariantAccessor access(sa);
+
+    VARIANT* const param = access.GetData();
+    if ( !param )
+    {
+        SafeArrayDestroy(sa);
+        return NULL;
+    }
+
+    param->vt = VT_BSTR;
+    param->bstrVal = SysAllocString(str.wc_str());
+
+    return sa;
+}
+
+} // anonymous namespace
+
+void wxWebViewIE::DoSetPage(const wxString& html, const wxString& baseUrl)
+{
+    {
+        SAFEARRAY* const psaStrings = MakeOneElementVariantSafeArray(wxString());
+        if ( !psaStrings )
+            return;
+
+        wxON_BLOCK_EXIT1(SafeArrayDestroy, psaStrings);
 
         wxCOMPtr<IHTMLDocument2> document(GetDocument());
 
@@ -161,50 +234,34 @@ void wxWebViewIE::DoSetPage(const wxString& html, const wxString& baseUrl)
 
         document->write(psaStrings);
         document->close();
-
-        SafeArrayDestroy(psaStrings);
-
-        bstr = SysAllocString(html.wc_str());
-
-        // Creates a new one-dimensional array
-        psaStrings = SafeArrayCreateVector(VT_VARIANT, 0, 1);
-        if (psaStrings != NULL)
-        {
-            hr = SafeArrayAccessData(psaStrings, (LPVOID*)&param);
-            param->vt = VT_BSTR;
-            param->bstrVal = bstr;
-            hr = SafeArrayUnaccessData(psaStrings);
-
-            document = GetDocument();
-
-            if(!document)
-                return;
-
-            document->write(psaStrings);
-
-            // SafeArrayDestroy calls SysFreeString for each BSTR
-            SafeArrayDestroy(psaStrings);
-
-            //We send the events when we are done to mimic webkit
-            //Navigated event
-            wxWebViewEvent event(wxEVT_WEBVIEW_NAVIGATED,
-                                 GetId(), baseUrl, "");
-            event.SetEventObject(this);
-            HandleWindowEvent(event);
-
-            //Document complete event
-            event.SetEventType(wxEVT_WEBVIEW_LOADED);
-            event.SetEventObject(this);
-            HandleWindowEvent(event);
-        }
-        else
-        {
-            wxLogError("wxWebViewIE::SetPage() : psaStrings is NULL");
-        }
     }
-    else
+
     {
-        wxLogError("wxWebViewIE::SetPage() : psaStrings is NULL during clear");
+        SAFEARRAY* const psaStrings = MakeOneElementVariantSafeArray(html);
+
+        if ( !psaStrings )
+            return;
+
+        wxON_BLOCK_EXIT1(SafeArrayDestroy, psaStrings);
+
+        wxCOMPtr<IHTMLDocument2> document(GetDocument());
+
+        if(!document)
+            return;
+
+        document->write(psaStrings);
+
+        //We send the events when we are done to mimic webkit
+        //Navigated event
+        wxWebViewEvent event(wxEVT_WEBVIEW_NAVIGATED,
+                             GetId(), baseUrl, "");
+        event.SetEventObject(this);
+        HandleWindowEvent(event);
+
+        //Document complete event
+        event.SetEventType(wxEVT_WEBVIEW_LOADED);
+        event.SetEventObject(this);
+        HandleWindowEvent(event);
     }
 }