Bug 1395526 - Avoid potential race with places initialisation when clipboard tests are starting to avoid intermittents. r?mak draft
authorMark Banner <standard8@mozilla.com>
Thu, 21 Sep 2017 21:21:41 +0100
changeset 673396 ae35cf5be664480ee554d18f3a7b8c5fef4708e3
parent 673329 97efdde466f18cf580fda9673cf4c38ee21fc7b7
child 734083 6215d216c5bd00ee4327185bdd29decbdf090297
push id82556
push userbmo:standard8@mozilla.com
push dateMon, 02 Oct 2017 12:39:56 +0000
reviewersmak
bugs1395526
milestone58.0a1
Bug 1395526 - Avoid potential race with places initialisation when clipboard tests are starting to avoid intermittents. r?mak MozReview-Commit-ID: 4Hr4zdZNHRp
browser/components/nsBrowserGlue.js
browser/components/places/tests/browser/browser.ini
browser/components/places/tests/browser/browser_copy_query_without_tree.js
browser/components/places/tests/browser/head.js
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -259,16 +259,17 @@ function BrowserGlue() {
  * OS X has the concept of zero-window sessions and therefore ignores the
  * browser-lastwindow-close-* topics.
  */
 const OBSERVE_LASTWINDOW_CLOSE_TOPICS = AppConstants.platform != "macosx";
 
 BrowserGlue.prototype = {
   _saveSession: false,
   _migrationImportsDefaultBookmarks: false,
+  _placesBrowserInitComplete: false,
 
   _setPrefToSaveSession: function BG__setPrefToSaveSession(aForce) {
     if (!this._saveSession && !aForce)
       return;
 
     Services.prefs.setBoolPref("browser.sessionstore.resume_session_once", true);
 
     // This method can be called via [NSApplication terminate:] on Mac, which
@@ -402,16 +403,20 @@ BrowserGlue.prototype = {
         } else if (data == "mock-fxaccounts") {
           Object.defineProperty(this, "fxAccounts", {
             value: subject.wrappedJSObject
           });
         } else if (data == "mock-alerts-service") {
           Object.defineProperty(this, "AlertsService", {
             value: subject.wrappedJSObject
           });
+        } else if (data == "places-browser-init-complete") {
+          if (this._placesBrowserInitComplete) {
+            Services.obs.notifyObservers(null, "places-browser-init-complete");
+          }
         }
         break;
       case "initial-migration-will-import-default-bookmarks":
         this._migrationImportsDefaultBookmarks = true;
         break;
       case "initial-migration-did-import-default-bookmarks":
         this._initPlaces(true);
         break;
@@ -1450,16 +1455,17 @@ BrowserGlue.prototype = {
     let dbStatus = PlacesUtils.history.databaseStatus;
 
     // Show a notification with a "more info" link for a locked places.sqlite.
     if (dbStatus == PlacesUtils.history.DATABASE_STATUS_LOCKED) {
       // Note: initPlaces should always happen when the first window is ready,
       // in any case, better safe than sorry.
       this._firstWindowReady.then(() => {
         this._showPlacesLockedNotificationBox();
+        this._placesBrowserInitComplete = true;
         Services.obs.notifyObservers(null, "places-browser-init-complete");
       });
       return;
     }
 
     let importBookmarks = !aInitialMigrationPerformed &&
                           (dbStatus == PlacesUtils.history.DATABASE_STATUS_CREATE ||
                            dbStatus == PlacesUtils.history.DATABASE_STATUS_CORRUPT);
@@ -1618,16 +1624,17 @@ BrowserGlue.prototype = {
         this._idleService.addIdleObserver(this, this._bookmarksBackupIdleTime);
       }
 
     })().catch(ex => {
       Cu.reportError(ex);
     }).then(() => {
       // NB: deliberately after the catch so that we always do this, even if
       // we threw halfway through initializing in the Task above.
+      this._placesBrowserInitComplete = true;
       Services.obs.notifyObservers(null, "places-browser-init-complete");
     });
   },
 
   /**
    * If a backup for today doesn't exist, this creates one.
    */
   _backupBookmarks: function BG__backupBookmarks() {
--- a/browser/components/places/tests/browser/browser.ini
+++ b/browser/components/places/tests/browser/browser.ini
@@ -22,22 +22,22 @@ support-files =
 [browser_bookmarkProperties_addLivemark.js]
 [browser_bookmarkProperties_bookmarkAllTabs.js]
 [browser_bookmarkProperties_cancel.js]
 [browser_bookmarkProperties_editTagContainer.js]
 [browser_bookmarkProperties_readOnlyRoot.js]
 [browser_bookmarksProperties.js]
 [browser_check_correct_controllers.js]
 [browser_click_bookmarks_on_toolbar.js]
-[browser_cutting_bookmarks.js]
-subsuite = clipboard
 [browser_controller_onDrop.js]
 [browser_copy_folder_tree.js]
 [browser_copy_query_without_tree.js]
 subsuite = clipboard
+[browser_cutting_bookmarks.js]
+subsuite = clipboard
 [browser_drag_bookmarks_on_toolbar.js]
 [browser_forgetthissite_single.js]
 [browser_history_sidebar_search.js]
 [browser_library_batch_delete.js]
 [browser_library_commands.js]
 [browser_library_delete_bookmarks_in_tags.js]
 [browser_library_delete_tags.js]
 [browser_library_downloads.js]
--- a/browser/components/places/tests/browser/browser_copy_query_without_tree.js
+++ b/browser/components/places/tests/browser/browser_copy_query_without_tree.js
@@ -3,29 +3,32 @@
  */
 
 /* test that copying a non movable query or folder shortcut makes a new query with the same url, not a deep copy */
 
 const SHORTCUT_URL = "place:folder=2";
 const QUERY_URL = "place:sort=8&maxResults=10";
 
 add_task(async function copy_toolbar_shortcut() {
+  await promisePlacesInitComplete();
+
   let library = await promiseLibrary();
 
   registerCleanupFunction(function() {
     library.close();
     PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId);
   });
 
   library.PlacesOrganizer.selectLeftPaneQuery("BookmarksToolbar");
 
   await promiseClipboard(function() { library.PlacesOrganizer._places.controller.copy(); },
                          PlacesUtils.TYPE_X_MOZ_PLACE);
 
   library.PlacesOrganizer.selectLeftPaneQuery("UnfiledBookmarks");
+
   await library.ContentTree.view.controller.paste();
 
   let toolbarCopyNode = library.ContentTree.view.view.nodeForTreeIndex(0);
   is(toolbarCopyNode.type,
      Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT,
      "copy is still a folder shortcut");
 
   PlacesUtils.bookmarks.removeItem(toolbarCopyNode.itemId);
--- a/browser/components/places/tests/browser/head.js
+++ b/browser/components/places/tests/browser/head.js
@@ -70,18 +70,18 @@ function promiseLibraryClosed(organizer)
  * @see waitForClipboard
  *
  * @param aPopulateClipboardFn
  *        Function to populate the clipboard.
  * @param aFlavor
  *        Data flavor to expect.
  */
 function promiseClipboard(aPopulateClipboardFn, aFlavor) {
-  return new Promise(resolve => {
-    waitForClipboard(data => !!data, aPopulateClipboardFn, resolve, aFlavor);
+  return new Promise((resolve, reject) => {
+    waitForClipboard(data => !!data, aPopulateClipboardFn, resolve, reject, aFlavor);
   });
 }
 
 /**
  * Waits for all pending async statements on the default connection, before
  * proceeding with aCallback.
  *
  * @param aCallback
@@ -466,8 +466,20 @@ var withSidebarTree = async function(typ
   // Need to executeSoon since the tree is initialized on sidebar load.
   info("withSidebarTree: executing the task");
   try {
     await taskFn(tree);
   } finally {
     SidebarUI.hide();
   }
 };
+
+function promisePlacesInitComplete() {
+  const gBrowserGlue = Cc["@mozilla.org/browser/browserglue;1"]
+                         .getService(Ci.nsIObserver);
+
+  let placesInitCompleteObserved = TestUtils.topicObserved("places-browser-init-complete")
+
+  gBrowserGlue.observe(null, "browser-glue-test",
+    "places-browser-init-complete");
+
+  return placesInitCompleteObserved;
+}