[Extensions] Fix thread-safety bug in PermissionsData

extensions/common/extension_messages.cc

Index: extensions/common/extension_messages.cc
diff --git a/extensions/common/extension_messages.cc b/extensions/common/extension_messages.cc
index 41b8023423f8b4969535e9a37c737551335b6fe1..5307c0bb6343b0cdc8e77c7d1d4fad49831fecb0 100644
--- a/extensions/common/extension_messages.cc
+++ b/extensions/common/extension_messages.cc
@@ -60,8 +60,8 @@ ExtensionMsg_Loaded_Params::ExtensionMsg_Loaded_Params(
       id(extension->id()),
       creation_flags(extension->creation_flags()) {
   if (include_tab_permissions) {
-    const extensions::PermissionsData::TabPermissionsMap& tab_permissions =
-        extension->permissions_data()->tab_specific_permissions();
+    extensions::PermissionsData::TabPermissionsMap tab_permissions =

[Jeffrey Yasskin, 2015/04/23 23:45:41]

This is a whole std::map, so copying it could be pretty expensive. Can you have
the getter do the iteration?

Otherwise, call it CopyTabSpecific...() to be clear there's a pretty high cost.

[Devlin, 2015/04/23 23:54:43]

That was why I changed the name. ;)  By "can you have the getter do the
iteration", are you suggesting having something like a
ForEachTabPermission(const base::Closure& closure);
method on PermissionsData?  If so, I suppose that could work, since this is the
only place we use it.

[Jeffrey Yasskin, 2015/04/24 00:03:55]

Calling a closure under a lock isn't great either, since you can't tell whether
the closure acquires another lock. You could have the PermissionsData do the
whole conversion (since this is the only use of GetTabSpecificPermissionsMap).
I'd be all right calling it "Copy..." too. Get... doesn't look expensive enough.

[Devlin, 2015/04/24 00:17:10]

Let's go with Copy, then.  I don't think PermissionsData should be doing
message-specific stuff.

Done.

+        extension->permissions_data()->GetTabSpecificPermissionsMap();
     for (const auto& pair : tab_permissions) {
       tab_specific_permissions[pair.first] =
           ExtensionMsg_PermissionSetStruct(*pair.second);