Bug 666938 - Add a console message when a binary component doesn't load because of a kVersion mismatch, r=Mossop
authorBenjamin Smedberg <benjamin@smedbergs.us>
Fri, 24 Jun 2011 14:08:13 -0400
changeset 72213 3b245f7f94d7fbcc652a08544d7993713d44b050
parent 72212 35178797a902c748b09c8b9894507c80d6a5f58d
child 72214 7313d4e3547f1abc2d2428b20abbfce698e52ef6
push id209
push userbzbarsky@mozilla.com
push dateTue, 05 Jul 2011 17:42:16 +0000
treeherdermozilla-aurora@cc6e30cce8af [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMossop
bugs666938
milestone7.0a1
Bug 666938 - Add a console message when a binary component doesn't load because of a kVersion mismatch, r=Mossop
xpcom/components/nsComponentManager.cpp
xpcom/components/nsNativeComponentLoader.cpp
xpcom/tests/unit/test_bug656331.js
--- a/xpcom/components/nsComponentManager.cpp
+++ b/xpcom/components/nsComponentManager.cpp
@@ -725,16 +725,17 @@ nsComponentManagerImpl::ManifestBinaryCo
     if (mKnownFileModules.Get(h)) {
         NS_WARNING("Attempting to register a binary component twice.");
         LogMessageWithContext(cx.mFile, cx.mPath, lineno,
                               "Attempting to register a binary component twice.");
         return;
     }
 
     const mozilla::Module* m = mNativeModuleLoader.LoadModule(clfile);
+    // The native module loader should report an error here, we don't have to
     if (!m)
         return;
 
     RegisterModule(m, clfile);
 }
 
 void
 nsComponentManagerImpl::ManifestXPT(ManifestProcessingContext& cx, int lineno, char *const * argv)
--- a/xpcom/components/nsNativeComponentLoader.cpp
+++ b/xpcom/components/nsNativeComponentLoader.cpp
@@ -53,16 +53,17 @@
 
 #include "nsNativeComponentLoader.h"
 
 #include "prlog.h"
 #include "prinit.h"
 #include "prerror.h"
 
 #include "nsComponentManager.h"
+#include "ManifestParser.h" // for LogMessage
 #include "nsCRTGlue.h"
 #include "nsThreadUtils.h"
 #include "nsTraceRefcntImpl.h"
 
 #include "nsILocalFile.h"
 #include "nsIProxyObjectManager.h"
 
 #ifdef XP_WIN
@@ -163,27 +164,18 @@ nsNativeModuleLoader::LoadModule(nsILoca
     rv = aFile->Load(&data.library);
 
     if (NS_FAILED(rv)) {
         char errorMsg[1024] = "<unknown; can't get error from NSPR>";
 
         if (PR_GetErrorTextLength() < (int) sizeof(errorMsg))
             PR_GetErrorText(errorMsg);
 
-        LOG(PR_LOG_ERROR,
-            ("nsNativeModuleLoader::LoadModule(\"%s\") - load FAILED, "
-             "rv: %lx, error:\n\t%s\n",
-             filePath.get(), rv, errorMsg));
-
-#ifdef DEBUG
-        fprintf(stderr,
-                "nsNativeModuleLoader::LoadModule(\"%s\") - load FAILED, "
-                "rv: %lx, error:\n\t%s\n",
-                filePath.get(), (unsigned long)rv, errorMsg);
-#endif
+        LogMessage("Failed to load native module at path '%s': (%lx) %s",
+                   filePath.get(), rv, errorMsg);
 
         return NULL;
     }
 
 #ifdef IMPLEMENT_BREAK_AFTER_LOAD
     nsCAutoString leafName;
     aFile->GetNativeLeafName(leafName);
 
@@ -197,33 +189,34 @@ nsNativeModuleLoader::LoadModule(nsILoca
             }
         }
 
         free(blist);
     }
 #endif
 
     void *module = PR_FindSymbol(data.library, "NSModule");
-    if (module) {
-        data.module = *(mozilla::Module const *const *) module;
-        if (mozilla::Module::kVersion == data.module->mVersion &&
-            mLibraries.Put(hashedFile, data))
-            return data.module;
-    }
-    else {
-        LOG(PR_LOG_ERROR,
-            ("nsNativeModuleLoader::LoadModule(\"%s\") - "
-             "Symbol NSModule not found", filePath.get()));
+    if (!module) {
+        LogMessage("Native module at path '%s' doesn't export symbol `NSModule`.",
+                   filePath.get());
+        PR_UnloadLibrary(data.library);
+        return NULL;
     }
 
-    // at some point we failed, clean up
-    data.module = nsnull;
-    PR_UnloadLibrary(data.library);
-
-    return NULL;
+    data.module = *(mozilla::Module const *const *) module;
+    if (mozilla::Module::kVersion != data.module->mVersion) {
+        LogMessage("Native module at path '%s' is incompatible with this version of Firefox, has version %i, expected %i.",
+                   filePath.get(), data.module->mVersion,
+                   mozilla::Module::kVersion);
+        PR_UnloadLibrary(data.library);
+        return NULL;
+    }
+        
+    mLibraries.Put(hashedFile, data); // infallible
+    return data.module;
 }
 
 const mozilla::Module*
 nsNativeModuleLoader::LoadModuleFromJAR(nsILocalFile* aJARFile, const nsACString &aPath)
 {
     NS_ERROR("Binary components cannot be loaded from JARs");
     return NULL;
 }
--- a/xpcom/tests/unit/test_bug656331.js
+++ b/xpcom/tests/unit/test_bug656331.js
@@ -1,6 +1,39 @@
+Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
+
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+
+function info(s) {
+  dump("TEST-INFO | test_bug656331.js | " + s + "\n");
+}
+
+var gMessageExpected = /Native module.*has version 3.*expected/;
+var gFound = false;
+
+const kConsoleListener = {
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIConsoleListener]),
+  
+  observe: function listener_observe(message) {
+    if (gMessageExpected.test(message.message))
+      gFound = true;
+  }
+};
+
 function run_test() {
-  manifest = do_get_file('bug656331.manifest');
+  let cs = Components.classes["@mozilla.org/consoleservice;1"].
+    getService(Ci.nsIConsoleService);
+  cs.registerListener(kConsoleListener);
+
+  let manifest = do_get_file('bug656331.manifest');
   Components.manager.autoRegister(manifest);
 
   do_check_false("{f18fb09b-28b4-4435-bc5b-8027f18df743}" in Components.classesByID);
+
+  do_test_pending();
+  Components.classes["@mozilla.org/thread-manager;1"].
+    getService(Ci.nsIThreadManager).mainThread.dispatch(function() {
+      cs.unregisterListener(kConsoleListener);
+      do_check_true(gFound);
+      do_test_finished();
+    }, 0);
 }