Issue #276 - introduce class BSTR_Argument

src/plugin/PluginClass.cpp

Index: src/plugin/PluginClass.cpp
===================================================================
--- a/src/plugin/PluginClass.cpp
+++ b/src/plugin/PluginClass.cpp
@@ -1,6 +1,7 @@
 #include "PluginStdAfx.h"
 
 #include "PluginClass.h"
+#include "COM_Value.h"
 #include "PluginSettings.h"
 #include "PluginSystem.h"
 #ifdef SUPPORT_FILTER
@@ -218,11 +219,10 @@
   CComQIPtr<IWebBrowser2> browser = GetBrowser();
   if (browser)
   {
-    CComBSTR bstrURL;
-
-    if (SUCCEEDED(browser->get_LocationURL(&bstrURL)))
+    AdblockPlus::COM::BSTR_Argument result;
+    if (SUCCEEDED(browser->get_LocationURL(&result)))
     {
-      url = bstrURL;
+      url = to_CString(result);
       CPluginClient::UnescapeUrl(url);
     }
   }
@@ -507,14 +507,14 @@
           vFlags.vt = VT_I4;
           vFlags.intVal = navOpenInNewTab;
 
-          CComBSTR curLoc;
+          AdblockPlus::COM::BSTR_Argument curLoc;
           browser->get_LocationURL(&curLoc);
           HRESULT hr = browser->Navigate(curLoc, &vFlags, NULL, NULL, NULL);
           if (FAILED(hr))
           {
             vFlags.intVal = navOpenInNewWindow;
 
-            hr = browser->Navigate(CComBSTR(curLoc), &vFlags, NULL, NULL, NULL);
+            hr = browser->Navigate(curLoc, &vFlags, NULL, NULL, NULL);
             if (FAILED(hr))
             {
               DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_NAVIGATION, PLUGIN_ERROR_NAVIGATION, "Navigation::Failed")
@@ -733,10 +733,10 @@
         if (pBrowser)
         {
           CString url;
-          CComBSTR bstrUrl;
-          if (SUCCEEDED(pBrowser->get_LocationURL(&bstrUrl)) && ::SysStringLen(bstrUrl) > 0)
+          AdblockPlus::COM::BSTR_Argument result;
+          if (SUCCEEDED(pBrowser->get_LocationURL(&result)) && !result.operator std::wstring().empty())

[sergei, 2014/07/28 09:18:26]

Add and call the method `Length` instead of such conversion.

[Eric, 2014/07/29 16:59:27]

Absolutely not. BSTR_Argument is a conversion class. I'm adamantly opposed to
going the Microsoft way with this and giving the class semantic properties. That
way lies simply reimplementing CComBSTR and dealing with two string classes.

Part of the ATL removal process is creating wrapper objects for COM objects
(here IBrowser2) that isolate type conversions. There will be an encapsulating
method "LocationURL" that returns std::wstring. The check with empty() will be
in the final version of the code.

Consult the original version of the ATL removal to see what the code looks like
with these wrapper functions in place.
http://codereview.adblockplus.org/5750789393874944/

           {
-            url = bstrUrl;
+            url = to_CString(result);
 
             CPluginClient::UnescapeUrl(url);
 
@@ -1042,8 +1042,7 @@
   vFlags.vt = VT_I4;
   vFlags.intVal = navOpenInNewTab;
 
-  CComBSTR navigatePath = CComBSTR(FirstRunPageFileUrl().c_str());
-  
+  AdblockPlus::COM::BSTR_Argument navigatePath(FirstRunPageFileUrl());
   HRESULT hr = GetAsyncBrowser()->Navigate(navigatePath, &vFlags, NULL, NULL, NULL);
   if (FAILED(hr))
   {