[IE] First round of ATL removal

src/plugin/Wrapper.h

 /**
  * \file Wrapper.h Wrappers for Windows platform calls and the like.
  *
  * The main purpose of this file is to isolate Windows-specific data types, replacing them with ones from the standard library.
  */
 
 #ifndef WRAPPER_H

[sergei, 2014/07/08 11:58:34]

`WRAPPER_H` is too generic and it's very easy to get it clashed with another
header.
I personally prefer to use `#pragma once`, because it's much safer and easier,
it is a very well supported by compilers and it's not so easy to find a compiler
which does not support it. I don't insist on `#pragma once` but if the policy is
to use ifdef/define/endif, then it should contain the the project name with the
full path to the file or in addition UUID.

[Eric, 2014/07/08 17:46:37]

This is the convention we've been using in all recent code.

If we want to change the convention, this review is not the right place to have
that discussion.
FWIW. Only recently did GCC support it correctly.

We don't have issues with files being copied before compilation (not in the
current build environment), but #pragma once can fail in those cases.

 #define WRAPPER_H
 
 /*
  * We need ATL for the time being to write the Browser class, which wraps CComQIPtr.
  */
 #include "ATL_Deprecate.h"
 
 #include <string>
 
+#include <Mshtml.h>
 #include <Exdisp.h>
 
 namespace Wrapper
 {
   /**
    * Wrapper for UrlUnescape
    *
    * Modifies the string value in place.
    */
   std::wstring & Unescape_URL( std::wstring & url ) ;
 
   /**
    * Wrapper around a pointer to an IWebBrowser2 interface.
    */
   class Browser

[Oleksandr, 2014/06/26 00:48:43]

I suppose the general idea is to make these classes to be smart pointers for COM
objects later, correct? Still, I don't quite understand the need for CComQIPtr
here, when we are using just plain interface pointers in other similar classes.
Can's it be just IWebBrowser* _browser here?

[Eric, 2014/06/26 15:13:56]

Yes, basically.
Yes. This was the first one I wrote, and I just copied the declaration that it
came from. I never changed it to match the others.

Changed in next patch set.

   {
     /**
      * The underlying browser
      */
-    CComQIPtr<IWebBrowser2> _browser;
+    IWebBrowser2 * _browser;
 
   public:
     /**
      * Ordinary constructor.
      */
-    Browser( CComQIPtr<IWebBrowser2> browser )
+    Browser( IWebBrowser2 * browser )
       : _browser( browser )
     {}
 
     /**
      * Navigate to a url in a new tab, or failing that, in a new window.
      *
      * return true iff navigation was successful
      */
     HRESULT navigate( std::wstring url );
 
     /**
      * Retrieve the "LocationURL" property
      */
     bool Location_URL( std::wstring & value ) const;

[sergei, 2014/07/08 11:58:34]

It should be without the underscore.

   };
 
   /**
    *
    */
   class Internet_Bind_Info

[sergei, 2014/07/08 11:58:34]

In the other parts of the project the naming convention is different, it looks
like `class SomeClass;` members are named as `m_someMember`.

   {
     IInternetBindInfo * _bind_info;
   public:
     Internet_Bind_Info( IInternetBindInfo * bind_info )
       : _bind_info( bind_info )
     {}
 
     std::wstring bind_string_single( BINDSTRING type );
   };
 
   /**
    *
    */
   class HTML_Element

[sergei, 2014/07/08 11:58:34]

But here for me `HTML_Element` looks better than `HTMLElement` and even better
than having `html` namespace.

   {
     IHTMLElement * _element;
   public:
     HTML_Element( IHTMLElement * element )
       : _element( element )
     {}
 
     /**
      * Retrieve an attribute value, if present.
      *
@@ skipping 92 matching lines @@
      * \return
      *   IDispatch point to the element if the element is found
      *   0 otherwise
      */
     IDispatch * at( long index );
   };
 
 }
 
 #endif