Bug 428600 - console spam from nsIHandlerService. r=dmose, sr=biesi, a1.9=damons
authordolske@mozilla.com
Tue, 29 Apr 2008 17:21:01 -0700
changeset 14793 06b2f768aeb580860d292212413c4fd33700878f
parent 14792 1fadcc793c14cdf1188f0299c5dd9a2693be84bd
child 14794 4b35d8a471d6c9d0f73ac98a90638622ddffeb0f
push idunknown
push userunknown
push dateunknown
reviewersdmose, biesi
bugs428600
milestone1.9pre
Bug 428600 - console spam from nsIHandlerService. r=dmose, sr=biesi, a1.9=damons
uriloader/exthandler/nsExternalHelperAppService.cpp
uriloader/exthandler/nsHandlerService.js
uriloader/exthandler/nsIHandlerService.idl
uriloader/exthandler/tests/unit/test_handlerService.js
--- a/uriloader/exthandler/nsExternalHelperAppService.cpp
+++ b/uriloader/exthandler/nsExternalHelperAppService.cpp
@@ -900,23 +900,26 @@ nsExternalHelperAppService::GetProtocolH
 
   PRBool exists;
   nsresult rv = GetProtocolHandlerInfoFromOS(aScheme, &exists, aHandlerInfo);
   if (NS_FAILED(rv)) {
     // Either it knows nothing, or we ran out of memory
     return NS_ERROR_FAILURE;
   }
   
-  rv = NS_ERROR_FAILURE;
   nsCOMPtr<nsIHandlerService> handlerSvc = do_GetService(NS_HANDLERSERVICE_CONTRACTID);
-  if (handlerSvc)
-    rv = handlerSvc->FillHandlerInfo(*aHandlerInfo, EmptyCString());
-
-  if (NS_SUCCEEDED(rv))
-    return NS_OK;
+  if (handlerSvc) {
+    PRBool hasHandler = PR_FALSE;
+    (void) handlerSvc->Exists(*aHandlerInfo, &hasHandler);
+    if (hasHandler) {
+      rv = handlerSvc->FillHandlerInfo(*aHandlerInfo, EmptyCString());
+      if (NS_SUCCEEDED(rv))
+        return NS_OK;
+    }
+  }
   
   return SetProtocolHandlerDefaults(*aHandlerInfo, exists);
 }
 
 NS_IMETHODIMP
 nsExternalHelperAppService::GetProtocolHandlerInfoFromOS(const nsACString &aScheme,
                                                          PRBool *found,
                                                          nsIHandlerInfo **aHandlerInfo)
@@ -2362,26 +2365,36 @@ NS_IMETHODIMP nsExternalHelperAppService
     return NS_ERROR_OUT_OF_MEMORY;
 
   // (2) Now, let's see if we can find something in our datastore
   // This will not overwrite the OS information that interests us
   // (i.e. default application, default app. description)
   nsresult rv;
   nsCOMPtr<nsIHandlerService> handlerSvc = do_GetService(NS_HANDLERSERVICE_CONTRACTID);
   if (handlerSvc) {
-    rv = handlerSvc->FillHandlerInfo(*_retval, EmptyCString());
-    LOG(("Data source: Via type: retval 0x%08x\n", rv));
+    PRBool hasHandler = PR_FALSE;
+    (void) handlerSvc->Exists(*_retval, &hasHandler);
+    if (hasHandler) {
+      rv = handlerSvc->FillHandlerInfo(*_retval, EmptyCString());
+      LOG(("Data source: Via type: retval 0x%08x\n", rv));
+    } else {
+      rv = NS_ERROR_NOT_AVAILABLE;
+    }
+ 
     found = found || NS_SUCCEEDED(rv);
 
     if (!found || NS_FAILED(rv)) {
       // No type match, try extension match
       if (!aFileExt.IsEmpty()) {
         nsCAutoString overrideType;
         rv = handlerSvc->GetTypeFromExtension(aFileExt, overrideType);
-        if (NS_SUCCEEDED(rv)) {
+        if (NS_SUCCEEDED(rv) && !overrideType.IsEmpty()) {
+          // We can't check handlerSvc->Exists() here, because we have a
+          // overideType. That's ok, it just results in some console noise.
+          // (If there's no handler for the override type, it throws)
           rv = handlerSvc->FillHandlerInfo(*_retval, overrideType);
           LOG(("Data source: Via ext: retval 0x%08x\n", rv));
           found = found || NS_SUCCEEDED(rv);
         }
       }
     }
   }
 
@@ -2450,17 +2463,17 @@ NS_IMETHODIMP nsExternalHelperAppService
       return rv;
     }
   }
 
   // Check user-set prefs
   nsCOMPtr<nsIHandlerService> handlerSvc = do_GetService(NS_HANDLERSERVICE_CONTRACTID);
   if (handlerSvc)
     rv = handlerSvc->GetTypeFromExtension(aFileExt, aContentType);
-  if (NS_SUCCEEDED(rv))
+  if (NS_SUCCEEDED(rv) && !aContentType.IsEmpty())
     return NS_OK;
 
   // Ask OS.
   PRBool found = PR_FALSE;
   nsCOMPtr<nsIMIMEInfo> mi = GetMIMEInfoFromOS(EmptyCString(), aFileExt, &found);
   if (mi && found)
     return mi->GetMIMEType(aContentType);
 
--- a/uriloader/exthandler/nsHandlerService.js
+++ b/uriloader/exthandler/nsHandlerService.js
@@ -255,23 +255,21 @@ HandlerService.prototype = {
       // nsIExternalProtocolHandlerService.getProtocolHandlerInfo().
       // Necessary because calling that from here would make XPConnect barf
       // when getService tried to re-enter the constructor for this
       // service.
       let osDefaultHandlerFound = {};
       let protoInfo = protoSvc.getProtocolHandlerInfoFromOS(scheme, 
                                osDefaultHandlerFound);
       
-      try {
+      if (this.exists(protoInfo))
         this.fillHandlerInfo(protoInfo, null);
-      } catch (ex) {
-        // pick some sane defaults
+      else
         protoSvc.setProtocolHandlerDefaults(protoInfo, 
                                             osDefaultHandlerFound.value);
-      }
 
       // cache the possible handlers to avoid extra xpconnect traversals.      
       let possibleHandlers = protoInfo.possibleApplicationHandlers;
 
       for each (var handlerPrefs in schemes[scheme]) {
 
         let handlerApp = Cc["@mozilla.org/uriloader/web-handler-app;1"].
                          createInstance(Ci.nsIWebHandlerApp);
@@ -399,18 +397,27 @@ HandlerService.prototype = {
 
     // Write the changes to the database immediately so we don't lose them
     // if the application crashes.
     if (this._ds instanceof Ci.nsIRDFRemoteDataSource)
       this._ds.Flush();
   },
 
   exists: function HS_exists(aHandlerInfo) {
-    var typeID = this._getTypeID(this._getClass(aHandlerInfo), aHandlerInfo.type);
-    return this._hasLiteralAssertion(typeID, NC_VALUE, aHandlerInfo.type);
+    var found;
+
+    try {
+      var typeID = this._getTypeID(this._getClass(aHandlerInfo), aHandlerInfo.type);
+      found = this._hasLiteralAssertion(typeID, NC_VALUE, aHandlerInfo.type);
+    } catch (e) {
+      // If the RDF threw (eg, corrupt file), treat as non-existent.
+      found = false;
+    }
+
+    return found;
   },
 
   remove: function HS_remove(aHandlerInfo) {
     var preferredHandlerID =
       this._getPreferredHandlerID(this._getClass(aHandlerInfo), aHandlerInfo.type);
     this._removeAssertions(preferredHandlerID);
 
     var infoID = this._getInfoID(this._getClass(aHandlerInfo), aHandlerInfo.type);
@@ -465,17 +472,17 @@ HandlerService.prototype = {
 
     if (typeID && this._hasValue(typeID, NC_VALUE)) {
       let type = this._getValue(typeID, NC_VALUE);
       if (type == "")
         throw Cr.NS_ERROR_FAILURE;
       return type;
     }
 
-    throw Cr.NS_ERROR_NOT_AVAILABLE;
+    return "";
   },
 
 
   //**************************************************************************//
   // Retrieval Methods
 
   /**
    * Retrieve the preferred action for the info record with the given ID.
--- a/uriloader/exthandler/nsIHandlerService.idl
+++ b/uriloader/exthandler/nsIHandlerService.idl
@@ -85,17 +85,19 @@ interface nsIHandlerService : nsISupport
    * In that situation, you can call this method to fill in the handler info
    * object with information about that other type by passing the other type
    * as the aOverrideType parameter.
    *
    * @param aHandlerInfo   the handler info object
    * @param aOverrideType  a type to use instead of aHandlerInfo::type
    *
    * Note: if there is no information in the datastore about this type,
-   * this method throws NS_ERROR_NOT_AVAILABLE.
+   * this method throws NS_ERROR_NOT_AVAILABLE. Callers are encouraged to
+   * check exists() before calling fillHandlerInfo(), to prevent spamming the
+   * console with XPCOM exception errors.
    */
   void fillHandlerInfo(in nsIHandlerInfo aHandlerInfo,
                        in ACString aOverrideType);
 
   /**
    * Save the preferred action, preferred handler, possible handlers, and
    * always ask properties of the given handler info object to the datastore.
    * Updates an existing record or creates a new one if necessary.
@@ -104,17 +106,18 @@ interface nsIHandlerService : nsISupport
    * the default value nsIHandlerInfo::useHelperApp.
    *
    * @param aHandlerInfo  the handler info object
    */
   void store(in nsIHandlerInfo aHandlerInfo);
 
   /**
    * Whether or not a record for the given handler info object exists
-   * in the datastore.
+   * in the datastore. If the datastore is corrupt (or some other error
+   * is caught in the implementation), false will be returned.
    *
    * @param aHandlerInfo  a handler info object
    *
    * @returns whether or not a record exists
    */
   boolean exists(in nsIHandlerInfo aHandlerInfo);
 
   /**
@@ -135,12 +138,12 @@ interface nsIHandlerService : nsISupport
    *
    * Note: in general, you should use nsIMIMEService::getTypeFromExtension
    * to get a MIME type from a file extension, as that method checks a variety
    * of other sources besides just the datastore.  Use this only when you want
    * to specifically get only the mapping available in the datastore.
    *
    * @param aFileExtension  the file extension
    *
-   * @returns the MIME type, if any; otherwise throws NS_ERROR_NOT_AVAILABLE
+   * @returns the MIME type, if any; otherwise returns an empty string ("").
    */
   ACString getTypeFromExtension(in ACString aFileExtension);
 };
--- a/uriloader/exthandler/tests/unit/test_handlerService.js
+++ b/uriloader/exthandler/tests/unit/test_handlerService.js
@@ -46,16 +46,19 @@ function run_test() {
                   getService(Ci.nsIMIMEService);
 
   const protoSvc = Cc["@mozilla.org/uriloader/external-protocol-service;1"].
                    getService(Ci.nsIExternalProtocolService);
   
   const prefSvc = Cc["@mozilla.org/preferences-service;1"].
                   getService(Ci.nsIPrefService);
                   
+  const ioService = Cc["@mozilla.org/network/io-service;1"].
+                    getService(Ci.nsIIOService);
+
   const rootPrefBranch = prefSvc.getBranch("");
   
   //**************************************************************************//
   // Sample Data
 
   // It doesn't matter whether or not this nsIFile is actually executable,
   // only that it has a path and exists.  Since we don't know any executable
   // that exists on all platforms (except possibly the application being
@@ -331,9 +334,53 @@ function run_test() {
   // Make sure the handler is the one we didn't remove.
   webPossibleHandler = possibleHandlersInfo.possibleApplicationHandlers.
                        queryElementAt(0, Ci.nsIWebHandlerApp);
   do_check_eq(webPossibleHandler.name, webHandler.name);
   do_check_true(webPossibleHandler.equals(webHandler));
 
   // FIXME: test round trip integrity for a protocol.
   // FIXME: test round trip integrity for a handler info with a web handler.
+
+  //**************************************************************************//
+  // getTypeFromExtension tests
+
+  // test non-existent extension
+  var lolType = handlerSvc.getTypeFromExtension("lolcat");
+  do_check_eq(lolType, "");
+
+
+  // add a handler for the extension
+  var lolHandler = mimeSvc.getFromTypeAndExtension("application/lolcat", null);
+
+  do_check_false(lolHandler.extensionExists("lolcat"));
+  lolHandler.preferredAction = Ci.nsIHandlerInfo.useHelperApp;
+  lolHandler.preferredApplicationHandler = localHandler;
+  lolHandler.alwaysAskBeforeHandling = false;
+
+  // store the handler
+  do_check_false(handlerSvc.exists(lolHandler));
+  handlerSvc.store(lolHandler);
+  do_check_true(handlerSvc.exists(lolHandler));
+
+  // Get a file:// string pointing to mimeTypes.rdf
+  var rdfFile = HandlerServiceTest._dirSvc.get("UMimTyp", Ci.nsIFile);
+  var fileHandler = ioService.getProtocolHandler("file").QueryInterface(Ci.nsIFileProtocolHandler);
+  var rdfFileURI = fileHandler.getURLSpecFromFile(rdfFile);
+
+  // Assign a file extenstion to the handler. handlerSvc.store() doesn't
+  // actually store any file extensions added with setFileExtensions(), you
+  // have to wade into RDF muck to do so.
+
+  // Based on toolkit/mozapps/downloads/content/helperApps.js :: addExtension()
+  var gRDF = Cc["@mozilla.org/rdf/rdf-service;1"].getService(Ci.nsIRDFService);
+  var mimeSource    = gRDF.GetUnicodeResource("urn:mimetype:application/lolcat");
+  var valueProperty = gRDF.GetUnicodeResource("http://home.netscape.com/NC-rdf#fileExtensions");
+  var mimeLiteral   = gRDF.GetLiteral("lolcat");
+
+  var DS = gRDF.GetDataSourceBlocking(rdfFileURI);
+  DS.Assert(mimeSource, valueProperty, mimeLiteral, true);
+
+
+  // test now-existent extension
+  lolType = handlerSvc.getTypeFromExtension("lolcat");
+  do_check_eq(lolType, "application/lolcat");
 }