Bug 641621: Fix bug in which we misinterpret an NPError return value as an nsresult. r=bsmedberg
authorJosh Aas <joshmoz@gmail.com>
Wed, 23 Mar 2011 11:46:09 -0700
changeset 63555 a200ce37157085713e2183471ee6fe192195c182
parent 63554 67ccac595b48a114aff6fa607f2e419031138b2d
child 63556 c2f9c8bad8af40d60ce4af7ea0882c63f5181464
push idunknown
push userunknown
push dateunknown
reviewersbsmedberg
bugs641621
milestone2.2a1pre
Bug 641621: Fix bug in which we misinterpret an NPError return value as an nsresult. r=bsmedberg
layout/generic/nsObjectFrame.cpp
modules/plugin/base/src/nsNPAPIPlugin.cpp
modules/plugin/base/src/nsNPAPIPluginInstance.cpp
modules/plugin/base/src/nsPluginNativeWindowGtk2.cpp
modules/plugin/base/src/nsPluginStreamListenerPeer.cpp
--- a/layout/generic/nsObjectFrame.cpp
+++ b/layout/generic/nsObjectFrame.cpp
@@ -3512,19 +3512,24 @@ NS_IMETHODIMP nsPluginInstanceOwner::Inv
   gfxIntSize oldSize;
   if (container) {
     oldSize = container->GetCurrentSize();
     SetCurrentImage(container);
   }
 
 #ifdef MOZ_USE_IMAGE_EXPOSE
   PRBool simpleImageRender = PR_FALSE;
-  mInstance->GetValueFromPlugin(NPPVpluginWindowlessLocalBool,
-                                &simpleImageRender);
-  if (simpleImageRender) {  
+  nsresult rv = mInstance->GetValueFromPlugin(NPPVpluginWindowlessLocalBool,
+                                              &simpleImageRender);
+  // If the call returned an error code make sure we still use our default value.
+  if (NS_FAILED(rv)) {
+    simpleImageRender = PR_FALSE;
+  }
+
+  if (simpleImageRender) {
     NativeImageDraw(invalidRect);
     return NS_OK;
   }
 #endif
 
 #ifndef XP_MACOSX
   // Windowed plugins should not be calling NPN_InvalidateRect, but
   // Silverlight does and expects it to "work"
@@ -4396,18 +4401,18 @@ void nsPluginInstanceOwner::RenderCoreAn
         delete mIOSurface;
         mIOSurface = NULL;
       }
     }
   }
 
   if (mCARenderer.isInit() == false) {
     void *caLayer = NULL;
-    mInstance->GetValueFromPlugin(NPPVpluginCoreAnimationLayer, &caLayer);
-    if (!caLayer) {
+    nsresult rv = mInstance->GetValueFromPlugin(NPPVpluginCoreAnimationLayer, &caLayer);
+    if (NS_FAILED(rv) || !caLayer) {
       return;
     }
 
     mCARenderer.SetupRenderer(caLayer, aWidth, aHeight);
 
     // Setting up the CALayer requires resetting the painting otherwise we
     // get garbage for the first few frames.
     FixUpPluginWindow(ePluginPaintDisable);
@@ -5951,18 +5956,22 @@ void nsPluginInstanceOwner::Paint(gfxCon
   if (!mInstance || !mObjectFrame)
     return;
 
 #ifdef MOZ_USE_IMAGE_EXPOSE
   // through to be able to paint the context passed in.  This allows
   // us to handle plugins that do not self invalidate (slowly, but
   // accurately), and it allows us to reduce flicker.
   PRBool simpleImageRender = PR_FALSE;
-  mInstance->GetValueFromPlugin(NPPVpluginWindowlessLocalBool,
-                                &simpleImageRender);
+  nsresult rv = mInstance->GetValueFromPlugin(NPPVpluginWindowlessLocalBool,
+                                              &simpleImageRender);
+  // If the call returned an error code make sure we still use our default value.
+  if (NS_FAILED(rv)) {
+    simpleImageRender = PR_FALSE;
+  }
   if (simpleImageRender) {
     gfxMatrix matrix = aContext->CurrentMatrix();
     if (!matrix.HasNonAxisAlignedTransform())
       NativeImageDraw();
     return;
   } 
 #endif
 
@@ -7018,16 +7027,21 @@ nsPluginInstanceOwner::SetAbsoluteScreen
   mAbsolutePositionClip = gfxRect(left, top, width, height);
 
   UpdateVisibility(!(width == 0 && height == 0));
 
   if (!mInstance)
     return NS_OK;
 
   PRBool simpleImageRender = PR_FALSE;
-  mInstance->GetValueFromPlugin(NPPVpluginWindowlessLocalBool,
-                                &simpleImageRender);
-  if (simpleImageRender)
+  nsresult rv = mInstance->GetValueFromPlugin(NPPVpluginWindowlessLocalBool,
+                                              &simpleImageRender);
+  // If the call returned an error code make sure we still use our default value.
+  if (NS_FAILED(rv)) {
+    simpleImageRender = PR_FALSE;
+  }
+  if (simpleImageRender) {
     NativeImageDraw();
+  }
   return NS_OK;
 }
 #endif
 
--- a/modules/plugin/base/src/nsNPAPIPlugin.cpp
+++ b/modules/plugin/base/src/nsNPAPIPlugin.cpp
@@ -2037,17 +2037,21 @@ NPError NP_CALLBACK
   case NPNVxDisplay : {
 #if defined(MOZ_WIDGET_GTK2) || defined(MOZ_WIDGET_QT)
     if (npp) {
       nsNPAPIPluginInstance *inst = (nsNPAPIPluginInstance *) npp->ndata;
       PRBool windowless = PR_FALSE;
       inst->IsWindowless(&windowless);
       NPBool needXEmbed = PR_FALSE;
       if (!windowless) {
-        inst->GetValueFromPlugin(NPPVpluginNeedsXEmbed, &needXEmbed);
+        res = inst->GetValueFromPlugin(NPPVpluginNeedsXEmbed, &needXEmbed);
+        // If the call returned an error code make sure we still use our default value.
+        if (NS_FAILED(res)) {
+          needXEmbed = PR_FALSE;
+        }
       }
       if (windowless || needXEmbed) {
         (*(Display **)result) = mozilla::DefaultXDisplay();
         return NPERR_NO_ERROR;
       }
     }
 #ifdef MOZ_WIDGET_GTK2
     // adobe nppdf calls XtGetApplicationNameAndClass(display,
--- a/modules/plugin/base/src/nsNPAPIPluginInstance.cpp
+++ b/modules/plugin/base/src/nsNPAPIPluginInstance.cpp
@@ -597,29 +597,36 @@ NS_IMETHODIMP nsNPAPIPluginInstance::Get
 #if (MOZ_PLATFORM_MAEMO == 5)
   // The maemo flash plugin does not remember this.  It sets the
   // value, but doesn't support the get value.
   if (variable == NPPVpluginWindowlessLocalBool) {
     *(NPBool*)value = mWindowlessLocal;
     return NS_OK;
   }
 #endif
+
   if (!mPlugin || !mPlugin->GetLibrary())
     return NS_ERROR_FAILURE;
 
   NPPluginFuncs* pluginFunctions = mPlugin->PluginFuncs();
 
   nsresult rv = NS_ERROR_FAILURE;
+
   if (pluginFunctions->getvalue && RUNNING == mRunning) {
     PluginDestructionGuard guard(this);
 
-    NS_TRY_SAFE_CALL_RETURN(rv, (*pluginFunctions->getvalue)(&mNPP, variable, value), this);
+    NPError pluginError = NPERR_GENERIC_ERROR;
+    NS_TRY_SAFE_CALL_RETURN(pluginError, (*pluginFunctions->getvalue)(&mNPP, variable, value), this);
     NPP_PLUGIN_LOG(PLUGIN_LOG_NORMAL,
     ("NPP GetValue called: this=%p, npp=%p, var=%d, value=%d, return=%d\n", 
-    this, &mNPP, variable, value, rv));
+    this, &mNPP, variable, value, pluginError));
+
+    if (pluginError == NPERR_NO_ERROR) {
+      rv = NS_OK;
+    }
   }
 
   return rv;
 }
 
 nsNPAPIPlugin* nsNPAPIPluginInstance::GetPlugin()
 {
   return mPlugin;
--- a/modules/plugin/base/src/nsPluginNativeWindowGtk2.cpp
+++ b/modules/plugin/base/src/nsPluginNativeWindowGtk2.cpp
@@ -73,17 +73,16 @@ private:
   /**
    * Either a GtkSocket or a special GtkXtBin widget (derived from GtkSocket)
    * that encapsulates the Xt toolkit within a Gtk Application.
    */
   GtkWidget* mSocketWidget;
   nsresult  CreateXEmbedWindow();
   nsresult  CreateXtWindow();
   void      SetAllocation();
-  PRBool    CanGetValueFromPlugin(nsCOMPtr<nsIPluginInstance> &aPluginInstance);
 #ifdef MOZ_COMPOSITED_PLUGINS
   nsresult  CreateXCompositedWindow();
   static GdkFilterReturn    plugin_composite_filter_func (GdkXEvent *xevent,
     GdkEvent *event,
     gpointer data);
 
   Damage     mDamage;
   GtkWidget* mParentWindow;
@@ -177,49 +176,55 @@ nsPluginNativeWindowGtk2::plugin_composi
     native_window->mPluginInstance->InvalidateRect(&rect);
 
   return GDK_FILTER_REMOVE;
 }
 #endif
 
 nsresult nsPluginNativeWindowGtk2::CallSetWindow(nsCOMPtr<nsIPluginInstance> &aPluginInstance)
 {
-  if(aPluginInstance) {
+  if (aPluginInstance) {
     if (type == NPWindowTypeWindow) {
-      nsresult rv;
-      if(!mSocketWidget) {
+      if (!mSocketWidget) {
+        nsresult rv;
+
         PRBool needXEmbed = PR_FALSE;
-        if (CanGetValueFromPlugin(aPluginInstance)) {
-          rv = aPluginInstance->GetValueFromPlugin(NPPVpluginNeedsXEmbed, &needXEmbed);
+        rv = aPluginInstance->GetValueFromPlugin(NPPVpluginNeedsXEmbed, &needXEmbed);
+        // If the call returned an error code make sure we still use our default value.
+        if (NS_FAILED(rv)) {
+          needXEmbed = PR_FALSE;
+        }
 #ifdef DEBUG
-          printf("nsPluginNativeWindowGtk2: NPPVpluginNeedsXEmbed=%d\n", needXEmbed);
+        printf("nsPluginNativeWindowGtk2: NPPVpluginNeedsXEmbed=%d\n", needXEmbed);
 #endif
-        }
-        nsresult rv;
-        if(needXEmbed) {
+
+        if (needXEmbed) {
 #ifdef MOZ_COMPOSITED_PLUGINS
           rv = CreateXCompositedWindow();
 #else
           rv = CreateXEmbedWindow();
 #endif
         }
         else {
           rv = CreateXtWindow();
         }
-        if(NS_FAILED(rv))
+
+        if (NS_FAILED(rv)) {
           return NS_ERROR_FAILURE;
+        }
       }
 
-      if(!mSocketWidget)
+      if (!mSocketWidget) {
         return NS_ERROR_FAILURE;
+      }
 
       // Make sure to resize and re-place the window if required.
       // Need to reset "window" each time as nsObjectFrame::DidReflow sets it
       // to the ancestor window.
-      if(GTK_IS_XTBIN(mSocketWidget)) {
+      if (GTK_IS_XTBIN(mSocketWidget)) {
         gtk_xtbin_resize(mSocketWidget, width, height);
         // Point the NPWindow structures window to the actual X window
         window = (void*)GTK_XTBIN(mSocketWidget)->xtwindow;
       }
       else { // XEmbed
         SetAllocation();
         window = (void*)gtk_socket_get_id(GTK_SOCKET(mSocketWidget));
       }
@@ -431,21 +436,16 @@ nsresult nsPluginNativeWindowGtk2::Creat
   mWsInfo.depth = xtbin->xtclient.xtdepth;
   // Leave mWsInfo.type = 0 - Who knows what this is meant to be?
 
   XFlush(mWsInfo.display);
 
   return NS_OK;
 }
 
-PRBool nsPluginNativeWindowGtk2::CanGetValueFromPlugin(nsCOMPtr<nsIPluginInstance> &aPluginInstance)
-{
-  return PR_TRUE;
-}
-
 /* static */
 gboolean
 plug_removed_cb (GtkWidget *widget, gpointer data)
 {
   // Gee, thanks for the info!
   return TRUE;
 }
 
--- a/modules/plugin/base/src/nsPluginStreamListenerPeer.cpp
+++ b/modules/plugin/base/src/nsPluginStreamListenerPeer.cpp
@@ -153,19 +153,23 @@ nsPluginByteRangeStreamListener::OnStart
   PRUint32 responseCode = 0;
   rv = httpChannel->GetResponseStatus(&responseCode);
   if (NS_FAILED(rv)) {
     return NS_ERROR_FAILURE;
   }
   
   if (responseCode != 200) {
     PRBool bWantsAllNetworkStreams = PR_FALSE;
-    pslp->GetPluginInstance()->
-    GetValueFromPlugin(NPPVpluginWantsAllNetworkStreams,
-                       (void*)&bWantsAllNetworkStreams);
+    rv = pslp->GetPluginInstance()->GetValueFromPlugin(NPPVpluginWantsAllNetworkStreams,
+                                                       &bWantsAllNetworkStreams);
+    // If the call returned an error code make sure we still use our default value.
+    if (NS_FAILED(rv)) {
+      bWantsAllNetworkStreams = PR_FALSE;
+    }
+
     if (!bWantsAllNetworkStreams){
       return NS_ERROR_FAILURE;
     }
   }
   
   // if server cannot continue with byte range (206 status) and sending us whole object (200 status)
   // reset this seekable stream & try serve it to plugin instance as a file
   mStreamConverter = finalStreamListener;
@@ -510,17 +514,17 @@ nsPluginStreamListenerPeer::SetupPluginC
 
   return rv;
 }
 
 NS_IMETHODIMP
 nsPluginStreamListenerPeer::OnStartRequest(nsIRequest *request,
                                            nsISupports* aContext)
 {
-  nsresult  rv = NS_OK;
+  nsresult rv = NS_OK;
 
   if (mRequests.IndexOfObject(GetBaseRequest(request)) == -1) {
     NS_ASSERTION(mRequests.Count() == 0,
                  "Only our initial stream should be unknown!");
     TrackRequest(request);
   }
   
   if (mHaveFiredOnStartRequest) {
@@ -544,18 +548,23 @@ nsPluginStreamListenerPeer::OnStartReque
       // return error will cancel this request
       // ...and we also need to tell the plugin that
       mRequestFailed = PR_TRUE;
       return NS_ERROR_FAILURE;
     }
     
     if (responseCode > 206) { // not normal
       PRBool bWantsAllNetworkStreams = PR_FALSE;
-      mPluginInstance->GetValueFromPlugin(NPPVpluginWantsAllNetworkStreams,
-                                          (void*)&bWantsAllNetworkStreams);
+      rv = mPluginInstance->GetValueFromPlugin(NPPVpluginWantsAllNetworkStreams,
+                                               &bWantsAllNetworkStreams);
+      // If the call returned an error code make sure we still use our default value.
+      if (NS_FAILED(rv)) {
+        bWantsAllNetworkStreams = PR_FALSE;
+      }
+
       if (!bWantsAllNetworkStreams) {
         mRequestFailed = PR_TRUE;
         return NS_ERROR_FAILURE;
       }
     }
   }
   
   // do a little sanity check to make sure our frame isn't gone