Bug 1553510 - Don't compute whether or not to show the Bookmarks Toolbar for new profiles until Places has finished initting. r=MattN

This causes BrowserGlue to wait until Places has notified that it's initted before checking
to see whether or not the Bookmarks Toolbar should be shown.

Also, if it turns out that the Bookmarks Toolbar is shown, we now use CustomizableUI to do
this, which means that the Bookmarks Toolbar will be made visible on all windows after the
check is run - not just new windows.

Differential Revision: https://phabricator.services.mozilla.com/D51701

--HG--
extra : moz-landing-system : lando

browser/components/BrowserGlue.jsm

@@ -20,6 +20,12 @@ ChromeUtils.defineModuleGetter(
   "resource://gre/modules/ActorManagerParent.jsm"
 );
 
+ChromeUtils.defineModuleGetter(
+  this,
+  "CustomizableUI",
+  "resource:///modules/CustomizableUI.jsm"
+);
+
 XPCOMUtils.defineLazyServiceGetter(
   this,
   "PushService",
@@ -751,6 +757,7 @@ BrowserGlue.prototype = {
   _saveSession: false,
   _migrationImportsDefaultBookmarks: false,
   _placesBrowserInitComplete: false,
+  _isNewProfile: undefined,
 
   _setPrefToSaveSession: function BG__setPrefToSaveSession(aForce) {
     if (!this._saveSession && !aForce) {
@@ -2677,6 +2684,17 @@ BrowserGlue.prototype = {
         }
         this._idleService.addIdleObserver(this, this._bookmarksBackupIdleTime);
       }
+
+      if (this._isNewProfile) {
+        try {
+          // New profiles may have existing bookmarks (imported from another browser or
+          // copied into the profile) and we want to show the bookmark toolbar for them
+          // in some cases.
+          this._maybeToggleBookmarkToolbarVisibility();
+        } catch (ex) {
+          Cu.reportError(ex);
+        }
+      }
     })()
       .catch(ex => {
         Cu.reportError(ex);
@@ -2813,11 +2831,9 @@ BrowserGlue.prototype = {
         toolbarIsCustomized ||
         getToolbarFolderCount() > NUM_TOOLBAR_BOOKMARKS_TO_UNHIDE
       ) {
-        xulStore.setValue(
-          BROWSER_DOCURL,
-          "PersonalToolbar",
-          "collapsed",
-          "false"
+        CustomizableUI.setToolbarVisibility(
+          CustomizableUI.AREA_BOOKMARKS,
+          true
         );
       }
     }
@@ -2844,19 +2860,11 @@ BrowserGlue.prototype = {
     let currentUIVersion;
     if (Services.prefs.prefHasUserValue("browser.migration.version")) {
       currentUIVersion = Services.prefs.getIntPref("browser.migration.version");
+      this._isNewProfile = false;
     } else {
       // This is a new profile, nothing to migrate.
       Services.prefs.setIntPref("browser.migration.version", UI_VERSION);
-
-      try {
-        // New profiles may have existing bookmarks (imported from another browser or
-        // copied into the profile) and we want to show the bookmark toolbar for them
-        // in some cases.
-        this._maybeToggleBookmarkToolbarVisibility();
-      } catch (ex) {
-        Cu.reportError(ex);
-      }
-      return;
+      this._isNewProfile = true;
     }
 
     if (currentUIVersion >= UI_VERSION) {

browser/components/tests/browser/browser_default_bookmark_toolbar_visibility.js

@@ -8,6 +8,26 @@
  * in NUM_TOOLBAR_BOOKMARKS_TO_UNHIDE may need to be adjusted there.
  */
 add_task(async function test_default_bookmark_toolbar_visibility() {
+  // The Bookmarks Toolbar visibility state should be set only after
+  // Places has notified that it's done initializing.
+  const browserGlue = Cc["@mozilla.org/browser/browserglue;1"].getService(
+    Ci.nsIObserver
+  );
+
+  let placesInitCompleteObserved = TestUtils.topicObserved(
+    "places-browser-init-complete"
+  );
+
+  // If places-browser-init-complete has already notified, this will cause it
+  // to notify again. Otherwise, we wait until the notify is done.
+  browserGlue.observe(
+    null,
+    "browser-glue-test",
+    "places-browser-init-complete"
+  );
+
+  await placesInitCompleteObserved;
+
   const BROWSER_DOCURL = AppConstants.BROWSER_CHROME_URL;
   let xulStore = Services.xulStore;