Refactor the way singleton-style resources are exposed via PPB_Instance

ppapi/proxy/ppb_instance_proxy.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/ppb_instance_proxy.h"
 
+#include "base/memory/ref_counted.h"
 #include "build/build_config.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/pp_time.h"
 #include "ppapi/c/pp_var.h"
 #include "ppapi/c/ppb_audio_config.h"
 #include "ppapi/c/ppb_instance.h"
 #include "ppapi/c/ppb_messaging.h"
 #include "ppapi/c/ppb_mouse_lock.h"
 #include "ppapi/c/private/pp_content_decryptor.h"
 #include "ppapi/proxy/content_decryptor_private_serializer.h"
@@ skipping 307 matching lines @@
   dispatcher()->Send(new PpapiHostMsg_PPBInstance_GetScreenSize(
       API_ID_PPB_INSTANCE, instance, &result, size));
   return result;
 }
 
 thunk::PPB_Flash_API* PPB_Instance_Proxy::GetFlashAPI() {
   InterfaceProxy* ip = dispatcher()->GetInterfaceProxy(API_ID_PPB_FLASH);
   return static_cast<PPB_Flash_Proxy*>(ip);
 }
 
-// TODO(raymes): We can most likely cut down this boilerplate for grabbing
-// singleton resource APIs.
-thunk::PPB_Flash_Functions_API* PPB_Instance_Proxy::GetFlashFunctionsAPI(
-    PP_Instance instance) {
-#if !defined(OS_NACL) && !defined(NACL_WIN64)
+Resource* PPB_Instance_Proxy::GetSingletonResource(PP_Instance instance,
+                                                   SingletonResourceID id) {
   InstanceData* data = static_cast<PluginDispatcher*>(dispatcher())->
       GetInstanceData(instance);
-  if (!data)
+
+  InstanceData::SingletonResourceMap::iterator it =
+      data->singleton_resources.find(id);
+  if (it != data->singleton_resources.end())
+    return it->second.get();
+
+  Resource* new_singleton(NULL);

[dmichael (off chromium), 2012/11/21 17:26:27]

How about making this a scoped_refptr? Just so if you or somebody else comes
along and adds an early return, we won't leak.

[raymes, 2012/11/21 20:59:10]

Done.

+  Connection connection(
+      PluginGlobals::Get()->plugin_proxy_delegate()->GetBrowserSender(),
+      dispatcher());
+
+  switch (id) {
+// Flash resources aren't needed for NaCl.
+#if !defined(OS_NACL) && !defined(NACL_WIN64)
+    case FLASH_SINGLETON_ID:
+      new_singleton = new FlashResource(connection, instance);
+      break;
+    case FLASH_CLIPBOARD_SINGLETON_ID:
+      new_singleton = new FlashClipboardResource(connection, instance);
+      break;
+#endif  // !defined(OS_NACL) && !defined(NACL_WIN64)
+    case GAMEPAD_SINGLETON_ID:
+      new_singleton = new GamepadResource(connection, instance);
+      break;
+    default:
+      // Unfortunately the default case is needed because we ifdef out some of
+      // the cases.

[dmichael (off chromium), 2012/11/21 17:26:27]

What about moving the #ifdef to the end, and having something like:
#else
    case FLASH_SINGLETON_ID:
    case FLASH_CLIPBOARD_SINGLETON_ID:
      NOTREACHED();
#endif

...I really like the compiler warning that points us at pieces of code that need
to be updated when we add an enum value.

[raymes, 2012/11/21 20:59:10]

Done.

+      break;
+  }
+
+  if (!new_singleton) {
+    // Getting here implies that a constructor is missing in the above switch.
+    NOTREACHED();
     return NULL;
+  }
 
-  if (!data->flash_resource.get()) {
-    Connection connection(
-        PluginGlobals::Get()->plugin_proxy_delegate()->GetBrowserSender(),
-        dispatcher());
-    data->flash_resource = new FlashResource(connection, instance);
-  }
-  return data->flash_resource.get();
-#else
-  // Flash functions aren't implemented for nacl.
-  NOTIMPLEMENTED();
-  return NULL;
-#endif  // !defined(OS_NACL) && !defined(NACL_WIN64)
-}
-
-thunk::PPB_Flash_Clipboard_API* PPB_Instance_Proxy::GetFlashClipboardAPI(
-    PP_Instance instance) {
-#if !defined(OS_NACL) && !defined(NACL_WIN64)
-  InstanceData* data = static_cast<PluginDispatcher*>(dispatcher())->
-      GetInstanceData(instance);
-  if (!data)
-    return NULL;
-
-  if (!data->flash_clipboard_resource.get()) {
-    Connection connection(
-        PluginGlobals::Get()->plugin_proxy_delegate()->GetBrowserSender(),
-        dispatcher());
-    data->flash_clipboard_resource =
-        new FlashClipboardResource(connection, instance);
-  }
-  return data->flash_clipboard_resource.get();
-#else
-  // Flash functions aren't implemented for nacl.
-  NOTIMPLEMENTED();
-  return NULL;
-#endif  // !defined(OS_NACL) && !defined(NACL_WIN64)
-}
-
-thunk::PPB_Gamepad_API* PPB_Instance_Proxy::GetGamepadAPI(
-    PP_Instance instance) {
-  InstanceData* data = static_cast<PluginDispatcher*>(dispatcher())->
-      GetInstanceData(instance);
-  if (!data)
-    return NULL;
-
-  if (!data->gamepad_resource.get()) {
-    Connection connection(
-        PluginGlobals::Get()->plugin_proxy_delegate()->GetBrowserSender(),
-        dispatcher());
-    data->gamepad_resource = new GamepadResource(connection, instance);
-  }
-  return data->gamepad_resource.get();
+  data->singleton_resources[id] = scoped_refptr<Resource>(new_singleton);
+  return new_singleton;
 }
 
 int32_t PPB_Instance_Proxy::RequestInputEvents(PP_Instance instance,
                                                uint32_t event_classes) {
   dispatcher()->Send(new PpapiHostMsg_PPBInstance_RequestInputEvents(
       API_ID_PPB_INSTANCE, instance, false, event_classes));
 
   // We always register for the classes we can handle, this function validates
   // the flags so we can notify it if anything was invalid, without requiring
   // a sync reply.
@@ skipping 787 matching lines @@
     PP_Instance instance) {
   InstanceData* data = static_cast<PluginDispatcher*>(dispatcher())->
       GetInstanceData(instance);
   if (!data)
     return;  // Instance was probably deleted.
   data->should_do_request_surrounding_text = false;
 }
 
 }  // namespace proxy
 }  // namespace ppapi