Skip to content

Commit

Permalink
Fix flaky test memory leak (web-platform-tests#34723)
Browse files Browse the repository at this point in the history
event-timing-test-utils.js tap() has some unknown flaky memory leak issue related to test_driver.Actions(), while test_driver also has a built in function for click which doesn't cause any memory leak.
This commit replace tap with click to fix the memory leak and re-enable
the disabled test.
Test coverage for tap will be added later once Bug 1338420 is fixed.
Also added helper function addListenersAndClick() to reduce repetitive
patterns between tests.

Bug: 1335002
Change-Id: Ic2670288e596ff5eac8f1e742e395717bdabde5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3747288
Reviewed-by: Lan Wei <[email protected]>
Commit-Queue: Aoyuan Zuo <[email protected]>
Reviewed-by: Michal Mocny <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1021762}

Co-authored-by: Aoyuan Zuo <[email protected]>
  • Loading branch information
2 people authored and pull[bot] committed Sep 7, 2023
1 parent 8815898 commit 1413556
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
<!DOCTYPE html>
<html>
<meta charset=utf-8 />
<meta name="timeout" content="long">
<title>Event Timing: first-input-interactionId-tap.</title>
<button id='test'>Tap</button>
<title>First Input: interactionId-click.</title>
<button id='testButtonId'>Click me</button>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script src=/resources/testdriver.js></script>
<script src=/resources/testdriver-actions.js></script>
<script src=/resources/testdriver-vendor.js></script>
<script src=resources/event-timing-test-utils.js></script>

Expand Down Expand Up @@ -49,8 +47,8 @@
}
})).observe({ entryTypes: ["event", "first-input"] });

addListenersAndTap(document.getElementById('test'), ['pointerdown', 'pointerup']);
}, "Event Timing: The interactionId of first input should match the event timing pointerdown entry when tap.");
addListenersAndClick(document.getElementById('testButtonId'));
}, "The interactionId of the first input entry should match the same pointerdown entry of event timing when click.");
</script>

</html>
15 changes: 2 additions & 13 deletions event-timing/interactionid-click.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,7 @@
<script>
let observedEntries = [];
let map = new Map();
async function clickOnElement(id) {
const element = document.getElementById(id);
const clickHandler = () => {
mainThreadBusy(200);
};
element.addEventListener("mousedown", clickHandler);
element.addEventListener("mouseup", clickHandler);
element.addEventListener("pointerdown", clickHandler);
element.addEventListener("pointerup", clickHandler);
element.addEventListener("click", clickHandler);
await test_driver.click(element);
}

function eventsForCheck(entry) {
if (entry.name === 'pointerdown' || entry.name === 'pointerup' || entry.name === 'click'
|| entry.name === 'mousedown' || entry.name === 'mouseup') {
Expand All @@ -44,7 +33,7 @@
assert_equals(map.get('pointerdown'), map.get('click'), 'Pointerdown and click should have the same interactionId');
t.done();
})).observe({type: 'event'});
clickOnElement('button');
addListenersAndClick(document.getElementById('button'));
}, "Event Timing: compare event timing interactionId.");
</script>
</html>
7 changes: 7 additions & 0 deletions event-timing/resources/event-timing-test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,13 @@ async function addListenersAndPress(element, key, events) {
pressKey(element, key);
}

// The testdriver.js, testdriver-vendor.js need to be included to use this
// function.
async function addListenersAndClick(element) {
addListeners(element, ['mousedown', 'mouseup', 'pointerdown', 'pointerup', 'click']);
await test_driver.click(element);
}

function filterAndAddToMap(events, map) {
return function (entry) {
if (events.includes(entry.name)) {
Expand Down

0 comments on commit 1413556

Please sign in to comment.