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

Unified Diff: chrome/browser/automation/testing_automation_provider.cc

Issue 8274012: Wait for URLBlacklist update tasks on automation tests. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 9 years, 2 months 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 side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/automation/testing_automation_provider.cc
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
index 50faa3c661a01925dc8d344aa34343863ecf1a71..ab056bb1c509a637e345b8b7c77b5ac70bc539ac 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -176,6 +176,31 @@ class AutomationInterstitialPage : public InterstitialPage {
DISALLOW_COPY_AND_ASSIGN(AutomationInterstitialPage);
};
+// A simple task that just signals the given event. Used to make sure all
+// pending tasks on a message loop have executed after the event is signaled.
+class SignalingTask : public Task {
Paweł Hajdan Jr. 2011/10/13 20:10:59 Huh? This is duplicating an existing class with th
Joao da Silva 2011/10/13 21:32:28 Done.
+ public:
+ explicit SignalingTask(base::WaitableEvent* event) : event_(event) {}
+
+ virtual void Run() OVERRIDE {
+ event_->Signal();
+ }
+
+ private:
+ base::WaitableEvent* const event_; // weak
+
+ DISALLOW_COPY_AND_ASSIGN(SignalingTask);
+};
+
+// Waits until all pending tasks on |id| are executed.
Paweł Hajdan Jr. 2011/10/13 20:10:59 We have lots of existing code that does the same t
Joao da Silva 2011/10/13 21:32:28 Done.
+void WaitForPendingTasksOn(BrowserThread::ID id) {
+ CHECK(!BrowserThread::CurrentlyOn(id));
Paweł Hajdan Jr. 2011/10/13 20:10:59 Instead of CHECKS, I really think you should retur
Joao da Silva 2011/10/13 21:32:28 Done.
+ base::WaitableEvent event(true /* manual reset */,
+ false /* not initially signaled */);
+ CHECK(BrowserThread::PostTask(id, FROM_HERE, new SignalingTask(&event)));
+ event.Wait();
Paweł Hajdan Jr. 2011/10/13 20:10:59 This has a return value, please check it.
Joao da Silva 2011/10/13 21:32:28 Wait() is void. A boolean is returned by TimedWait
+}
+
} // namespace
TestingAutomationProvider::TestingAutomationProvider(Profile* profile)
@@ -2733,12 +2758,10 @@ namespace {
// Used by AutomationProvider::GetBrowserInfo().
class GetChildProcessHostInfoTask : public Task {
public:
- GetChildProcessHostInfoTask(base::WaitableEvent* event,
- ListValue* child_processes)
- : event_(event),
- child_processes_(child_processes) {}
+ explicit GetChildProcessHostInfoTask(ListValue* child_processes)
+ : child_processes_(child_processes) {}
- virtual void Run() {
+ virtual void Run() OVERRIDE {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
for (BrowserChildProcessHost::Iterator iter; !iter.Done(); ++iter) {
// Only add processes which are already started,
@@ -2754,11 +2777,9 @@ class GetChildProcessHostInfoTask : public Task {
item->SetInteger("pid", base::GetProcId(info->handle()));
child_processes_->Append(item);
}
- event_->Signal();
}
private:
- base::WaitableEvent* const event_; // weak
ListValue* child_processes_;
DISALLOW_COPY_AND_ASSIGN(GetChildProcessHostInfoTask);
@@ -2880,12 +2901,10 @@ void TestingAutomationProvider::GetBrowserInfo(
// Add all child processes in a list of dictionaries, one dictionary item
// per child process.
ListValue* child_processes = new ListValue;
- base::WaitableEvent event(true /* manual reset */,
- false /* not initially signaled */);
CHECK(BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
- new GetChildProcessHostInfoTask(&event, child_processes)));
- event.Wait();
+ new GetChildProcessHostInfoTask(child_processes)));
+ WaitForPendingTasksOn(BrowserThread::IO);
return_value->Set("child_processes", child_processes);
// Add all extension processes in a list of dictionaries, one dictionary
@@ -5899,6 +5918,14 @@ void TestingAutomationProvider::SetPolicies(
}
}
+ // OverridePolicies() triggers preference updates, which triggers preference
+ // listeners. Some policies (e.g. URLBlacklist) post tasks to other message
+ // loops before they start being enforced; make sure those tasks have
+ // finished.
+ WaitForPendingTasksOn(BrowserThread::IO);
+ WaitForPendingTasksOn(BrowserThread::FILE);
+ WaitForPendingTasksOn(BrowserThread::IO);
+
reply.SendSuccess(NULL);
#endif // defined(OFFICIAL_BUILD)
}
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698