Opened on 04/08/2014 at 06:02:02 AM

Closed on 11/18/2014 at 02:57:45 PM

Last modified on 11/18/2014 at 03:02:47 PM

#276 closed change (rejected)

[meta] Stop using ATL

Reported by: fhd Assignee:
Priority: P5 Milestone:
Module: Adblock-Plus-for-Internet-Explorer Keywords: meta
Cc: trev, fhd, Blocked By: #1232, #1233, #1234, #1467
Blocking: #1163 Platform: Internet Explorer
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

Description (last modified by


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
#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

Attachments (0)

Change History (25)

comment:1 Changed on 04/08/2014 at 08:05:14 AM by trev

  • Cc trev added
  • Description modified (diff)

comment:2 Changed on 04/29/2014 at 11:47:07 AM by

  • Owner set to
  • Status changed from new to assigned

comment:3 Changed on 04/29/2014 at 12:30:15 PM by

I've started a Discourse thread on this issue.

comment:4 Changed on 04/30/2014 at 03:55:37 AM by fhd

  • Cc fhd added

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

comment:5 Changed on 04/30/2014 at 09:50:33 AM by philll

  • Status changed from assigned to new

The assigned state will be dropped by #403

comment:6 Changed on 05/01/2014 at 06:46:07 PM by

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.

comment:7 Changed on 05/01/2014 at 06:48:17 PM by

  • Cc added

comment:8 Changed on 05/10/2014 at 02:51:35 PM by

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


This is the exact list of all ATL symbols used.


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

comment:9 Changed on 06/21/2014 at 08:48:29 PM by

  • Review URL(s) modified (diff)

comment:10 Changed on 07/29/2014 at 06:46:02 PM by

  • Blocking 1163 added

comment:11 Changed on 08/13/2014 at 06:14:06 AM by fhd

  • Platform set to Unknown

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.

comment:12 Changed on 08/14/2014 at 10:30:30 AM by 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.

Last edited on 08/14/2014 at 10:30:56 AM by sergz

comment:13 Changed on 08/14/2014 at 11:33:34 AM by 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.

comment:14 Changed on 08/14/2014 at 05:23:36 PM by

  • 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.
Last edited on 08/14/2014 at 05:24:22 PM by

comment:15 Changed on 08/14/2014 at 09:36:20 PM by

  • Blocked By 1232 added

comment:16 Changed on 08/14/2014 at 09:38:08 PM by

  • Description modified (diff)

comment:17 Changed on 08/14/2014 at 10:11:46 PM by

  • Blocked By 1233 added

comment:18 Changed on 08/14/2014 at 10:24:02 PM by

  • Blocked By 1234 added

comment:19 Changed on 08/15/2014 at 05:38:43 AM by fhd

  • Keywords meta added
  • Platform changed from Unknown to Internet Explorer

comment:20 Changed on 08/15/2014 at 05:38:56 AM by fhd

  • Summary changed from Stop using ATL to [meta] Stop using ATL

comment:21 Changed on 08/15/2014 at 05:45:10 AM by fhd

  • Review URL(s) modified (diff)

comment:22 Changed on 10/08/2014 at 04:18:17 PM by

  • Blocked By 1467 added

comment:23 Changed on 10/21/2014 at 11:59:33 PM by oleksandr

  • Priority changed from P4 to P5

comment:24 Changed on 11/18/2014 at 02:57:45 PM by oleksandr

  • Resolution set to rejected
  • Status changed from new to closed

comment:25 Changed on 11/18/2014 at 03:02:47 PM by trev

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

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from
Note: See TracTickets for help on using tickets.