Be more thorough checking for NULL NPP values

webkit/plugins/npapi/plugin_host.cc

Index: webkit/plugins/npapi/plugin_host.cc
diff --git a/webkit/plugins/npapi/plugin_host.cc b/webkit/plugins/npapi/plugin_host.cc
index 9aed03b2d44853faccb3456209aa3307a42d8666..5fd6492fb77c30f59ea30e848c53a5b98a0be6f0 100644
--- a/webkit/plugins/npapi/plugin_host.cc
+++ b/webkit/plugins/npapi/plugin_host.cc
@@ -693,6 +693,10 @@ NPError NPN_GetValue(NPP id, NPNVariable variable, void* value) {
   switch (static_cast<int>(variable)) {
     case NPNVWindowNPObject: {
       scoped_refptr<PluginInstance> plugin(FindInstance(id));
+      if (!plugin.get()) {

[ananta, 2011/03/23 18:39:26]

Should we move this code outside the switch block to reduce the clutter?

[davidben, 2011/03/23 18:51:08]

Some variables don't seem to require a plugin instance. For instance,
NPNVjavascriptEnabledBool and NPNVSupportsWindowless. I don't know if
"officially" you're allowed to query them with a null instance, or if plugins
actually do, but we and Mozilla both accept it, so it's conceivable that someone
does.

(Hence why I'm not very thrilled about doing the check on nspluginwrapper's end,
if it does in fact depend on the particular variable.)

+        NOTREACHED();
+        return NPERR_INVALID_INSTANCE_ERROR;
+      }
       NPObject *np_object = plugin->webplugin()->GetWindowScriptNPObject();
       // Return value is expected to be retained, as
       // described here:
@@ -709,6 +713,10 @@ NPError NPN_GetValue(NPP id, NPNVariable variable, void* value) {
     }
     case NPNVPluginElementNPObject: {
       scoped_refptr<PluginInstance> plugin(FindInstance(id));
+      if (!plugin.get()) {
+        NOTREACHED();
+        return NPERR_INVALID_INSTANCE_ERROR;
+      }
       NPObject *np_object = plugin->webplugin()->GetPluginElement();
       // Return value is expected to be retained, as
       // described here:
@@ -728,7 +736,7 @@ NPError NPN_GetValue(NPP id, NPNVariable variable, void* value) {
       scoped_refptr<PluginInstance> plugin = FindInstance(id);
       if (!plugin.get()) {
         NOTREACHED();
-        return NPERR_GENERIC_ERROR;
+        return NPERR_INVALID_INSTANCE_ERROR;
       }
       gfx::PluginWindowHandle handle = plugin->window_handle();
       *((void**)value) = (void*)handle;
@@ -763,6 +771,10 @@ NPError NPN_GetValue(NPP id, NPNVariable variable, void* value) {
     case NPNVprivateModeBool: {
       NPBool* private_mode = reinterpret_cast<NPBool*>(value);
       scoped_refptr<PluginInstance> plugin(FindInstance(id));
+      if (!plugin.get()) {
+        NOTREACHED();
+        return NPERR_INVALID_INSTANCE_ERROR;
+      }
       *private_mode = plugin->webplugin()->IsOffTheRecord();
       rv = NPERR_NO_ERROR;
       break;
@@ -778,6 +790,10 @@ NPError NPN_GetValue(NPP id, NPNVariable variable, void* value) {
       // with the variable definition, in order to avoid duplicate case clauses
       // in this big switch statement.
       scoped_refptr<PluginInstance> plugin(FindInstance(id));
+      if (!plugin.get()) {
+        NOTREACHED();
+        return NPERR_INVALID_INSTANCE_ERROR;
+      }
       if (plugin->plugin_lib()->plugin_info().path.value() ==
             webkit::npapi::kDefaultPluginLibraryName) {
         plugin->webplugin()->OnMissingPluginStatus(variable -
@@ -789,6 +805,10 @@ NPError NPN_GetValue(NPP id, NPNVariable variable, void* value) {
     case NPNVpluginDrawingModel: {
       // return the drawing model that was negotiated when we initialized.
       scoped_refptr<PluginInstance> plugin(FindInstance(id));
+      if (!plugin.get()) {
+        NOTREACHED();
+        return NPERR_INVALID_INSTANCE_ERROR;
+      }
       *reinterpret_cast<int*>(value) = plugin->drawing_model();
       rv = NPERR_NO_ERROR;
       break;
@@ -869,6 +889,10 @@ NPError NPN_SetValue(NPP id, NPPVariable variable, void* value) {
   // Allows the plugin to set various modes
 
   scoped_refptr<PluginInstance> plugin(FindInstance(id));
+  if (!plugin.get()) {
+    NOTREACHED();
+    return NPERR_INVALID_INSTANCE_ERROR;
+  }
   switch(variable) {
     case NPPVpluginWindowBool: {
       // Sets windowless mode for display of the plugin