1621944 - [Intermittent] The onboarding tip is displayed in about:newtab only after clicking in the Address Bar at browser normal restart

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[Valentina Virlics [:vvalentina], Ecosystem QA]

Attachment: onboarding tip.mov

[Affected versions]:

• Firefox Release 74 Build ID 20200309095159;

[Affected Platforms]:

• Windows 10.14
• Mac 10.14;
• Linux Debian 9;

[Prerequisites]:

• Have a Firefox Release 74 build installed;
• Have a profile created in the last 24 hours;
• Firefox is opened and wasn't updated in the last 24 hours;
• Firefox is set as your default browser;
• “Restore previous session” option is unchecked;
• Have a Normandy testing recipe created and published for the Quantumbar V1 experiment;
• “user.js” file is saved in the profile directory;
• Google is the default search engine;
• Be enrolled in the “treatment” branch;

[Steps to reproduce]:

1. Open the Firefox browser with the profile from prerequisites.
2. Close the profile normally. (click on the “X” button)
3. Start the browser using the same profile.
4. Observe the onboarding tip.

[Expected result]:

• The onboarding tip is displayed.

[Actual result]:

• The onboarding tip is NOT displayed.

[Notes]:

• Clicking in the Address Bar will trigger the onboarding tip.
• Issue is constantly reproducible on Mac OS and Linux, and intermittently on Windows.
• Not reproducible for other search engines.
• Reproduced the issue using “en-US” and “es-ES” builds.

Comment 1

[Marco Bonardo [:mak]]

Drew, could you or Harry have a look at what's up here?

Comment 2

[Drew Willcoxon :adw]

I was able to reproduce this just by modifying a mozilla-release build to default all the update1 prefs to true. I also reproduced it by running the actual Release Channel Test of QuantumBar v1 (recipe 925) and then manually setting the update1.searchTips pref to true. In both cases, this error happens:

JavaScript error: resource:///modules/UrlbarProviderTopSites.jsm, line 82: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]

This is an error in the top sites provider, which apparently prevents the search tips provider from subsequently running.

I can't reproduce this on 75, so it looks like this might have been fixed somehow, but it does indicate that search tips would not work as expected if we would have enabled them on 74. I'm still trying to figure out why this pref access fails in the top sites provider.

Hopefully this is all moot since we won't enable search tips at all on 74, not even in the experiment, but I'd like to find out why the pref access fails in case it matters for 75 and later.

Comment 3

[Drew Willcoxon :adw]

OK, this looks like a real problem due to the combination of the search tips and top sites providers, and it happened to be fixed by bug 1619547. That's why this is a problem on 74 but not 75.

browser.newtabpage.activity-stream.feeds.topsites is not defined in firefox.js, so I presume it must be programmatically created by activity stream at some point. It's hard to tell. So I'm guessing that there's some window of time at startup where the pref hasn't been created yet.

When the browser starts up, the search tips provider gets notified about a location change, so it checks to see whether it should show a tip. When the selected tab on startup is about:newtab, the answer is yes unless some other condition isn't met. So it starts a search to show a tip, which triggers calls to provider.isActive for all providers. That's how the top site provider's isActive is called.

At that point, the pref hasn't been created yet, so top sites's isActive throws, which short circuits the search.

Bug 1619547 fixed this on 75 because it added a small timeout before the search tips provider starts a search.

I think we should at least modify the pref get to also pass a default value so that it doesn't throw. We may also want to try-catch provider.isActive calls and other calls to providers? Not sure whether that's a good idea.

Comment 4

[Drew Willcoxon :adw]

Attachment: Bug 1621944 - Make the top sites provider not assume the activity stream top sites pref exists.

Comment 5

[Drew Willcoxon :adw]

Attachment: Bug 1621944 - Try-catch all provider calls.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e185a156e46
Make the top sites provider not assume the activity stream top sites pref exists. r=harry
https://hg.mozilla.org/integration/autoland/rev/77710cb1b474
Try-catch all provider calls. r=mak

Comment 7

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/5e185a156e46
https://hg.mozilla.org/mozilla-central/rev/77710cb1b474

Comment 8

[Drew Willcoxon :adw]

I'd like to uplift the pref getter fix. The only reason people wouldn't hit this bug is that the timeout after which the tip is shown just happens to be longer than the time it takes for activity stream to define the pref. That's not very reassuring. The other fix can ride the trains.

Comment 9

[Drew Willcoxon :adw]

Comment on attachment 9134570 [details]
Bug 1621944 - Make the top sites provider not assume the activity stream top sites pref exists.

Beta/Release Uplift Approval Request

• User impact if declined: Some users may see this bug on 75, and 75 is where this feature is debuting. There's chance involved, so I can't say how many users -- maybe a lot, maybe few.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Hitting the circumstances for this bug is due to chance and is probably hard to trigger, but you can verify that the search tip appears correctly:

1. Open about:config and verify that the following prefs are true:
   • browser.urlbar.update1
   • browser.urlbar.update1.searchTips
2. Set the pref browser.urlbar.searchTips.test.ignoreShowLimits to true. This pref doesn't exist by default.
3. Restart Firefox. about:newtab should be the selected tab when you restart, so don't check the "restore previous session" option.
4. You should see the onboarding search tip appear in the urlbar panel. See the end of the video in comment 0 for what it looks like ("Type less, find more"...).

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a one-line patch that just adds a default value to a preference get. Note that I said this code is covered by automated tests -- that's true, but this bug in particular does not have a test.
• String changes made/needed:

Comment 10

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9134570 [details]
Bug 1621944 - Make the top sites provider not assume the activity stream top sites pref exists.

thanks, approved for 75.0b7

Comment 11

[Oana Pop-Rus]

https://hg.mozilla.org/releases/mozilla-beta/rev/b341f56382a4

Comment 12

[Valentina Virlics [:vvalentina], Ecosystem QA]

I have verified that the issue is no longer reproducible on Firefox Nightly 76.0a1 (Build ID 20200322212426) and Firefox Beta 75.0b7 (Build ID 20200322132212) using Windows 10, Mac 10.14 and Linux Ubuntu 16.
The onboarding tip is automatically displayed, without any external action, at browser normal restart.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.