[meta] Stop using ATL

[fhd]

Background

We're currently using some things from ATL, which requires installing the Windows Driver Kit 7.1.0 (newer versions won't work) to build ABP for IE. If we got rid of this, building ABP would only require Visual Studio Express and Windows SDK.

What to change

Replace anything we use from ATL with either our own code, or a third party dependency that we can include in source.

Ticket	Status	Resolution	Summary	Component	Owner
#1232	closed	fixed	[Stop using ATL] Replace ATL::CSimpleArray with std::set	Adblock-Plus-for-Internet-Explorer	eric@adblockplus.org
#1233	closed	rejected	[Stop using ATL] Replace ATL::CComBSTR	Adblock-Plus-for-Internet-Explorer
#1234	new		[Stop using ATL] Replace ATL::CString with std::wstring	Adblock-Plus-for-Internet-Explorer	eric@…
#1467	new		[Stop using ATL] Convert ATL mutexes to standard library ones.	Adblock-Plus-for-Internet-Explorer

[eric@adblockplus.org]

I've started a ​Discourse thread on this issue.

[fhd]

As Wladimir said there, let's move the discussion here.

[philll]

The assigned state will be dropped by #403

[eric@adblockplus.org]

This guy has figured it out: ​Writing a BHO in Plain C++.

The advantage of the approach here is that it's a minimal COM object needed to act as a BHO. There's nothing extraneous here, very much unlike the plugin class definition that the ATL wizard outputs (which is what seems to be in our code base). I was rather worried that I'd have to reverse engineer what we had to keep and what could be discarded. This article act as a road map for a rewrite.

That said, there are also some ATL utility classes we use, but those were never going to be where the bulk of the work lay.

[eric@adblockplus.org]

There are five ATL headers used. At the coarsest level, the scope of this issue is to eliminate these headers.

atlbase.h
atlstr.h
atltypes.h
atlcom.h
atlhost.h

This is the exact list of all ATL symbols used.

_AtlBaseModule
CAtlBaseModule
CComAggObject
CComModule
CComAutoCriticalSection
CComBSTR
CComClassFactory
CComCoClass
CComCreator
CComCreator2
CComMultiThreadModel
CComObject
CComObjectNoLock
CComObjectRootEx
CComPtr
CComQIPtr
CComSingleThreadModel
CComVariant
CRect
CSimpleArray
CString
CStringA
CStringW
CW2A
IDispatchImpl
IObjectWithSiteImpl
OLE2T

By far the most-used symbols are CString and the other string classes.

[fhd]

Is the current plan still to replace all of ATL? From what I've heard you decided to focus on low hanging fruits, like CString. Seems reasonable not too put too much effort into it.

[sergz]

Is the current plan still to replace all of ATL?

My vote is to keep using ATL, it will definitely result in more robust application, less and clearer code, as well as save a lot of resources, especially in a long term.

ATL::CString and others

I would avoid emphasizing of it, just say, that the new code should use std::wstring or the proper structure, and if we touch the old code and it does not cause avalanche-like changes we can replace ATL::CString by std::string.
BTW, what is the main problem with CString except different from std interface? In some cases it's very convenient and replacing it by our hand-made functions violates NIH syndrome.

[fhd]

BTW, what is the main problem with CString except different from std interface? In some cases it's very convenient and replacing it by our hand-made functions violates NIH syndrome.

From what I see, these hand-made functions are mostly wrappers around functions that work with CString, that's good IMO, assuming we stick to std::string functionality as much as possible - duplicating things std::string can do would not be good.

In general, it seems like a good idea to use just one string class across the code base. It's hard for me to make a general statement though, I have seen very little of how we use CString. I don't think we have to be religious about it if it makes the code worse - assuming we are not going to get rid of ATL completely.

[eric@adblockplus.org]

• Using a single string class consistently across our code base is a clear good. (I'm counting wstring and string as the same for the purposes of this design discussion.) As of today we have five string classes:
  • wstring (std)
  • BSTR (Microsoft)
  • CComBSTR (Microsoft)
  • CString (Microsoft)
  • BString (ours)
  We're using all the Microsoft classes semantically at some point in the code, though by far the bulk of such uses are in CString. One major goal for string types is to get all the semantic uses into wstring.

• Most occurrences of CString, however, either do not involve string semantics or do so only in very light ways (e.g. empty(), length(), and concatenation). A large number are just used to pass values around unaltered and unexamined.

• CString is anything but low-hanging fruit. It's textually extensive. It's also worth doing in order to simplify the code. Converting back and forth between morally-equivalent types is nothing but junk code; you might really want it, but that doesn't mean it's good for you.

• The functions I wrote were to replace member functions in CString. Some did not have direct equivalents in the standard library (such those to trim whitespace). Some were syntactic sugar around existing library functions (such as begins_with as a particular form of compare). The case of CString::tokenize was a bit special; it was expedient to replicate the semantics of the CString function. Overall, it's rather a small amount of code we needed that wasn't already in wstring.

[trev]

For reference, replacing ATL completely is no longer planned - the benefits aren't worth the effort.