Merge mozilla-beta to release. a=merge
authorRyan VanderMeulen <ryanvm@gmail.com>
Wed, 18 Jan 2017 13:19:06 -0500
changeset 465642 aa8b10dca83df22981e98ce094adc7bcfb54101a
parent 465641 1f694d0ba1e8cbfd7d87c27946c95ed326dd85bf (current diff)
parent 465433 dd4b8065a9a9ba168c84674bcd4071b0b4a8b7b0 (diff)
child 465643 ea82b5e20cbbd103f8fa65f0df0386ee4135cc47
push id42661
push userfelipc@gmail.com
push dateTue, 24 Jan 2017 16:14:20 +0000
reviewersmerge
milestone51.0
Merge mozilla-beta to release. a=merge
--- a/browser/components/migration/AutoMigrate.jsm
+++ b/browser/components/migration/AutoMigrate.jsm
@@ -171,20 +171,24 @@ const AutoMigrate = {
       return suggestedProfile;
     }
     if (profiles && profiles.length > 1) {
       throw new Error("Don't know how to pick a profile when more than 1 profile is present.");
     }
     return profiles ? profiles[0] : null;
   },
 
+  _pendingUndoTasks: false,
   canUndo: Task.async(function* () {
     if (this._savingPromise) {
       yield this._savingPromise;
     }
+    if (this._pendingUndoTasks) {
+      return false;
+    }
     let fileExists = false;
     try {
       fileExists = yield OS.File.exists(kUndoStateFullPath);
     } catch (ex) {
       Cu.reportError(ex);
     }
     return fileExists;
   }),
@@ -192,16 +196,18 @@ const AutoMigrate = {
   undo: Task.async(function* () {
     let histogram = Services.telemetry.getHistogramById("FX_STARTUP_MIGRATION_AUTOMATED_IMPORT_UNDO");
     histogram.add(0);
     if (!(yield this.canUndo())) {
       histogram.add(5);
       throw new Error("Can't undo!");
     }
 
+    this._pendingUndoTasks = true;
+    this._removeNotificationBars();
     histogram.add(10);
 
     let readPromise = OS.File.read(kUndoStateFullPath, {
       encoding: "utf-8",
       compression: "lz4",
     });
     let stateData = this._dejsonifyUndoState(yield readPromise);
     yield this._removeUnchangedBookmarks(stateData.get("bookmarks"));
@@ -213,41 +219,46 @@ const AutoMigrate = {
     yield this._removeUnchangedLogins(stateData.get("logins"));
     histogram.add(25);
 
     // This is async, but no need to wait for it.
     NewTabUtils.links.populateCache(() => {
       NewTabUtils.allPages.update();
     }, true);
 
-    this.removeUndoOption(this.UNDO_REMOVED_REASON_UNDO_USED);
+    this._purgeUndoState(this.UNDO_REMOVED_REASON_UNDO_USED);
     histogram.add(30);
   }),
 
-  removeUndoOption(reason) {
-    // We don't wait for the off-main-thread removal to complete. OS.File will
-    // ensure it happens before shutdown.
-    OS.File.remove(kUndoStateFullPath, {ignoreAbsent: true});
-
-    let migrationBrowser = Preferences.get(kAutoMigrateBrowserPref, "unknown");
-    Services.prefs.clearUserPref(kAutoMigrateBrowserPref);
-
+  _removeNotificationBars() {
     let browserWindows = Services.wm.getEnumerator("navigator:browser");
     while (browserWindows.hasMoreElements()) {
       let win = browserWindows.getNext();
       if (!win.closed) {
         for (let browser of win.gBrowser.browsers) {
           let nb = win.gBrowser.getNotificationBox(browser);
           let notification = nb.getNotificationWithValue(kNotificationId);
           if (notification) {
             nb.removeNotification(notification);
           }
         }
       }
     }
+  },
+
+  _purgeUndoState(reason) {
+    // We don't wait for the off-main-thread removal to complete. OS.File will
+    // ensure it happens before shutdown.
+    OS.File.remove(kUndoStateFullPath, {ignoreAbsent: true}).then(() => {
+      this._pendingUndoTasks = false;
+    });
+
+    let migrationBrowser = Preferences.get(kAutoMigrateBrowserPref, "unknown");
+    Services.prefs.clearUserPref(kAutoMigrateBrowserPref);
+
     let histogram =
       Services.telemetry.getKeyedHistogramById("FX_STARTUP_MIGRATION_UNDO_REASON");
     histogram.add(migrationBrowser, reason);
   },
 
   getBrowserUsedForMigration() {
     let browserId = Services.prefs.getCharPref(kAutoMigrateBrowserPref);
     if (browserId) {
@@ -273,17 +284,18 @@ const AutoMigrate = {
     if (!notificationBox || notificationBox.getNotificationWithValue(kNotificationId)) {
       return;
     }
 
     // At this stage we're committed to show the prompt - unless we shouldn't,
     // in which case we remove the undo prefs (which will cause canUndo() to
     // return false from now on.):
     if (!this.shouldStillShowUndoPrompt()) {
-      this.removeUndoOption(this.UNDO_REMOVED_REASON_OFFER_EXPIRED);
+      this._purgeUndoState(this.UNDO_REMOVED_REASON_OFFER_EXPIRED);
+      this._removeNotificationBars();
       return;
     }
 
     let browserName = this.getBrowserUsedForMigration();
     let message;
     if (browserName) {
       message = MigrationUtils.getLocalizedString("automigration.undo.message",
                                                   [browserName]);
@@ -291,17 +303,18 @@ const AutoMigrate = {
       message = MigrationUtils.getLocalizedString("automigration.undo.unknownBrowserMessage");
     }
 
     let buttons = [
       {
         label: MigrationUtils.getLocalizedString("automigration.undo.keep.label"),
         accessKey: MigrationUtils.getLocalizedString("automigration.undo.keep.accesskey"),
         callback: () => {
-          this.removeUndoOption(this.UNDO_REMOVED_REASON_OFFER_REJECTED);
+          this._purgeUndoState(this.UNDO_REMOVED_REASON_OFFER_REJECTED);
+          this._removeNotificationBars();
         },
       },
       {
         label: MigrationUtils.getLocalizedString("automigration.undo.dontkeep.label"),
         accessKey: MigrationUtils.getLocalizedString("automigration.undo.dontkeep.accesskey"),
         callback: () => {
           this.undo();
         },
--- a/browser/components/migration/tests/browser/browser.ini
+++ b/browser/components/migration/tests/browser/browser.ini
@@ -1,1 +1,2 @@
 [browser_undo_notification.js]
+[browser_undo_notification_multiple_dismissal.js]
--- a/browser/components/migration/tests/browser/browser_undo_notification.js
+++ b/browser/components/migration/tests/browser/browser_undo_notification.js
@@ -1,19 +1,17 @@
 "use strict";
 
 let scope = {};
 Cu.import("resource:///modules/AutoMigrate.jsm", scope);
 let oldCanUndo = scope.AutoMigrate.canUndo;
 let oldUndo = scope.AutoMigrate.undo;
 registerCleanupFunction(function() {
-  Cu.reportError("Cleaning up");
   scope.AutoMigrate.canUndo = oldCanUndo;
   scope.AutoMigrate.undo = oldUndo;
-  Cu.reportError("Cleaned up");
 });
 
 const kExpectedNotificationId = "automigration-undo";
 
 add_task(function* autoMigrationUndoNotificationShows() {
   let getNotification = browser =>
     gBrowser.getNotificationBox(browser).getNotificationWithValue(kExpectedNotificationId);
 
new file mode 100644
--- /dev/null
+++ b/browser/components/migration/tests/browser/browser_undo_notification_multiple_dismissal.js
@@ -0,0 +1,122 @@
+"use strict";
+
+
+const kExpectedNotificationId = "automigration-undo";
+
+/**
+ * Pretend we can undo something, trigger a notification, pick the undo option,
+ * and verify that the notifications are all dismissed immediately.
+ */
+add_task(function* checkNotificationsDismissed() {
+  yield SpecialPowers.pushPrefEnv({set: [
+    ["browser.migrate.automigrate.enabled", true],
+    ["browser.migrate.automigrate.ui.enabled", true],
+  ]});
+  let getNotification = browser =>
+    gBrowser.getNotificationBox(browser).getNotificationWithValue(kExpectedNotificationId);
+
+  Services.prefs.setCharPref("browser.migrate.automigrate.browser", "someunknownbrowser");
+
+  let {guid, lastModified} = yield PlacesUtils.bookmarks.insert(
+    {title: "Some imported bookmark", parentGuid: PlacesUtils.bookmarks.toolbarGuid, url: "http://www.example.com"}
+  );
+
+  let testUndoData = {
+    visits: [],
+    bookmarks: [{guid, lastModified: lastModified.getTime()}],
+    logins: [],
+  };
+  let path = OS.Path.join(OS.Constants.Path.profileDir, "initialMigrationMetadata.jsonlz4");
+  registerCleanupFunction(() => {
+    return OS.File.remove(path, {ignoreAbsent: true});
+  });
+  yield OS.File.writeAtomic(path, JSON.stringify(testUndoData), {
+    encoding: "utf-8",
+    compression: "lz4",
+    tmpPath: path + ".tmp",
+  });
+
+  let firstTab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "about:home", false);
+  if (!getNotification(firstTab.linkedBrowser)) {
+    info(`Notification not immediately present on first tab, waiting for it.`);
+    yield BrowserTestUtils.waitForNotificationBar(gBrowser, firstTab.linkedBrowser, kExpectedNotificationId);
+  }
+  let secondTab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "about:home", false);
+  if (!getNotification(secondTab.linkedBrowser)) {
+    info(`Notification not immediately present on second tab, waiting for it.`);
+    yield BrowserTestUtils.waitForNotificationBar(gBrowser, secondTab.linkedBrowser, kExpectedNotificationId);
+  }
+
+  // Create a listener for the removal in the first tab, and a listener for bookmarks removal,
+  // then click 'Don't keep' in the second tab, and verify that the notification is removed
+  // before we start removing bookmarks.
+  let haveRemovedBookmark = false;
+  let bmObserver;
+  let bookmarkRemovedPromise = new Promise(resolve => {
+    bmObserver = {
+      onItemRemoved(itemId, parentId, index, itemType, uri, removedGuid) {
+        if (guid == removedGuid) {
+          haveRemovedBookmark = true;
+          resolve();
+        }
+      },
+    };
+    PlacesUtils.bookmarks.addObserver(bmObserver, false);
+    registerCleanupFunction(() => PlacesUtils.bookmarks.removeObserver(bmObserver));
+  });
+
+  let firstTabNotificationRemovedPromise = new Promise(resolve => {
+    let notification = getNotification(firstTab.linkedBrowser);
+    // Save this reference because notification.parentNode will be null once it's removed.
+    let notificationBox = notification.parentNode;
+    let mut = new MutationObserver(mutations => {
+      // Yucky, but we have to detect either the removal via animation (with marginTop)
+      // or when the element is removed. We can't just detect the element being removed
+      // because this happens asynchronously (after the animation) and so it'd race
+      // with the rest of the undo happening.
+      for (let mutation of mutations) {
+        if (mutation.target == notification && mutation.attributeName == "style" &&
+            parseInt(notification.style.marginTop, 10) < 0) {
+          ok(!haveRemovedBookmark, "Should not have removed bookmark yet");
+          mut.disconnect();
+          resolve();
+          return;
+        }
+        if (mutation.target == notificationBox && mutation.removedNodes.length &&
+            mutation.removedNodes[0] == notification) {
+          ok(!haveRemovedBookmark, "Should not have removed bookmark yet");
+          mut.disconnect();
+          resolve();
+          return;
+        }
+      }
+    });
+    mut.observe(notification.parentNode, {childList: true});
+    mut.observe(notification, {attributes: true});
+  });
+
+  let prefResetPromise = new Promise(resolve => {
+    const kObservedPref = "browser.migrate.automigrate.browser";
+    let obs = () => {
+      Services.prefs.removeObserver(kObservedPref, obs);
+      ok(!Services.prefs.prefHasUserValue(kObservedPref),
+         "Pref should have been reset");
+      resolve();
+    };
+    Services.prefs.addObserver(kObservedPref, obs, false);
+  });
+
+  // Click "Don't keep" button:
+  let notificationToActivate = getNotification(secondTab.linkedBrowser);
+  notificationToActivate.querySelector("button:not(.notification-button-default)").click();
+  info("Waiting for notification to be removed in first (background) tab");
+  yield firstTabNotificationRemovedPromise;
+  info("Waiting for bookmark to be removed");
+  yield bookmarkRemovedPromise;
+  info("Waiting for prefs to be reset");
+  yield prefResetPromise;
+
+  info("Removing spare tabs");
+  yield BrowserTestUtils.removeTab(firstTab);
+  yield BrowserTestUtils.removeTab(secondTab);
+});
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -1238,21 +1238,27 @@ TabChild::DestroyWindow()
 void
 TabChild::ActorDestroy(ActorDestroyReason why)
 {
   mIPCOpen = false;
 
   DestroyWindow();
 
   if (mTabChildGlobal) {
-    // The messageManager relays messages via the TabChild which
-    // no longer exists.
-    static_cast<nsFrameMessageManager*>
-      (mTabChildGlobal->mMessageManager.get())->Disconnect();
-    mTabChildGlobal->mMessageManager = nullptr;
+    // We should have a message manager if the global is alive, but it
+    // seems sometimes we don't.  Assert in aurora/nightly, but don't
+    // crash in release builds.
+    MOZ_DIAGNOSTIC_ASSERT(mTabChildGlobal->mMessageManager);
+    if (mTabChildGlobal->mMessageManager) {
+      // The messageManager relays messages via the TabChild which
+      // no longer exists.
+      static_cast<nsFrameMessageManager*>
+        (mTabChildGlobal->mMessageManager.get())->Disconnect();
+      mTabChildGlobal->mMessageManager = nullptr;
+    }
   }
 
   CompositorBridgeChild* compositorChild = static_cast<CompositorBridgeChild*>(CompositorBridgeChild::Get());
   compositorChild->CancelNotifyAfterRemotePaint(this);
 
   if (GetTabId() != 0) {
     NestedTabChildMap().erase(GetTabId());
   }
@@ -2350,26 +2356,36 @@ TabChild::RecvLoadRemoteScript(const nsS
 }
 
 bool
 TabChild::RecvAsyncMessage(const nsString& aMessage,
                            InfallibleTArray<CpowEntry>&& aCpows,
                            const IPC::Principal& aPrincipal,
                            const ClonedMessageData& aData)
 {
-  if (mTabChildGlobal) {
-    nsCOMPtr<nsIXPConnectJSObjectHolder> kungFuDeathGrip(GetGlobal());
-    StructuredCloneData data;
-    UnpackClonedMessageDataForChild(aData, data);
-    RefPtr<nsFrameMessageManager> mm =
-      static_cast<nsFrameMessageManager*>(mTabChildGlobal->mMessageManager.get());
-    CrossProcessCpowHolder cpows(Manager(), aCpows);
-    mm->ReceiveMessage(static_cast<EventTarget*>(mTabChildGlobal), nullptr,
-                       aMessage, false, &data, &cpows, aPrincipal, nullptr);
+  if (!mTabChildGlobal) {
+    return true;
   }
+
+  // We should have a message manager if the global is alive, but it
+  // seems sometimes we don't.  Assert in aurora/nightly, but don't
+  // crash in release builds.
+  MOZ_DIAGNOSTIC_ASSERT(mTabChildGlobal->mMessageManager);
+  if (!mTabChildGlobal->mMessageManager) {
+    return true;
+  }
+
+  nsCOMPtr<nsIXPConnectJSObjectHolder> kungFuDeathGrip(GetGlobal());
+  StructuredCloneData data;
+  UnpackClonedMessageDataForChild(aData, data);
+  RefPtr<nsFrameMessageManager> mm =
+    static_cast<nsFrameMessageManager*>(mTabChildGlobal->mMessageManager.get());
+  CrossProcessCpowHolder cpows(Manager(), aCpows);
+  mm->ReceiveMessage(static_cast<EventTarget*>(mTabChildGlobal), nullptr,
+                     aMessage, false, &data, &cpows, aPrincipal, nullptr);
   return true;
 }
 
 bool
 TabChild::RecvAppOfflineStatus(const uint32_t& aId, const bool& aOffline)
 {
   // Instantiate the service to make sure gIOService is initialized
   nsCOMPtr<nsIIOService> ioService = mozilla::services::GetIOService();
--- a/gfx/2d/DrawTargetD2D1.cpp
+++ b/gfx/2d/DrawTargetD2D1.cpp
@@ -495,17 +495,17 @@ DrawTargetD2D1::Stroke(const Path *aPath
   FinalizeDrawing(aOptions.mCompositionOp, aPattern);
 }
 
 void
 DrawTargetD2D1::Fill(const Path *aPath,
                      const Pattern &aPattern,
                      const DrawOptions &aOptions)
 {
-  if (aPath->GetBackendType() != BackendType::DIRECT2D1_1) {
+  if (!aPath || aPath->GetBackendType() != BackendType::DIRECT2D1_1) {
     gfxDebug() << *this << ": Ignoring drawing call for incompatible path.";
     return;
   }
   const PathD2D *d2dPath = static_cast<const PathD2D*>(aPath);
 
   PrepareForDrawing(aOptions.mCompositionOp, aPattern);
 
   mDC->SetAntialiasMode(D2DAAMode(aOptions.mAntialiasMode));
--- a/gfx/2d/DrawTargetSkia.cpp
+++ b/gfx/2d/DrawTargetSkia.cpp
@@ -729,17 +729,17 @@ DrawTargetSkia::StrokeLine(const Point &
 }
 
 void
 DrawTargetSkia::Fill(const Path *aPath,
                     const Pattern &aPattern,
                     const DrawOptions &aOptions)
 {
   MarkChanged();
-  if (aPath->GetBackendType() != BackendType::SKIA) {
+  if (!aPath || aPath->GetBackendType() != BackendType::SKIA) {
     return;
   }
 
   const PathSkia *skiaPath = static_cast<const PathSkia*>(aPath);
 
   AutoPaintSetup paint(mCanvas.get(), aOptions, aPattern);
 
   if (!skiaPath->GetPath().isFinite()) {