Bug 1284125 - Fix intermittent browser_inspector_delete-selected-node-02.js. r?jdescottes
The intermittency is caused by a race condition. When delete is clicked in the
context menu, markupview changes the selection and sends the removeNode request
to the server. The selection change triggers the usual inspector update process
in the client. At the same time, the server removes the node and queues the
mutation triggered by the removal. And here lies the issue.
If the inspector components finish updating BEFORE the removal mutation is
received from the server, the test continues before the breadcrumbs learn
about the deletion and the test fails. If the update is still pending when
the mutation is received, the breadcrumbs have time to update before the
test continues to make assertions about the breadcrumb contents.
The fix here is to make the test to ensure that the breadcrumbs have seen the
deletion before it continues. To enable that, the breadcrumbs need to tell
the world that it has been updated even if the breadcrumbs were just trimmed
and a non-element node was selected that does not trigger the full update
process (early return in the code).
As the inspector update process has been collecting cruft for years and tests
make a lot of assumptions about the emitted events, it's not safe to trigger a
new inspector update in this special case. Therefore, only the
breadcrumbs-updated event is emitted in this special case and the test waits
for that if the deleted node is still present in the breadcrumbs.
MozReview-Commit-ID: AjC6k6SzLCu
--- a/devtools/client/inspector/breadcrumbs.js
+++ b/devtools/client/inspector/breadcrumbs.js
@@ -810,34 +810,40 @@ HTMLBreadcrumbs.prototype = {
if (!this.selection.isConnected()) {
// remove all the crumbs
this.cutAfter(-1);
return;
}
// If this was an interesting deletion; then trim the breadcrumb trail
+ let trimmed = false;
if (reason === "markupmutation") {
for (let {type, removed} of mutations) {
if (type !== "childList") {
continue;
}
for (let node of removed) {
let removedIndex = this.indexOf(node);
if (removedIndex > -1) {
this.cutAfter(removedIndex - 1);
+ trimmed = true;
}
}
}
}
if (!this.selection.isElementNode()) {
// no selection
this.setCursor(-1);
+ if (trimmed) {
+ // Since something changed, notify the interested parties.
+ this.inspector.emit("breadcrumbs-updated", this.selection.nodeFront);
+ }
return;
}
let idx = this.indexOf(this.selection.nodeFront);
// Is the node already displayed in the breadcrumbs?
// (and there are no mutations that need re-display of the crumbs)
if (idx > -1 && !hasInterestingMutations) {
--- a/devtools/client/inspector/test/browser_inspector_delete-selected-node-02.js
+++ b/devtools/client/inspector/test/browser_inspector_delete-selected-node-02.js
@@ -88,16 +88,17 @@ add_task(function* () {
expectedCrumbs = ["html", "body", "div#deleteToMakeSingleTextNode"];
yield assertNodeSelectedAndCrumbsUpdated(expectedCrumbs,
Node.ELEMENT_NODE);
}
function* deleteNodeWithContextMenu(selector) {
yield selectNode(selector, inspector);
+ let nodeToBeDeleted = inspector.selection.nodeFront;
info("Getting the node container in the markup view.");
let container = yield getContainerForSelector(selector, inspector);
let allMenuItems = openContextMenuAndGetAllItems(inspector, {
target: container.tagLine,
});
let menuItem = allMenuItems.find(item => item.id === "node-menu-delete");
@@ -106,16 +107,26 @@ add_task(function* () {
is(menuItem.disabled, false, "delete menu item is enabled");
menuItem.click();
// close the open context menu
EventUtils.synthesizeKey("VK_ESCAPE", {});
info("Waiting for inspector to update.");
yield inspector.once("inspector-updated");
+
+ // Since the mutations are sent asynchronously from the server, the
+ // inspector-updated event triggered by the deletion might happen before
+ // the mutation is received and the element is removed from the
+ // breadcrumbs. See bug 1284125.
+ if (inspector.breadcrumbs.indexOf(nodeToBeDeleted) > -1) {
+ info("Crumbs haven't seen deletion. Waiting for breadcrumbs-updated.");
+ yield inspector.once("breadcrumbs-updated");
+ }
+
return menuItem;
}
function* assertNodeSelectedAndCrumbsUpdated(expectedCrumbs,
expectedNodeType) {
info("Performing checks");
let actualNodeType = inspector.selection.nodeFront.nodeType;
is(actualNodeType, expectedNodeType, "The node has the right type");