Bug 428600 - console spam from nsIHandlerService. r=dmose, sr=biesi, a1.9=damons

uriloader/exthandler/nsExternalHelperAppService.cpp

@@ -905,13 +905,16 @@ nsExternalHelperAppService::GetProtocolHandlerInfo(const nsACString &aScheme,
     return NS_ERROR_FAILURE;
   }
   
-  rv = NS_ERROR_FAILURE;
   nsCOMPtr<nsIHandlerService> handlerSvc = do_GetService(NS_HANDLERSERVICE_CONTRACTID);
-  if (handlerSvc)
+  if (handlerSvc) {
-    rv = handlerSvc->FillHandlerInfo(*aHandlerInfo, EmptyCString());
+    PRBool hasHandler = PR_FALSE;
-
+    (void) handlerSvc->Exists(*aHandlerInfo, &hasHandler);
-  if (NS_SUCCEEDED(rv))
+    if (hasHandler) {
-    return NS_OK;
+      rv = handlerSvc->FillHandlerInfo(*aHandlerInfo, EmptyCString());
+      if (NS_SUCCEEDED(rv))
+        return NS_OK;
+    }
+  }
   
   return SetProtocolHandlerDefaults(*aHandlerInfo, exists);
 }
@@ -2367,8 +2370,15 @@ NS_IMETHODIMP nsExternalHelperAppService::GetFromTypeAndExtension(const nsACStri
   nsresult rv;
   nsCOMPtr<nsIHandlerService> handlerSvc = do_GetService(NS_HANDLERSERVICE_CONTRACTID);
   if (handlerSvc) {
-    rv = handlerSvc->FillHandlerInfo(*_retval, EmptyCString());
+    PRBool hasHandler = PR_FALSE;
-    LOG(("Data source: Via type: retval 0x%08x\n", rv));
+    (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)) {
@@ -2376,7 +2386,10 @@ NS_IMETHODIMP nsExternalHelperAppService::GetFromTypeAndExtension(const nsACStri
       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);
@@ -2455,7 +2468,7 @@ NS_IMETHODIMP nsExternalHelperAppService::GetTypeFromExtension(const nsACString&
   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.

uriloader/exthandler/nsHandlerService.js

@@ -260,13 +260,11 @@ HandlerService.prototype = {
       let protoInfo = protoSvc.getProtocolHandlerInfoFromOS(scheme, 
                                osDefaultHandlerFound);
       
-      try {
+      if (this.exists(protoInfo))
         this.fillHandlerInfo(protoInfo, null);
-      } catch (ex) {
+      else
-        // pick some sane defaults
         protoSvc.setProtocolHandlerDefaults(protoInfo, 
                                             osDefaultHandlerFound.value);
-      }
 
       // cache the possible handlers to avoid extra xpconnect traversals.      
       let possibleHandlers = protoInfo.possibleApplicationHandlers;
@@ -404,8 +402,17 @@ HandlerService.prototype = {
   },
 
   exists: function HS_exists(aHandlerInfo) {
-    var typeID = this._getTypeID(this._getClass(aHandlerInfo), aHandlerInfo.type);
+    var found;
-    return this._hasLiteralAssertion(typeID, NC_VALUE, aHandlerInfo.type);
+
+    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) {
@@ -470,7 +477,7 @@ HandlerService.prototype = {
       return type;
     }
 
-    throw Cr.NS_ERROR_NOT_AVAILABLE;
+    return "";
   },
 
 

uriloader/exthandler/nsIHandlerService.idl

@@ -90,7 +90,9 @@ interface nsIHandlerService : nsISupports
    * @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);
@@ -109,7 +111,8 @@ interface nsIHandlerService : nsISupports
 
   /**
    * 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
    *
@@ -140,7 +143,7 @@ interface nsIHandlerService : nsISupports
    *
    * @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);
 };

uriloader/exthandler/tests/unit/test_handlerService.js

@@ -51,6 +51,9 @@ function run_test() {
   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("");
   
   //**************************************************************************//
@@ -336,4 +339,48 @@ function run_test() {
 
   // 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");
 }