Opened on 02/27/2015 at 04:39:33 PM
Last modified on 03/13/2015 at 09:02:46 AM
#2058 new change
Remove thread associated with CPluginTab
Reported by: | eric@adblockplus.org | Assignee: | |
---|---|---|---|
Priority: | P4 | Milestone: | |
Module: | Adblock-Plus-for-Internet-Explorer | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Platform: | Internet Explorer | |
Ready: | yes | Confidential: | no |
Tester: | Verified working: | ||
Review URL(s): |
Description
Background
The thread associated with CPluginTabBase no longer has anything specific to do with tabs. Instead, it's acting as a log processing pump, polling to perform deferred logging of errors. It's a violation of any number of good software practices.
- It violates basic encapsulation in the worst way, using member variables in CPluginTabBase to implement a behavior used in error processing.
- It's a needless use of threads, since we need at most one thread to deal with deferred logging, but we're using one thread per tab to do it.
- It runs constantly as a polling loop, where it only needs to run when there are deferred errors to log.
- In the case that the thread is told to stop running, it can leave deferred errors stranded in the queue. That is, there's nowhere where the queue is flushed at termination.
What to change
Remove the thread from CPluginTab. Eliminate member variables m_isActivated and m_continueThreadRunning, which are only used for the thread. Eliminate member function ThreadProc. Eliminate functions OnActivate and OnUpdate, which are only used to update m_isActivated.
There are a number of things that could be done at the same time, though none of them are, strictly speaking, necessary to fulfill the narrower goal of this ticket, which is simply to get the thread out of the class CPluginTab.
- Encapsulate deferred error logging and its queue into a separate class.
- Change from using a polling mechanism to one using notify/wait.
Attachments (0)
Change History (1)
comment:1 Changed on 03/13/2015 at 09:02:46 AM by oleksandr
- Priority changed from Unknown to P4
- Ready set