I updated to 2017-06-30 today which is the first Nightly build that contains bug 1367696, it turned out the logic didn't hold for me, and I still sees new user tours. Here is the about:config value I saw: browser.onboarding.tourset-version;1 (default) browser.onboarding.tour-type;new browser.onboarding.seen-tourset-version;0 browser.onboarding.notification.lastPrompted;onboarding-tour-addons browser.onboarding.notification.finished;false (default) browser.onboarding.hidden;false (default) browser.onboarding.enabled;true (default) Fred, could you check this?

It works after a restart. Looks like we're checking the prefs before the migration code is running in final-ui-startup.

(In reply to Dave Townsend [:mossop] from comment #1) > It works after a restart. Looks like we're checking the prefs before the > migration code is running in final-ui-startup. Yeah I figured that's the case when I see the about:config values... can we make startup() run after migration instead?

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #2) > (In reply to Dave Townsend [:mossop] from comment #1) > > It works after a restart. Looks like we're checking the prefs before the > > migration code is running in final-ui-startup. > > Yeah I figured that's the case when I see the about:config values... can we > make startup() run after migration instead? Not startup itself that always runs sooner. We'll have to add an observer notification for final-ui-startup then defer to let the migration code to run then do the checking.

Please manually test your path by migrate a profile from before bug 1367696 to after your WIP. You can use `mozdownload -t daily --date` to find the correct Nightly version to test on.

I found ngBrowserGlue will send a `browser-ui-startup-complete` notification when all browser-ui related work (include _migrateUI) is done. For our case it's good for the onboarding script to be started as late as possible. so I add an observer to listen on `browser-ui-startup-complete` and run all onboarding scripts after received the notification.

Comment on attachment 8883813 [details] Bug 1377470 - run onboarding scripts after browser UI is ready; https://reviewboard.mozilla.org/r/154770/#review160328 ::: browser/extensions/onboarding/bootstrap.js:66 (Diff revision 1) > > function uninstall(aData, aReason) {} > > function startup(aData, reason) { > + // wait until all browser-ui preparation work (including _migrateUI) is done > + Services.obs.addObserver(listener, "browser-ui-startup-complete"); Given that this is a system add-on it could get updated and restarted while the application is already running in which case this notification will never happen. You need to verify the reason for the startup and act accordingly. ::: browser/extensions/onboarding/bootstrap.js:72 (Diff revision 1) > +} > + > +function shutdown(aData, reason) {} > + > +function listener() { > + Services.obs.removeObserver(this, "browser-ui-startup-complete"); this is null here, you need to remove listener as the observer.

Thanks for review! After double check it seems Activity Stream does handle the similar issue, so borrow the bootstrap check from them.

Comment on attachment 8883813 [details] Bug 1377470 - run onboarding scripts after browser UI is ready; https://reviewboard.mozilla.org/r/154770/#review160882 ::: browser/extensions/onboarding/bootstrap.js:18 (Diff revision 2) > XPCOMUtils.defineLazyModuleGetter(this, "Services", > "resource://gre/modules/Services.jsm"); > +XPCOMUtils.defineLazyModuleGetter(this, "setTimeout", > + "resource://gre/modules/Timer.jsm"); > > +const BROWSER_READY_NOTIFICATION = "sessionstore-windows-restored"; Why this notification rather than final-ui-startup? I think with this notification you might miss restored tabs ::: browser/extensions/onboarding/bootstrap.js:98 (Diff revision 2) > > function uninstall(aData, aReason) {} > > function startup(aData, reason) { > - OnboardingTourType.check(); > - Services.mm.loadFrameScript("resource://onboarding/onboarding.js", true); > + // Only start Onboarding up when the browser UI is ready > + if (Cc["@mozilla.org/toolkit/app-startup;1"].getService(Ci.nsIAppStartup).startingUp) { Just compare reason to APP_STARTUP

Issue addressed and update README for 1. how to show the onboarding tour, 2. update the overall architecture Please kindly review it again

Comment on attachment 8883813 [details] Bug 1377470 - run onboarding scripts after browser UI is ready; https://reviewboard.mozilla.org/r/154770/#review161360

Test works well by update from release version to patched nightly (after version update, the system addon startup aReadon is `ADDON_INSTALL` though) But the test from nightly(2017-06-28) to patched nightly does not work since ALL system addons does not load at first start (can't see pocket, screenshots). It does not affect onboarding behavior and only happens while manual select the same profile, but I'll file another bug for that issue.

Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ed0ec2f26de4 run onboarding scripts after browser UI is ready;r=mossop

I have verified this issue is fixed on today's nightly.