Skip to content

Commit

Permalink
Remove no-op i.remove() call from test.
Browse files Browse the repository at this point in the history
The .remove() call should have no impact on the test results, but it
makes our leak detector think we may have a memory leak. Removing this
seems to upset the delicate timing using nested renderAnimationFrames
in nested.py, so I'm replacing this with an explicit timeout.

Fixed: 1359561
Change-Id: Iff00340632913ce58c832aa519b768e0adecf7a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4927189
Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1208761}
  • Loading branch information
otherdaniel authored and chromium-wpt-export-bot committed Oct 12, 2023
1 parent 013e699 commit 7c2914d
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 16 deletions.
1 change: 0 additions & 1 deletion x-frame-options/support/helper.sub.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ function xfo_test({ url, check, message }) {
}

document.body.append(i);
t.add_cleanup(() => i.remove());
}, message);
}

Expand Down
29 changes: 14 additions & 15 deletions x-frame-options/support/nested.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,20 @@ def main(request, response):
var i = document.createElement("iframe");
i.src = "%s/x-frame-options/support/xfo.py?value=%s";
i.onload = _ => {
// Why two rAFs? Because that seems to be enough to stop the
// load event from racing with the onmessage event.
requestAnimationFrame(_ => {
requestAnimationFrame(_ => {
// The race condition problem we have is it is possible
// that the sub iframe is loaded before the postMessage is
// dispatched, as a result, the "Failed" message is sent
// out. So the way we fixed is we simply let the timeout
// to happen if we expect the "Loaded" postMessage to be
// sent
if (!gotMessage && %s != true) {
window.parent.postMessage("Failed", "*");
}
});
});
// Why 100ms timeout? Because that seems to be enough to stop the
// load event from racing with the onmessage event, and it's at least
// as long as the two renderAnimationFrame calls that used to be here.
setTimeout(_ => {
// The race condition problem we have is it is possible
// that the sub iframe is loaded before the postMessage is
// dispatched, as a result, the "Failed" message is sent
// out. So the way we fixed is we simply let the timeout
// to happen if we expect the "Loaded" postMessage to be
// sent
if (!gotMessage && %s != true) {
window.parent.postMessage("Failed", "*");
}
}, 100);
};
document.body.appendChild(i);
</script>
Expand Down

0 comments on commit 7c2914d

Please sign in to comment.