Fix assert that BackgroundDownloader's timer os deleted on wrong thread
The timer and two interface pointers are leaked if a download is
active when the browser is shutting down.
Releasing these resources on the file thread is impractical to do at
this time due to the browser shutdown sequence. The file thread is
terminated before the dtor of this class is called and there is no
obvious way to hook up the shutdown of the browser to terminate a
download in progress and dispose of these resources.
BUG=341083
Review URL: https://codereview.chromium.org/156983005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@249662 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/component_updater/background_downloader_win.cc b/chrome/browser/component_updater/background_downloader_win.cc
index 004f5f28..dff644e 100644
--- a/chrome/browser/component_updater/background_downloader_win.cc
+++ b/chrome/browser/component_updater/background_downloader_win.cc
@@ -396,6 +396,19 @@
}
BackgroundDownloader::~BackgroundDownloader() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+ // The following objects have thread affinity and can't be destroyed on the
+ // UI thread. The resources managed by these objects are acquired at the
+ // beginning of a download and released at the end of the download. Most of
+ // the time, when this destructor is called, these resources have been already
+ // disposed by. Releasing the ownership here is a NOP. However, if the browser
+ // is shutting down while a download is in progress, the timer is active and
+ // the interface pointers are valid. Releasing the ownership means leaking
+ // these objects and their associated resources.
+ timer_.release();
+ bits_manager_.Detach();
+ job_.Detach();
}
void BackgroundDownloader::DoStartDownload(const GURL& url) {
diff --git a/chrome/browser/component_updater/background_downloader_win.h b/chrome/browser/component_updater/background_downloader_win.h
index d1a08f84..500bef8 100644
--- a/chrome/browser/component_updater/background_downloader_win.h
+++ b/chrome/browser/component_updater/background_downloader_win.h
@@ -20,7 +20,9 @@
// Implements a downloader in terms of the BITS service. The public interface
// of this class and the CrxDownloader overrides are expected to be called
// from the UI thread. The rest of the class code runs on the FILE thread in
-// a single threaded apartment.
+// a single threaded apartment. Instances of this class are created and
+// destroyed in the UI thread. See the implementation of the class destructor
+// for details regarding the clean up of resources acquired in this class.
class BackgroundDownloader : public CrxDownloader {
protected:
friend class CrxDownloader;
@@ -70,6 +72,9 @@
net::URLRequestContextGetter* context_getter_;
scoped_refptr<base::SequencedTaskRunner> task_runner_;
+ // The timer and the BITS interface pointers have thread affinity. These
+ // members are initialized on the FILE thread and they must be destroyed
+ // on the FILE thread.
scoped_ptr<base::RepeatingTimer<BackgroundDownloader> > timer_;
base::win::ScopedComPtr<IBackgroundCopyManager> bits_manager_;
diff --git a/chrome/browser/component_updater/component_updater_service.cc b/chrome/browser/component_updater/component_updater_service.cc
index 780354b..2095c05 100644
--- a/chrome/browser/component_updater/component_updater_service.cc
+++ b/chrome/browser/component_updater/component_updater_service.cc
@@ -155,8 +155,8 @@
// the tasks. Also when we do network requests there is only one |url_fetcher_|
// in flight at a time.
// There are no locks in this code, the main structure |work_items_| is mutated
-// only from the UI thread. The unpack and installation is done in the file
-// thread and the network requests are done in the IO thread and in the file
+// only from the UI thread. The unpack and installation is done in a blocking
+// pool thread. The network requests are done in the IO thread or in the file
// thread.
class CrxUpdateService : public ComponentUpdateService {
public:
