Issue #3391 - Initialize themed drawing variables using DetachedInitializer

Issue #3391 - Initialize themed drawing variables using DetachedInitializer

Add static 'Initializer()' function to 'CPluginClass'. Also add static
detached initializer member to control execution of that function. Spawn an
initializer thread in the constructor. Add convenience method
EnsureInitialized() which simply forwards to the detached initializer.

Move declarations for themed drawing entry point variables into an anonymous
namespace. Move their initialization into 'Initializer()'. Convert
's_hUxtheme' into a local variable, since we no longer need to rely on its
value to guard against multiple execution. Similarly, remove now-unneeded
synchronization around these initializations, since that facility is now
provided by 'DetachedInitialization'.

Call EnsureInitialized() before using themed drawing variables. This
eliminates possible race conditions with using these variables before they
have had a chance to initialize.

Refactored the body of 'CloseTheme()' into that of its one calling function.

-----

This review depends upon another one.
    https://codereview.adblockplus.org/29332660/

[Eric, 2015-12-14 18:46:02]

This is the second in the series of detached initialization change sets. The
ultimate goal is to factor out the entirety of 'InitObject()' into the new
detached initializer. Doing that all at once would make for an enormous review,
and in addition one based on the new control class. Therefore, I've picked off a
nicely self-contained piece of the larger problem as a starter.

This change sets up the detached initializer infrastructure, which is quite
modest in `CPluginClass`, since all the complexity has been broken out into the
new class. I've moved the initialization into the constructor (not 'SetSite()')
to allow the thread to start as soon as possible, since we're initialization
class variables, not instance ones. (I think most all of them are like this, if
not all.) It shows the uses of 'EnsureInitialized()', which eliminate all the
race conditions we currently have with detached initialization.

It also demonstrates how the new infrastructure simplifies things generally. For
example, 's_hUxtheme' was shadowing the single-execution principle that the
infrastructure now does, so it could be converted into a local variable. It
should also be apparent that this kind of initialization needs to be done just
once per process, not once per BHO instance.

There are some things that are not currently in 'InitObject()' that could
usefully be put into the new detached initializer. Most notable is starting up
the engine instance. Also in the category are instantiating singletons such as
'Dictionary'.

https://codereview.adblockplus.org/29332665/diff/29332666/src/plugin/PluginCl...
File src/plugin/PluginClass.cpp (left):

https://codereview.adblockplus.org/29332665/diff/29332666/src/plugin/PluginCl...
src/plugin/PluginClass.cpp:178: ((CPluginClass*)thisPtr)->Unadvise();
This statement is the motivation to clean up detached initialization now. All
the explicit calls to Unadvise() are messy at best, and probably buggy.
Certainly they're an obstacle to get event registration and instance lifetime in
good working order.

https://codereview.adblockplus.org/29332665/diff/29332666/src/plugin/PluginCl...
src/plugin/PluginClass.cpp:240: startInitObjectThread.detach(); // TODO: but
actually we should wait for the thread in the dtr.
This comment is sort-of-right; at least it expresses the right concern. We
should wait for a detached initialization thread at every point we rely upon its
side effects. That will generally include the destructor, but certainly isn't
limited to it. The function 'DetachedInitializer::EnsureInitialized()' does
exactly what this comment is concerned with. We won't need anything like this
comment after we've factored out 'InitObject()'.

[Eric, 2015-12-14 19:27:28]

Oops. Sent initial mail before I added reviewers.

Please expand and read the first message. It's important, but isn't properly
part of the description.