Skip to content

Commit

Permalink
compute pressure: Fix while loop to end earlier
Browse files Browse the repository at this point in the history
On some testing bot, the while loop in testing seems
to take most of the time and the callback of the observer
is not invoked on time, which leads to timeout.

A step_timeout is also added to give a time to the event loop
to invoke the callback from the observer.

Bug: 1501324
Change-Id: I33e2253067e537e036b484e7e3fb62fe567346b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5126032
Reviewed-by: Reilly Grant <[email protected]>
Reviewed-by: Raphael Kubo Da Costa <[email protected]>
Commit-Queue: Arnaud Mandy <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1250091}
  • Loading branch information
arskama authored and chromium-wpt-export-bot committed Jan 22, 2024
1 parent ce63114 commit 8987e79
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ pressure_test(async (t, mockPressureService) => {
while (observerChanges.length < minChangesThreshold) {
mockPressureService.setPressureUpdate(
'cpu', readings[i++ % readings.length]);
// Allow tasks to run (avoid a micro-task loop).
await new Promise((resolve) => t.step_timeout(resolve, 0));
await t.step_wait(
() => mockPressureService.updatesDelivered() >= i,
`At least ${i} readings have been delivered`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,12 @@ pressure_test(async (t, mockPressureService) => {
await new Promise(async resolve => {
const observerChanges = [];
const observer = new PressureObserver(changes => {
if (observerChanges.length >= (minChangesThreshold - 1) && !gotPenalty) {
// Add an assert to the maximum threshold possible.
t.step(() => {
assert_less_than_equal(observerChanges.length, maxChangesThreshold,
"Sample count reaching maxChangesThreshold.");
});

if (observerChanges.length >= (minChangesThreshold - 1)) {
const lastSample = observerChanges.at(-1);
if ((changes[0].time - lastSample[0].time) >= minPenaltyTimeInMs) {
// The update delivery might still be working even if
// maxChangesThreshold have been reached and before disconnect() is
// processed. This will corrupt the result for the above t.step().
// processed.
// Therefore we are adding a flag to dismiss any updates after the
// penalty is detected, which is the condition for the test to pass.
gotPenalty = true;
Expand All @@ -46,12 +40,17 @@ pressure_test(async (t, mockPressureService) => {
// pressureChanges.length, as system load and browser optimizations can
// cause the actual timer used by mockPressureService to deliver readings
// to be a bit slower or faster than requested.
while (true) {
while (observerChanges.length <= maxChangesThreshold || !gotPenalty) {
mockPressureService.setPressureUpdate(
'cpu', readings[i++ % readings.length]);
// Allow tasks to run (avoid a micro-task loop).
await new Promise((resolve) => t.step_timeout(resolve, 0));
await t.step_wait(
() => mockPressureService.updatesDelivered() >= i,
`At least ${i} readings have been delivered`);
}

assert_true(gotPenalty, 'Penalty not triggered');

});
}, 'Rate obfuscation mitigation should have been triggered, when changes is higher than minimum changes before penalty');

0 comments on commit 8987e79

Please sign in to comment.