Merge branch 'fix-basicstring-use'

Fix recently introduced bug in wxWMP10MediaBackend and simplify
wxBasicString.

See #17889.

Closes https://github.com/wxWidgets/wxWidgets/pull/515

include/wx/msw/ole/oleutils.h

@@ -65,47 +65,43 @@ inline void wxOleUninitialize()
 
 class WXDLLIMPEXP_CORE wxBasicString
 {
-public:    
+public:
-    // Takes over the ownership of bstr
+    // Constructs with the owned BSTR set to NULL
-    explicit wxBasicString(BSTR bstr = NULL);
+    wxBasicString() : m_bstrBuf(NULL) {}
 
-    // Constructs the BSTR from wxString
+    // Constructs with the owned BSTR created from a wxString
-    wxBasicString(const wxString& str);
+    wxBasicString(const wxString& str)
+        : m_bstrBuf(SysAllocString(str.wc_str(*wxConvCurrent))) {}
 
-    // Creates a copy of the BSTR owned by bstr
+    // Constructs with the owned BSTR as a copy of the BSTR owned by bstr
-    wxBasicString(const wxBasicString& bstr);
+    wxBasicString(const wxBasicString& bstr) : m_bstrBuf(bstr.Copy()) {}
 
     // Frees the owned BSTR
-    ~wxBasicString();
+    ~wxBasicString() { SysFreeString(m_bstrBuf); }
 
-    // Returns the owned BSTR and gives up its ownership
+    // Creates the owned BSTR from a wxString
+    void AssignFromString(const wxString& str);
+
+    // Returns the owned BSTR and gives up its ownership,
+    // the caller is responsible for freeing it
     BSTR Detach();
 
-    // Frees the owned BSTR
+    // Returns a copy of the owned BSTR,
-    void Free();
+    // the caller is responsible for freeing it
-
-    // 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
+    // Returns the address of the owned BSTR, not to be called
-    // when wxBasicString already contains a non-NULL BSTR
+    // 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);
 
-    // Creates its BSTR from wxString
+    /// Returns the owned BSTR while keeping its ownership
-    wxBasicString& operator=(const wxString& str);
-
-    // Takes over the ownership of bstr
-    wxBasicString& operator=(BSTR bstr);
-
-    /// Returns the owned BSTR while keeping its ownership    
     operator BSTR() const { return m_bstrBuf; }
 
     // retrieve a copy of our string - caller must SysFreeString() it later!
-    wxDEPRECATED_MSG("use Copy() instead") 
+    wxDEPRECATED_MSG("use Copy() instead")
     BSTR Get() const { return Copy(); }
 private:
     // actual string

src/msw/mediactrl_wmp10.cpp

@@ -1061,12 +1061,12 @@ bool wxWMP10MediaBackend::ShowPlayerControls(wxMediaCtrlPlayerControls flags)
     if(!flags)
     {
         m_pWMPPlayer->put_enabled(VARIANT_FALSE);
-        m_pWMPPlayer->put_uiMode(wxBasicString(wxT("none")));
+        m_pWMPPlayer->put_uiMode(wxBasicString(wxS("none")));
     }
     else
     {
         // TODO: use "custom"? (note that CE only supports none/full)
-        m_pWMPPlayer->put_uiMode(wxBasicString(wxT("full")));
+        m_pWMPPlayer->put_uiMode(wxBasicString(wxS("full")));
         m_pWMPPlayer->put_enabled(VARIANT_TRUE);
     }
 
@@ -1358,7 +1358,7 @@ wxLongLong wxWMP10MediaBackend::GetDownloadTotal()
     if(m_pWMPPlayer->get_currentMedia(&pWMPMedia) == 0)
     {
         BSTR bsOut;
-        pWMPMedia->getItemInfo(wxBasicString(wxT("FileSize")),
+        pWMPMedia->getItemInfo(wxBasicString(wxS("FileSize")),
                                &bsOut);
 
         wxString sFileSize = wxConvertStringFromOle(bsOut);

src/msw/ole/automtn.cpp

@@ -130,8 +130,8 @@ bool wxAutomationObject::Invoke(const wxString& member, int action,
     }
 
     int namedArgStringCount = namedArgCount + 1;
-    wxVector<wxBasicString> argNames(namedArgStringCount, wxString());
+    wxVector<wxBasicString> argNames(namedArgStringCount);
-    argNames[0] = member;
+    argNames[0].AssignFromString(member);
 
     // Note that arguments are specified in reverse order
     // (all totally logical; hey, we're dealing with OLE here.)
@@ -141,7 +141,7 @@ bool wxAutomationObject::Invoke(const wxString& member, int action,
     {
         if ( !INVOKEARG(i).GetName().empty() )
         {
-            argNames[(namedArgCount-j)] = INVOKEARG(i).GetName();
+            argNames[(namedArgCount-j)].AssignFromString(INVOKEARG(i).GetName());
             j ++;
         }
     }

src/msw/ole/oleutils.cpp

@@ -70,79 +70,39 @@ WXDLLEXPORT wxString wxConvertStringFromOle(BSTR bStr)
 // ----------------------------------------------------------------------------
 // wxBasicString
 // ----------------------------------------------------------------------------
-
+void wxBasicString::AssignFromString(const wxString& str)
-wxBasicString::wxBasicString(BSTR bstr)
-{
-    m_bstrBuf = bstr;
-}
-
-wxBasicString::wxBasicString(const wxString& str)
 {
+    SysFreeString(m_bstrBuf);
     m_bstrBuf = SysAllocString(str.wc_str(*wxConvCurrent));
 }
 
-wxBasicString::wxBasicString(const wxBasicString& bstr)
+BSTR wxBasicString::Detach()
 {
-    m_bstrBuf = bstr.Copy();
+    BSTR bstr = m_bstrBuf;
+
+    m_bstrBuf = NULL;
+
+    return bstr;
 }
 
-wxBasicString::~wxBasicString()
+BSTR* wxBasicString::ByRef()
 {
-    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"));
+        wxS("Can't get direct access to initialized BSTR"));
-    return &m_bstrBuf;
+    return &m_bstrBuf;
 }
 
 wxBasicString& wxBasicString::operator=(const wxBasicString& src)
 {
-    if ( this != &src )
+    if ( this != &src )
-    {
+    {
-        wxCHECK_MSG(m_bstrBuf == NULL || m_bstrBuf != src.m_bstrBuf, 
+        wxCHECK_MSG(m_bstrBuf == NULL || m_bstrBuf != src.m_bstrBuf,
-            *this, wxS("Attempting to assign already owned BSTR"));
+            *this, wxS("Attempting to assign already owned BSTR"));
-        SysFreeString(m_bstrBuf);
+        SysFreeString(m_bstrBuf);
-        m_bstrBuf = src.Copy();
+        m_bstrBuf = src.Copy();
-    }
+    }
-
-    return *this;
-}
 
-wxBasicString& wxBasicString::operator=(const wxString& str)
+    return *this;
-{
-    SysFreeString(m_bstrBuf);
-    m_bstrBuf = SysAllocString(str.wc_str(*wxConvCurrent));
-
-    return *this;
-}
-
-wxBasicString& wxBasicString::operator=(BSTR bstr)
-{
-    wxCHECK_MSG(m_bstrBuf == NULL || m_bstrBuf != bstr, 
-        *this, wxS("Attempting to assign already owned BSTR"));
-
-    SysFreeString(m_bstrBuf);
-    m_bstrBuf = bstr;
-
-    return *this;
 }
 
 // ----------------------------------------------------------------------------

src/msw/webview_ie.cpp

@@ -1009,8 +1009,8 @@ void wxWebViewIE::FindInternal(const wxString& text, int flags, int internal_fla
         if(SUCCEEDED(document->QueryInterface(wxIID_IMarkupContainer, (void **)&pIMC)))
         {
             wxCOMPtr<wxIMarkupPointer> ptrBegin, ptrEnd;
-            wxBasicString attr_bstr(wxString("style=\"background-color:#ffff00\""));
+            wxBasicString attr_bstr(wxS("style=\"background-color:#ffff00\""));
-            wxBasicString text_bstr(text.wc_str());
+            wxBasicString text_bstr(text);
             
             pIMS->CreateMarkupPointer(&ptrBegin);
             pIMS->CreateMarkupPointer(&ptrEnd);