Bug 1621944 - Try-catch all provider calls. r=mak
authorDrew Willcoxon <adw@mozilla.com>
Fri, 20 Mar 2020 20:00:41 +0000
changeset 519910 77710cb1b47488059eaa5410ce64e85398cae74d
parent 519909 5e185a156e46909e96c54da4e5b2f8ae24237c2b
child 519911 8776ab936463c052a24f66aa7d05205bc0c6d081
push id37235
push userccoroiu@mozilla.com
push dateSat, 21 Mar 2020 03:59:46 +0000
treeherdermozilla-central@bbb78f71a5de [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1621944
milestone76.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1621944 - Try-catch all provider calls. r=mak Differential Revision: https://phabricator.services.mozilla.com/D67582
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/UrlbarProvidersManager.jsm
browser/components/urlbar/UrlbarUtils.jsm
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -715,17 +715,17 @@ class UrlbarInput {
           });
           let provider = UrlbarProvidersManager.getProvider(
             result.providerName
           );
           if (!provider) {
             Cu.reportError(`Provider not found: ${result.providerName}`);
             return;
           }
-          provider.pickResult(result);
+          provider.tryMethod("pickResult", result);
           return;
         }
         break;
       }
       case UrlbarUtils.RESULT_TYPE.OMNIBOX: {
         this.controller.engagementEvent.record(event, {
           numChars: this._lastSearchString.length,
           selIndex,
--- a/browser/components/urlbar/UrlbarProvidersManager.jsm
+++ b/browser/components/urlbar/UrlbarProvidersManager.jsm
@@ -182,17 +182,17 @@ class ProvidersManager {
     logger.debug(`Context sources ${queryContext.sources}`);
 
     let query = new Query(queryContext, controller, muxer, providers);
     this.queries.set(queryContext, query);
 
     // Update the behavior of extension providers.
     for (let provider of this.providers) {
       if (provider.type == UrlbarUtils.PROVIDER_TYPE.EXTENSION) {
-        await provider.updateBehavior(queryContext);
+        await provider.tryMethod("updateBehavior", queryContext);
       }
     }
 
     await query.start();
   }
 
   /**
    * Cancels a running query.
@@ -234,17 +234,17 @@ class ProvidersManager {
    * urlbar.
    *
    * @param {boolean} isPrivate True if the engagement is in a private context.
    * @param {string} state The state of the engagement, one of: start,
    *        engagement, abandonment, discard.
    */
   notifyEngagementChange(isPrivate, state) {
     for (let provider of this.providers) {
-      provider.onEngagement(isPrivate, state);
+      provider.tryMethod("onEngagement", isPrivate, state);
     }
   }
 }
 
 var UrlbarProvidersManager = new ProvidersManager();
 
 /**
  * Tracks a query status.
@@ -287,18 +287,18 @@ class Query {
       throw new Error("This Query has been started already");
     }
     this.started = true;
 
     // Check which providers should be queried.
     let providers = [];
     let maxPriority = -1;
     for (let provider of this.providers) {
-      if (provider.isActive(this.context)) {
-        let priority = provider.getPriority(this.context);
+      if (provider.tryMethod("isActive", this.context)) {
+        let priority = provider.tryMethod("getPriority", this.context);
         if (priority >= maxPriority) {
           // The provider's priority is at least as high as the max.
           if (priority > maxPriority) {
             // The provider's priority is higher than the max.  Remove all
             // previously added providers, since their priority is necessarily
             // lower, by setting length to zero.
             providers.length = 0;
             maxPriority = priority;
@@ -325,17 +325,19 @@ class Query {
         // search is canceled, unblocking start().
         this._sleepTimer = new SkippableTimer({
           name: "Query provider timer",
           time: UrlbarPrefs.get("delay"),
           logger,
         });
         await this._sleepTimer.promise;
       }
-      promises.push(provider.startQuery(this.context, this.add.bind(this)));
+      promises.push(
+        provider.tryMethod("startQuery", this.context, this.add.bind(this))
+      );
     }
 
     logger.info(`Queried ${promises.length} providers`);
     if (promises.length) {
       await Promise.all(promises.map(p => p.catch(Cu.reportError)));
 
       if (this._chunkTimer) {
         // All the providers are done returning results, so we can stop chunking.
@@ -356,17 +358,17 @@ class Query {
    * @note Invoking cancel multiple times is a no-op.
    */
   cancel() {
     if (this.canceled) {
       return;
     }
     this.canceled = true;
     for (let provider of this.providers) {
-      provider.cancelQuery(this.context);
+      provider.tryMethod("cancelQuery", this.context);
     }
     if (this._chunkTimer) {
       this._chunkTimer.cancel().catch(Cu.reportError);
     }
     if (this._sleepTimer) {
       this._sleepTimer.fire().catch(Cu.reportError);
     }
   }
--- a/browser/components/urlbar/UrlbarUtils.jsm
+++ b/browser/components/urlbar/UrlbarUtils.jsm
@@ -692,16 +692,36 @@ class UrlbarProvider {
    * The type of the provider, must be one of UrlbarUtils.PROVIDER_TYPE.
    * @abstract
    */
   get type() {
     throw new Error("Trying to access the base class, must be overridden");
   }
 
   /**
+   * Calls a method on the provider in a try-catch block and reports any error.
+   * Unlike most other provider methods, `tryMethod` is not intended to be
+   * overridden.
+   *
+   * @param {string} methodName The name of the method to call.
+   * @param {*} args The method arguments.
+   * @returns {*} The return value of the method, or undefined if the method
+   *          throws an error.
+   * @abstract
+   */
+  tryMethod(methodName, ...args) {
+    try {
+      return this[methodName](...args);
+    } catch (ex) {
+      Cu.reportError(ex);
+    }
+    return undefined;
+  }
+
+  /**
    * Whether this provider should be invoked for the given context.
    * If this method returns false, the providers manager won't start a query
    * with this provider, to save on resources.
    * @param {UrlbarQueryContext} queryContext The query context object
    * @returns {boolean} Whether this provider should be invoked for the search.
    * @abstract
    */
   isActive(queryContext) {