Make wxBasicString safer and easier to use as a BSTR RAII wrapper.

include/wx/msw/ole/oleutils.h

@@ -66,9 +66,8 @@ inline void wxOleUninitialize()
 class WXDLLIMPEXP_CORE wxBasicString
 {
 public:    
-
     // Takes over the ownership of bstr
-    wxBasicString(BSTR bstr = NULL);
+    explicit wxBasicString(BSTR bstr = NULL);
 
     // Constructs the BSTR from wxString
     wxBasicString(const wxString& str);
@@ -79,6 +78,19 @@ public:
     // Frees the owned BSTR
     ~wxBasicString();
 
+    // Returns the owned BSTR and gives up its ownership
+    BSTR Detach();
+
+    // Frees the owned BSTR
+    void Free();
+
+    // Returns a copy of the owned BSTR,
+    // the caller is responsible for freeing it
+    BSTR Copy() const { return SysAllocString(m_bstrBuf); }
+
+    // Returns the address of the owned BSTR, not to be called
+    // when wxBasicString already contains a non-NULL BSTR
+    BSTR* ByRef();
 
     // Sets its BSTR to a copy of the BSTR owned by bstr
     wxBasicString& operator=(const wxBasicString& bstr);
@@ -89,23 +101,12 @@ public:
     // Takes over the ownership of bstr
     wxBasicString& operator=(BSTR bstr);
 
-	// Returns the owned BSTR and gives up its ownership
-	BSTR Detach();
-
     /// Returns the owned BSTR while keeping its ownership    
     operator BSTR() const { return m_bstrBuf; }
 
-	// Returns the address of the owned BSTR
-	operator BSTR*() { return &m_bstrBuf; }
-
-	// Returns a copy of the owned BSTR
-	BSTR Copy() const { return SysAllocString(m_bstrBuf); }
-	
-	
     // retrieve a copy of our string - caller must SysFreeString() it later!
     wxDEPRECATED_MSG("use Copy() instead") 
     BSTR Get() const { return Copy(); }
-
 private:
     // actual string
     BSTR m_bstrBuf;

src/msw/ole/oleutils.cpp

@@ -91,10 +91,34 @@ wxBasicString::~wxBasicString()
     SysFreeString(m_bstrBuf);
 }
 
+BSTR wxBasicString::Detach()
+{
+    BSTR bstr = m_bstrBuf;
+
+    m_bstrBuf = NULL;
+
+    return bstr;
+}
+
+void wxBasicString::Free()
+{
+    SysFreeString(m_bstrBuf);
+    m_bstrBuf = NULL;
+}
+
+BSTR* wxBasicString::ByRef()
+{
+    wxASSERT_MSG(!m_bstrBuf,
+        wxS("Can't get direct access to initialized BSTR"));
+    return &m_bstrBuf;
+}
+
 wxBasicString& wxBasicString::operator=(const wxBasicString& src)
 {
     if ( this != &src )
     {
+        wxCHECK_MSG(m_bstrBuf == NULL || m_bstrBuf != src.m_bstrBuf, 
+            *this, wxS("Attempting to assign already owned BSTR"));
         SysFreeString(m_bstrBuf);
         m_bstrBuf = src.Copy();
     }
@@ -105,14 +129,15 @@ wxBasicString& wxBasicString::operator=(const wxBasicString& src)
 wxBasicString& wxBasicString::operator=(const wxString& str)
 {
     SysFreeString(m_bstrBuf);
-	m_bstrBuf = ::SysAllocString(str.wc_str(*wxConvCurrent));
+    m_bstrBuf = SysAllocString(str.wc_str(*wxConvCurrent));
 
     return *this;
 }
 
 wxBasicString& wxBasicString::operator=(BSTR bstr)
 {
-	wxCHECK_MSG(m_bstrBuf != bstr, *this, wxS("Attempting to attach already attached BSTR!"));
+    wxCHECK_MSG(m_bstrBuf == NULL || m_bstrBuf != bstr, 
+        *this, wxS("Attempting to assign already owned BSTR"));
 
     SysFreeString(m_bstrBuf);
     m_bstrBuf = bstr;
@@ -120,15 +145,6 @@ wxBasicString& wxBasicString::operator=(BSTR bstr)
     return *this;
 }
 
-BSTR wxBasicString::Detach()
-{
-	BSTR bstr = m_bstrBuf;
-
-	m_bstrBuf = NULL;
-	
-	return bstr;
-}
-
 // ----------------------------------------------------------------------------
 // Convert variants
 // ----------------------------------------------------------------------------

src/msw/webview_ie.cpp

@@ -232,7 +232,7 @@ wxString wxWebViewIE::GetPageSource() const
             if(SUCCEEDED(hr))
             {
                 wxBasicString bstr;
-                if ( htmlTag->get_outerHTML(bstr) == S_OK )
+                if ( htmlTag->get_outerHTML(bstr.ByRef()) == S_OK )
                     source = bstr;
             }
         }
@@ -577,7 +577,7 @@ wxString wxWebViewIE::GetCurrentTitle() const
     if(document)
     {
         wxBasicString title;
-        if ( document->get_nameProp(title) == S_OK )
+        if ( document->get_nameProp(title.ByRef()) == S_OK )
             s = title;
     }
 
@@ -706,7 +706,7 @@ bool wxWebViewIE::IsEditable() const
     if(document)
     {
         wxBasicString mode;
-        if ( document->get_designMode(mode) == S_OK )
+        if ( document->get_designMode(mode.ByRef()) == S_OK )
         {
             if ( wxString(mode) == "On" )
                 return true;
@@ -732,7 +732,7 @@ bool wxWebViewIE::HasSelection() const
         if(SUCCEEDED(hr))
         {
             wxBasicString type;
-            if ( selection->get_type(type) == S_OK )
+            if ( selection->get_type(type.ByRef()) == S_OK )
                 sel = type;
         }
         return sel != "None";
@@ -768,7 +768,7 @@ wxString wxWebViewIE::GetSelectedText() const
                 if(SUCCEEDED(hr))
                 {
                     wxBasicString text;
-                    if ( range->get_text(text) == S_OK )
+                    if ( range->get_text(text.ByRef()) == S_OK )
                         selected = text;
                 }
             }
@@ -801,7 +801,7 @@ wxString wxWebViewIE::GetSelectedSource() const
                 if(SUCCEEDED(hr))
                 {
                     wxBasicString text;
-                    if ( range->get_htmlText(text) == S_OK )
+                    if ( range->get_htmlText(text.ByRef()) == S_OK )
                         selected = text;
                 }
             }
@@ -842,7 +842,7 @@ wxString wxWebViewIE::GetPageText() const
         if(SUCCEEDED(hr))
         {
             wxBasicString out;
-            if ( body->get_innerText(out) == S_OK )
+            if ( body->get_innerText(out.ByRef()) == S_OK )
                 text = out;
         }
         return text;
@@ -950,8 +950,6 @@ wxCOMPtr<IHTMLDocument2> wxWebViewIE::GetDocument() const
 bool wxWebViewIE::IsElementVisible(wxCOMPtr<IHTMLElement> elm)
 {
     wxCOMPtr<IHTMLElement> elm1 = elm;   
-    wxBasicString display_bstr;
-    wxBasicString visibility_bstr;
     bool is_visible = true;
     //This method is not perfect but it does discover most of the hidden elements.
     //so if a better solution is found, then please do improve.
@@ -963,15 +961,18 @@ bool wxWebViewIE::IsElementVisible(wxCOMPtr<IHTMLElement> elm)
             wxCOMPtr<wxIHTMLCurrentStyle> style;
             if(SUCCEEDED(elm2->get_currentStyle(&style)))
             {
+                wxBasicString display_bstr;
+                wxBasicString visibility_bstr;
+
                 //Check if the object has the style display:none.
-                if((style->get_display(display_bstr) != S_OK) || 
+                if((style->get_display(display_bstr.ByRef()) != S_OK) || 
-                   ((BSTR)display_bstr != NULL && (wxCRT_StricmpW(display_bstr, L"none") == 0)))
+                    wxString(display_bstr).IsSameAs(wxS("none"), false))
                 {
                     is_visible = false;
                 }
                 //Check if the object has the style visibility:hidden.
-                if((is_visible && (style->get_visibility(visibility_bstr) != S_OK)) ||
+                if((is_visible && (style->get_visibility(visibility_bstr.ByRef()) != S_OK)) ||
-                  ((BSTR)visibility_bstr != NULL && wxCRT_StricmpW(visibility_bstr, L"hidden") == 0))
+                    wxString(visibility_bstr).IsSameAs(wxS("hidden"), false))
                 {
                     is_visible = false;
                 }