[IE] First round of ATL removal

src/plugin/Wrapper.h

Index: src/plugin/Wrapper.h
===================================================================
new file mode 100644
--- /dev/null
+++ b/src/plugin/Wrapper.h
@@ -0,0 +1,188 @@
+/**
+ * \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 <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;
+
+  public:
+    /**
+     * Ordinary constructor.
+     */
+    Browser( CComQIPtr<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.
+     *
+     * \param attr_value
+     *   Returns attribute value, if attribute is present; otherwise unaltered.
+     * \return
+     *   true if attribute is present
+     *   false otherwise
+     */
+    bool attribute( std::wstring attr_name, std::wstring & attr_value ) const;
+
+    /**
+     * Retrieve an integer attribute value, if the attribute is present and its value is an integer.
+     *
+     * \param attr_value
+     *   Returns attribute value, if attribute is present and is an integer; otherwise unaltered.
+     * \return
+     *   true if attribute is present and is an integer
+     *   false otherwise
+     */
+    bool attribute( std::wstring attr_name, int & attr_value ) const;
+
+    /**
+     * Retrieve the id property.
+     */
+    bool id( std::wstring & value ) const;
+
+    /**
+     * Retrieve the class name property.
+     */
+    bool class_name( std::wstring & value ) const;
+
+    /**
+     * Retrieve the innerHTML property.
+     *
+     * This function is considered always to succeed, setting 'value' to 
+     * Thus the return value is void instead of bool.
+     * 
+     */
+    bool inner_HTML( std::wstring & value ) const;
+
+    /**
+     * Retrieve the tag property
+     */
+    bool tag_name( std::wstring & value ) const;
+  };
+
+  /**
+   *
+   */
+  class HTML_Style
+  {
+    IHTMLStyle * _style;
+  public:
+    /**
+     * Ordinary constructor
+     */
+    HTML_Style( IHTMLStyle * style )
+      : _style( style )
+    {}
+
+    /**
+     * Retrieve the display property.
+     */
+    bool display( std::wstring & value ) const;
+
+    /**
+     * Assign the display property.
+     */
+    bool assign_display( const std::wstring & value );
+
+    /**
+     * Retrieve the CSS text.
+     */
+    bool CSS_text( std::wstring & value ) const;
+  };
+
+  /**
+   */
+  class HTML_Element_Collection
+  {
+    IHTMLElementCollection * _elements;
+  public:
+    HTML_Element_Collection( IHTMLElementCollection * elements )
+      : _elements( elements )
+    {}
+
+    /**
+     * Retrieve an item by index.
+     *
+     * This is a wrapper around IHTMLElementCollection::item
+     *   and where the 'name' argument is an integer, returning a value by index.
+     * Note that the wrapped function item() returns S_OK even when the function is not found.
+     * Thus we indicate all error conditions with a zero return value.
+     *
+     * \return
+     *   IDispatch point to the element if the element is found
+     *   0 otherwise
+     */
+    IDispatch * at( long index );
+  };
+
+}
+
+#endif