PPAPI: Fix GetBrowserInterface race conditions

ppapi/proxy/interface_list.cc

 // Copyright (c) 2012 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 "ppapi/proxy/interface_list.h"
 
 #include "base/hash.h"
 #include "base/lazy_instance.h"
 #include "base/memory/singleton.h"
 #include "ppapi/c/dev/ppb_audio_input_dev.h"
@@ skipping 301 matching lines @@
   AddPPP(PPP_VIDEODECODER_DEV_INTERFACE,
          PPP_VideoDecoder_Proxy::GetProxyInterface());
 #endif
 }
 
 InterfaceList::~InterfaceList() {
 }
 
 // static
 InterfaceList* InterfaceList::GetInstance() {
+  // CAUTION: This function is called without the ProxyLock to avoid excessive
+  // excessive locking from C++ wrappers. (See also GetBrowserInterface.)
   return Singleton<InterfaceList>::get();
 }
 
 // static
 void InterfaceList::SetProcessGlobalPermissions(
     const PpapiPermissions& permissions) {
   g_process_global_permissions.Get() = permissions;
 }
 
 InterfaceProxy::Factory InterfaceList::GetFactoryForID(ApiID id) const {
   int index = static_cast<int>(id);
   COMPILE_ASSERT(API_ID_NONE == 0, none_must_be_zero);
   if (id <= 0 || id >= API_ID_COUNT)
     return NULL;
   return id_to_factory_[index];
 }
 
 const void* InterfaceList::GetInterfaceForPPB(const std::string& name) {
+  // CAUTION: This function is called without the ProxyLock to avoid excessive
+  // excessive locking from C++ wrappers. (See also GetBrowserInterface.)
   NameToInterfaceInfoMap::iterator found =
       name_to_browser_info_.find(name);
   if (found == name_to_browser_info_.end())
     return NULL;
 
   if (g_process_global_permissions.Get().HasPermission(
-          found->second.required_permission)) {
-    // Only log interface use once per plugin.
-    if (!found->second.interface_logged) {
+          found->second->required_permission)) {
+    // Only log interface use approximately once per plugin. If the atomic
+    // sequence number rolls over, we might log again, but that's OK.
+    if (found->second->interface_request_count_.GetNext() == 0) {
       PluginGlobals::Get()->GetBrowserSender()->Send(
           new PpapiHostMsg_LogInterfaceUsage(HashInterfaceName(name)));

[dmichael (off chromium), 2014/09/12 18:12:07]

note that previously, even with the "interface_logged" race fixed, we could
still enter GetBrowserSender from multiple threads if the interface name was
different. Hence why I fixed that race too.

-      found->second.interface_logged = true;
     }
-    return found->second.iface;
+    return found->second->iface;
   }
   return NULL;
 }
 
 const void* InterfaceList::GetInterfaceForPPP(const std::string& name) const {
   NameToInterfaceInfoMap::const_iterator found =
       name_to_plugin_info_.find(name);
   if (found == name_to_plugin_info_.end())
     return NULL;
-  return found->second.iface;
+  return found->second->iface;
 }
 
 void InterfaceList::AddProxy(ApiID id,
                              InterfaceProxy::Factory factory) {
   // For interfaces with no corresponding _Proxy objects, the macros will
   // generate calls to this function with API_ID_NONE. This means we
   // should just skip adding a factory for these functions.
   if (id == API_ID_NONE)
     return;
 
   // The factory should be an exact dupe of the one we already have if it
   // has already been registered before.
   int index = static_cast<int>(id);
   DCHECK(!id_to_factory_[index] || id_to_factory_[index] == factory);
 
   id_to_factory_[index] = factory;
 }
 
 void InterfaceList::AddPPB(const char* name,
                            const void* iface,
                            Permission perm) {
   DCHECK(name_to_browser_info_.find(name) == name_to_browser_info_.end());
-  name_to_browser_info_[name] = InterfaceInfo(iface, perm);
+  name_to_browser_info_.add(
+      name, scoped_ptr<InterfaceInfo>(new InterfaceInfo(iface, perm)));
 }
 
 void InterfaceList::AddPPP(const char* name,
                            const void* iface) {
   DCHECK(name_to_plugin_info_.find(name) == name_to_plugin_info_.end());
-  name_to_plugin_info_[name] = InterfaceInfo(iface, PERMISSION_NONE);
+  name_to_plugin_info_.add(
+      name,
+      scoped_ptr<InterfaceInfo>(new InterfaceInfo(iface, PERMISSION_NONE)));
 }
 
-// static
 int InterfaceList::HashInterfaceName(const std::string& name) {
   uint32 data = base::Hash(name.c_str(), name.size());
   // Strip off the signed bit because UMA doesn't support negative values,
   // but takes a signed int as input.
   return static_cast<int>(data & 0x7fffffff);
 }
 
 }  // namespace proxy
 }  // namespace ppapi