Commit a1ab1d2f authored by Drew Willcoxon's avatar Drew Willcoxon
Browse files

Bug 1746459 - Wait for Firefox Suggest scenario initialization before trying... 

Bug 1746459 - Wait for Firefox Suggest scenario initialization before trying to show the online opt-in dialog. r=daisuke

The problem is a race between scenario initialization and
`maybeShowOnboardingDialog()` on startup. If `maybeShowOnboardingDialog()` is
called before scenario initialization finishes, then the
`quickSuggestShouldShowOnboardingDialog` Nimbus variable will retain its false
default value, and the dialog will not be shown like it should be. Scenario
initialization depends on the Nimbus urlbar feature initialization, which
depends on loading the feature JSON from disk, so it's plausible this race is
more likely to happen on slow machines, as we've seen with QA's slow Ubuntu
machines in other bugs.

This revision fixes it by making a promise that's resolved when scenario
initialization finishes, and then `maybeShowOnboardingDialog()` awaits the
promise before starting.

I thought about adding an idle callback for `maybeShowOnboardingDialog()`
directly in `UrlbarPrefs.updateFirefoxSuggestScenario()`. However, that would
change the timing of the dialog more than I'm comfortable with, since right now
scenario initializaton happens before BrowserGlue adds its idle callback for
`maybeShowOnboardingDialog()`. We can maybe revisit startup and initialization
at some point to make this a little nicer.

Differential Revision: https://phabricator.services.mozilla.com/D134553
parent 2d6b0d32

browser/components/urlbar/UrlbarPrefs.jsm

+23 −0
Original line number Diff line number Diff line
@@ -496,6 +496,12 @@ class Preferences { 
      "suggest.searches",

    ];



    // This is resolved when the first update to the Firefox Suggest scenario

    // (on startup) finishes.

    this._firefoxSuggestScenarioStartupPromise = new Promise(

      resolve => (this._resolveFirefoxSuggestScenarioStartupPromise = resolve)

    );



    // This is set to true when we update the Firefox Suggest scenario to

    // prevent re-entry due to pref observers.

    this._updatingFirefoxSuggestScenario = false;
@@ -631,6 +637,13 @@ class Preferences { 
    } finally {

      this._updatingFirefoxSuggestScenario = false;

    }



    // Null check `_resolveFirefoxSuggestScenarioStartupPromise` since some

    // tests force `isStartup` after actual startup and it's been set to null.

    if (isStartup && this._resolveFirefoxSuggestScenarioStartupPromise) {

      this._resolveFirefoxSuggestScenarioStartupPromise();

      this._resolveFirefoxSuggestScenarioStartupPromise = null;

    }

  }



  _updateFirefoxSuggestScenarioHelper(isStartup, testOverrides) {
@@ -1039,6 +1052,16 @@ class Preferences { 
    }

  }



  /**

   * @returns {Promise}

   *   This can be used to wait until the initial Firefox Suggest scenario has

   *   been set on startup. It's resolved when the first call to

   *   `updateFirefoxSuggestScenario()` finishes.

   */

  get firefoxSuggestScenarioStartupPromise() {

    return this._firefoxSuggestScenarioStartupPromise;

  }



  /**

   * @returns {boolean}

   *   Whether the Firefox Suggest scenario is being updated. While true,

browser/components/urlbar/UrlbarQuickSuggest.jsm

+5 −0
Original line number Diff line number Diff line
@@ -203,6 +203,11 @@ class Suggestions { 
   *   True if the dialog was shown and false if not.

   */

  async maybeShowOnboardingDialog() {

    // The call to this method races scenario initialization on startup, and the

    // Nimbus variables we rely on below depend on the scenario, so wait for it

    // to be initialized.

    await UrlbarPrefs.firefoxSuggestScenarioStartupPromise;



    // If quicksuggest is not available, the onboarding dialog is configured to

    // be skipped, the user has already seen the dialog, or has otherwise opted

    // in already, then we won't show the quicksuggest onboarding.