From 90a523da7187cc3ef710c9c4fe4acc2b2ba9ffc1 Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Sun, 14 Jun 2015 18:59:04 +0200
Subject: [PATCH] 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 | 157 ++++++++++++++++++++++++++++-------------
 1 file changed, 107 insertions(+), 50 deletions(-)

diff --git a/src/msw/webview_ie.cpp b/src/msw/webview_ie.cpp
index 74d1236cb3..658bc12224 100644
--- a/src/msw/webview_ie.cpp
+++ b/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);
-    if (psaStrings != NULL)
+
+// Simple RAII wrapper for accessing SAFEARRAY<VARIANT>. This class is not specific
+// 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);
-        param->vt = VT_BSTR;
-        param->bstrVal = bstr;
+        HRESULT hr = SafeArrayAccessData(sa, (LPVOID*)&param);
+        if ( FAILED(hr) )
+        {
+            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);
     }
 }