Wait for URLBlacklist update tasks on automation tests.

chrome/browser/automation/testing_automation_provider.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/automation/testing_automation_provider.h"
 
 #include <map>
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/file_path.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/json/string_escape.h"
 #include "base/path_service.h"
 #include "base/process.h"
 #include "base/process_util.h"
 #include "base/stringprintf.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/autocomplete/autocomplete.h"
 #include "chrome/browser/autocomplete/autocomplete_edit.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "chrome/browser/autofill/autofill_manager.h"
 #include "chrome/browser/autofill/credit_card.h"
 #include "chrome/browser/autofill/personal_data_manager.h"
@@ skipping 136 matching lines @@
   }
 
   virtual std::string GetHTMLContents() { return contents_; }
 
  private:
   const std::string contents_;
 
   DISALLOW_COPY_AND_ASSIGN(AutomationInterstitialPage);
 };
 
+// Waits for all currently pending tasks on |identifier| to be processed before
+// returning. Returns false if |identifier| is the current thread, or a task
+// could not be posted. Returns true otherwise.
+bool WaitForPendingTasksOn(BrowserThread::ID identifier) {
+  // Avoid deadlocking self.
+  if (BrowserThread::CurrentlyOn(identifier))
+    return false;
+  base::WaitableEvent event(true,    // manual reset
+                            false);  // not initially signaled
+  if (!BrowserThread::PostTask(
+      identifier,
+      FROM_HERE,
+      base::Bind(&base::WaitableEvent::Signal, base::Unretained(&event))))
+    return false;
+  event.Wait();

[Paweł Hajdan Jr., 2011/10/19 09:20:02]

I think this also returns bool, so please return event.Wait in that case.
Otherwise, please ignore.

+  return true;
+}
+
 }  // namespace
 
 TestingAutomationProvider::TestingAutomationProvider(Profile* profile)
     : AutomationProvider(profile),
 #if defined(TOOLKIT_VIEWS)
       popup_menu_waiter_(NULL),
 #endif
       redirect_query_(0) {
   BrowserList::AddObserver(this);
   registrar_.Add(this, chrome::NOTIFICATION_SESSION_END,
@@ skipping 2538 matching lines @@
   reply.SendError("Invalid action");
 }
 
 namespace {
 
 // Task to get info about BrowserChildProcessHost. Must run on IO thread to
 // honor the semantics of BrowserChildProcessHost.
 // 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,
       // since we need their handle.
       if ((*iter)->handle() == base::kNullProcessHandle) {
         continue;
       }
       ChildProcessInfo* info = *iter;
       DictionaryValue* item = new DictionaryValue;
       item->SetString("name", info->name());
       item->SetString("type",
                       ChildProcessInfo::GetTypeNameInEnglish(info->type()));
       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);
 };
 
 }  // namespace
 
 // Sample json input: { "command": "GetBrowserInfo" }
 // Refer to GetBrowserInfo() in chrome/test/pyautolib/pyauto.py for
 // sample json output.
@@ skipping 101 matching lines @@
 #else
   int flags = ChildProcessHost::CHILD_NORMAL;
 #endif
 
   return_value->SetString("child_process_path",
                           ChildProcessHost::GetChildPath(flags).value());
   // Child processes are the processes for plugins and other workers.
   // 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)));
+  CHECK(WaitForPendingTasksOn(BrowserThread::IO));

[jam, 2011/10/19 23:56:27]

instead of blocking one thread on another, why not post a task to the other
thread and then have that post a task back to this thread, and then that second
task is what sends the reply message? this is the pattern that we use all over
chrome

   return_value->Set("child_processes", child_processes);
 
   // Add all extension processes in a list of dictionaries, one dictionary
   // item per extension process.
   ListValue* extension_views = new ListValue;
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   std::vector<Profile*> profiles(profile_manager->GetLoadedProfiles());
   for (size_t i = 0; i < profiles.size(); ++i) {
     ExtensionProcessManager* process_manager =
         profiles[i]->GetExtensionProcessManager();
@@ skipping 2994 matching lines @@
   }
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(providers); ++i) {
     DictionaryValue* policies = NULL;
     if (args->GetDictionary(providers[i].name, &policies) && policies) {
       policy::PolicyMap* map = new policy::PolicyMap;
       map->LoadFrom(policies, list);
       providers[i].provider->OverridePolicies(map);
     }
   }
 
+  // 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.
+  CHECK(WaitForPendingTasksOn(BrowserThread::IO));

[jam, 2011/10/19 23:56:27]

ditto

+  CHECK(WaitForPendingTasksOn(BrowserThread::FILE));
+  CHECK(WaitForPendingTasksOn(BrowserThread::IO));
+
   reply.SendSuccess(NULL);
 #endif  // defined(OFFICIAL_BUILD)
 }
 
 void TestingAutomationProvider::GetPolicyDefinitionList(
     DictionaryValue* args,
     IPC::Message* reply_message) {
   AutomationJSONReply reply(this, reply_message);
 
 #if !defined(ENABLE_CONFIGURATION_POLICY)
@@ skipping 614 matching lines @@
 
   Send(reply_message_);
   redirect_query_ = 0;
   reply_message_ = NULL;
 }
 
 void TestingAutomationProvider::OnRemoveProvider() {
   if (g_browser_process)
     g_browser_process->GetAutomationProviderList()->RemoveProvider(this);
 }