Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(328)

Side by Side Diff: components/update_client/update_client.cc

Issue 1439153002: Revert of Change the update_client task runner behavior to continue on shutdown. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/update_client/update_client.h" 5 #include "components/update_client/update_client.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <queue> 8 #include <queue>
9 #include <set> 9 #include <set>
10 #include <utility> 10 #include <utility>
(...skipping 50 matching lines...) Expand 10 before | Expand all | Expand 10 after
61 // pointer to itself. Otherwise, a life time circular dependency between this 61 // pointer to itself. Otherwise, a life time circular dependency between this
62 // instance and its inner members prevents the destruction of this instance. 62 // instance and its inner members prevents the destruction of this instance.
63 // Using unretained references is allowed in this case since the life time of 63 // Using unretained references is allowed in this case since the life time of
64 // the UpdateClient instance exceeds the life time of its inner members, 64 // the UpdateClient instance exceeds the life time of its inner members,
65 // including any thread objects that might execute callbacks bound to it. 65 // including any thread objects that might execute callbacks bound to it.
66 UpdateClientImpl::UpdateClientImpl( 66 UpdateClientImpl::UpdateClientImpl(
67 const scoped_refptr<Configurator>& config, 67 const scoped_refptr<Configurator>& config,
68 scoped_ptr<PingManager> ping_manager, 68 scoped_ptr<PingManager> ping_manager,
69 UpdateChecker::Factory update_checker_factory, 69 UpdateChecker::Factory update_checker_factory,
70 CrxDownloader::Factory crx_downloader_factory) 70 CrxDownloader::Factory crx_downloader_factory)
71 : is_stopped_(false), 71 : config_(config),
72 config_(config),
73 ping_manager_(ping_manager.Pass()), 72 ping_manager_(ping_manager.Pass()),
74 update_engine_( 73 update_engine_(
75 new UpdateEngine(config, 74 new UpdateEngine(config,
76 update_checker_factory, 75 update_checker_factory,
77 crx_downloader_factory, 76 crx_downloader_factory,
78 ping_manager_.get(), 77 ping_manager_.get(),
79 base::Bind(&UpdateClientImpl::NotifyObservers, 78 base::Bind(&UpdateClientImpl::NotifyObservers,
80 base::Unretained(this)))) {} 79 base::Unretained(this)))) {
80 }
81 81
82 UpdateClientImpl::~UpdateClientImpl() { 82 UpdateClientImpl::~UpdateClientImpl() {
83 DCHECK(thread_checker_.CalledOnValidThread()); 83 DCHECK(thread_checker_.CalledOnValidThread());
84 84
85 DCHECK(task_queue_.empty()); 85 while (!task_queue_.empty()) {
86 DCHECK(tasks_.empty()); 86 delete task_queue_.front();
87 task_queue_.pop();
88 }
87 89
88 config_ = nullptr; 90 config_ = nullptr;
89 } 91 }
90 92
91 void UpdateClientImpl::Install(const std::string& id, 93 void UpdateClientImpl::Install(const std::string& id,
92 const CrxDataCallback& crx_data_callback, 94 const CrxDataCallback& crx_data_callback,
93 const CompletionCallback& completion_callback) { 95 const CompletionCallback& completion_callback) {
94 DCHECK(thread_checker_.CalledOnValidThread()); 96 DCHECK(thread_checker_.CalledOnValidThread());
95 97
96 if (IsUpdating(id)) { 98 if (IsUpdating(id)) {
(...skipping 44 matching lines...) Expand 10 before | Expand all | Expand 10 after
141 void UpdateClientImpl::OnTaskComplete( 143 void UpdateClientImpl::OnTaskComplete(
142 const CompletionCallback& completion_callback, 144 const CompletionCallback& completion_callback,
143 Task* task, 145 Task* task,
144 int error) { 146 int error) {
145 DCHECK(thread_checker_.CalledOnValidThread()); 147 DCHECK(thread_checker_.CalledOnValidThread());
146 DCHECK(task); 148 DCHECK(task);
147 149
148 base::ThreadTaskRunnerHandle::Get()->PostTask( 150 base::ThreadTaskRunnerHandle::Get()->PostTask(
149 FROM_HERE, base::Bind(completion_callback, error)); 151 FROM_HERE, base::Bind(completion_callback, error));
150 152
151 // Remove the task from the set of the running tasks. Only tasks handled by
152 // the update engine can be in this data structure.
153 tasks_.erase(task); 153 tasks_.erase(task);
154
155 // Delete the completed task. A task can be completed because the update
156 // engine has run it or because it has been canceled but never run.
157 delete task; 154 delete task;
158 155
159 if (is_stopped_)
160 return;
161
162 // Pick up a task from the queue if the queue has pending tasks and no other 156 // Pick up a task from the queue if the queue has pending tasks and no other
163 // task is running. 157 // task is running.
164 if (tasks_.empty() && !task_queue_.empty()) { 158 if (tasks_.empty() && !task_queue_.empty()) {
165 RunTask(scoped_ptr<Task>(task_queue_.front()).Pass()); 159 RunTask(scoped_ptr<Task>(task_queue_.front()).Pass());
166 task_queue_.pop(); 160 task_queue_.pop();
167 } 161 }
168 } 162 }
169 163
170 void UpdateClientImpl::AddObserver(Observer* observer) { 164 void UpdateClientImpl::AddObserver(Observer* observer) {
171 DCHECK(thread_checker_.CalledOnValidThread()); 165 DCHECK(thread_checker_.CalledOnValidThread());
(...skipping 22 matching lines...) Expand all
194 for (const auto& task : tasks_) { 188 for (const auto& task : tasks_) {
195 const auto ids(task->GetIds()); 189 const auto ids(task->GetIds());
196 if (std::find(ids.begin(), ids.end(), id) != ids.end()) { 190 if (std::find(ids.begin(), ids.end(), id) != ids.end()) {
197 return true; 191 return true;
198 } 192 }
199 } 193 }
200 194
201 return false; 195 return false;
202 } 196 }
203 197
204 void UpdateClientImpl::Stop() {
205 DCHECK(thread_checker_.CalledOnValidThread());
206
207 is_stopped_ = true;
208
209 // In the current implementation it is sufficient to cancel the pending
210 // tasks only. The tasks that are run by the update engine will stop
211 // making progress naturally, as the main task runner stops running task
212 // actions. Upon the browser shutdown, the resources employed by the active
213 // tasks will leak, as the operating system kills the thread associated with
214 // the update engine task runner. Further refactoring may be needed in this
215 // area, to cancel the running tasks by canceling the current action update.
216 // This behavior would be expected, correct, and result in no resource leaks
217 // in all cases, in shutdown or not.
218 //
219 // Cancel the pending tasks. These tasks are safe to cancel and delete since
220 // they have not picked up by the update engine, and not shared with any
221 // task runner yet.
222 while (!task_queue_.empty()) {
223 const auto task(task_queue_.front());
224 task_queue_.pop();
225 task->Cancel();
226 }
227 }
228
229 scoped_refptr<UpdateClient> UpdateClientFactory( 198 scoped_refptr<UpdateClient> UpdateClientFactory(
230 const scoped_refptr<Configurator>& config) { 199 const scoped_refptr<Configurator>& config) {
231 scoped_ptr<PingManager> ping_manager(new PingManager(*config)); 200 scoped_ptr<PingManager> ping_manager(new PingManager(*config));
232 return new UpdateClientImpl(config, ping_manager.Pass(), 201 return new UpdateClientImpl(config, ping_manager.Pass(),
233 &UpdateChecker::Create, &CrxDownloader::Create); 202 &UpdateChecker::Create, &CrxDownloader::Create);
234 } 203 }
235 204
236 } // namespace update_client 205 } // namespace update_client
OLDNEW
« no previous file with comments | « components/update_client/update_client.h ('k') | components/update_client/update_client_internal.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698