Separate plugin_metadata from plugin_installer, thread-safe plugin_finder

chrome/browser/policy/policy_browsertest.cc

Index: chrome/browser/policy/policy_browsertest.cc
diff --git a/chrome/browser/policy/policy_browsertest.cc b/chrome/browser/policy/policy_browsertest.cc
index f696d0637eacd99c3c8612321e0dafd7949e8ffd..0e94e12f5aeb0567dd82dce3ac3d1f855197475b 100644
--- a/chrome/browser/policy/policy_browsertest.cc
+++ b/chrome/browser/policy/policy_browsertest.cc
@@ -278,12 +278,24 @@ const webkit::WebPluginInfo* GetFlashPlugin(
   return flash;
 }
 
+void PostTaskEnablePlugin(PluginPrefs* plugin_prefs,
+                          const webkit::WebPluginInfo* plugin,
+                          bool enabled,
+                          bool* ok) {
+  plugin_prefs->EnablePlugin(enabled, plugin->path,
+                             base::Bind(CopyValueAndQuit<bool>, ok));
+}
+
 bool SetPluginEnabled(PluginPrefs* plugin_prefs,
                       const webkit::WebPluginInfo* plugin,
                       bool enabled) {
   bool ok = false;
-  plugin_prefs->EnablePlugin(enabled, plugin->path,

[Bernhard Bauer, 2012/09/24 18:06:04]

Wait, I don't understand. We call the callback synchronously (instead of posting
it), and that's why the tests were failing? Couldn't we just remove the
RunMessageLoop() call in that case? To depend on whether we really end up
posting a task to the message loop or not seems kinda clunky.

[ibraaaa, 2012/09/24 18:18:36]

Yes, that is the reason we are failing.

Do you mean updating SetPluginEnabled to run the message loop only when
PluginPrefs#EnablePlugin posts a task?

If yes, then we can but I didn't do it because we will need to update the test
code below to pass in the expected value to SetPluginEnabled + create a special
CopyValueAndQuit to be used for plugin policy tests only that quits the message
loop based on the result, is this ok for you?

On 2012/09/24 18:06:04, Bernhard Bauer wrote:

[Bernhard Bauer, 2012/09/24 18:27:31]

Hm, what I meant was that whether PluginPrefs::EnablePlugin() posts a task or
not is an implementation detail, and the test should not depend on it.

OTOH, maybe we should simplify the behavior of EnablePlugin() and just make sure
that we always post the callback, even if we could do it synchronously. WDYT?

[ibraaaa, 2012/09/24 18:31:26]

Well, I am fine but why do we add a needless callback to the browser code if we
can adapt the tests:), WDYT?

On 2012/09/24 18:27:31, Bernhard Bauer wrote:

[ibraaaa, 2012/09/24 20:07:13]

Done.

-                             base::Bind(CopyValueAndQuit<bool>, &ok));
+  MessageLoop::current()->PostTask(FROM_HERE,
+                                   base::Bind(&PostTaskEnablePlugin,
+                                              base::Unretained(plugin_prefs),
+                                              base::Unretained(plugin),
+                                              enabled,
+                                              base::Unretained(&ok)));
   content::RunMessageLoop();
   return ok;
 }