From c076f81115c45b4f2654a58090c198773017e7f8 Mon Sep 17 00:00:00 2001
From: Mike Conley <mconley@mozilla.com>
Date: Thu, 7 Nov 2019 21:41:04 +0000
Subject: [PATCH] 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            | 38 +++++++++++--------
 ...ser_default_bookmark_toolbar_visibility.js | 20 ++++++++++
 2 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/browser/components/BrowserGlue.jsm b/browser/components/BrowserGlue.jsm
index 8f1c471d81ac..6d128a520a6f 100644
--- a/browser/components/BrowserGlue.jsm
+++ b/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) {
diff --git a/browser/components/tests/browser/browser_default_bookmark_toolbar_visibility.js b/browser/components/tests/browser/browser_default_bookmark_toolbar_visibility.js
index 7a34f042e5bc..5c3bcf7171f9 100644
--- a/browser/components/tests/browser/browser_default_bookmark_toolbar_visibility.js
+++ b/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;