fix: Tasks scheduled through 'SchedulerBinding.instance.scheduleTask'… - #82781

Having this time out is not great. Can this run a a fake async zone where we can control time to ensure that the task will execute after the expected time has elapsed?

I have fixed the other issues, but when writing the test using the FakeAsync Zone, the test failed. After debugging, I found that the Timer. run in FakeAsync will be immediately added to the _timers set, and the tasks in the set will be processed synchronously while _firetimersWhile, so the EventLoop will schedule and add the tasks in a cycle. No other code will continue to run. But this is not the same as actually running on the device.

The following related code：

class FakeAsync {

  /// Creates a new timer controlled by `this` that fires [callback] after
  /// [duration] (or every [duration] if [periodic] is `true`).
  Timer _createTimer(Duration duration, Function callback, bool periodic) {
    var timer = FakeTimer._(duration, callback, periodic, this);
    _timers.add(timer);
    return timer;
  }
}

  /// Invoke the callback for each timer until [predicate] returns `false` for
  /// the next timer that would be fired.
  ///
  /// Microtasks are flushed before and after each timer is fired. Before each
  /// timer fires, [_elapsed] is updated to the appropriate duration.
  void _fireTimersWhile(bool Function(FakeTimer timer) predicate) {
    flushMicrotasks();
    for (;;) {
      if (_timers.isEmpty) break;

      var timer = minBy(_timers, (FakeTimer timer) => timer._nextCall)!;
      if (!predicate(timer)) break;

      _elapseTo(timer._nextCall);
      timer._fire();
      flushMicrotasks();
    }
  }

cc  when you have time~ thanks  @goderbauer @Hixie

I don't fully understand what you mean by this. It looks like the timeout I was concerned about is now gone from the test?

If it is not an EventLoop, timeout appears in the following line of code
await scheduler.scheduleTask(() => taskExecuted = true, Priority.idle);

Because in the original code, once 'schedulingStrategy' has returned the fase, it will not be executed regardless of whether there is a queue left in the _taskQueue, so this task will never be executed, causing the test to time out.

I tried to use FakeAsync to control the animation time, but the Timer.run in FakeAsync is a synchronous queue, which causes the system to execute Timer.run(_runTask) in a loop, thus timed out. So I can only write test cases in this way in the end. If you have better ideas, can you give me some suggestions

It looks like this can be simplified since there isn't really a case where handleEventLoopCallback returns true and _taskQueue.isNotEmpty is false, which means only _taskQueue.isNotEmpty decides here whether we queue up another event loop callback.

Actually, this doesn't look correct: If handleEventLoopCallback returns false because we're locked (see line 457) we are not allowed to call _ensureEventLoopCallback because of the assert(!locked) in that method.

Yeah, I also think it doesn't look correct with the previous change. After I thought it over and over again, I think the problem is inside the handleEventLoopCallback.
Because

According to the source code, The handleEventLoopCallback returns false in three cases：

• _taskQueue.isEmpty || locked
• schedulingStrategy() return true, and _taskQueue becomes empty after performing a task
• schedulingStrategy() return false , although _taskQueue is not empty

I think the third case is problematic, because going to the third logic says the _taskQueue is not empty and locked is false . This indicates that a task still needs to be executed, except that this time the schedulingStrategy is not satisfied. It should return true to continue the _ensureEventLoopCallback like this.

This seems incorrect to me, since it can cause tasks to fire even if the locked is true.

There was something wrong with my previous revision, you can check out my latest comment .
https://github.com/flutter/flutter/pull/82781/files#r671808835

I will revert the previous changes. And I think the problem is just that handleEventLoopCallback .

If locked is true, this method will return fasle at the first step and the event will not be executed

Call this debugResetEventLoop to make it clear that this shouldn't be called by an app and make it only do something when asserts are enabled.

Yeah I'll try to change it

done you can review it

This is odd though, because the scheduled Timer.run would still execute...

This flag _hasRequestedAnEventLoopCallback was corrected by the previous test error, as you can see from my previous debugging procedure

#82781 (comment)

I don't fully understand what you mean by this. It looks like the timeout I was concerned about is now gone from the test?

I think we can remove this right? you get a new scheduler for each test.

It'd be best not to expose new API if we can avoid it.

I have try to new scheduler , but it's TestSchedulerBinding class with BindingBase. So it can only initialize once. In the end, I had to do it this way

Do we need semantics binding here?

Yeah, I have used the animation in the test, it needs the SemanticsBinding .

I have try to new scheduler , but it's TestSchedulerBinding class with BindingBase. So it can only initialize once. In the end, I had to do it this way

Yeah, I have used the animation in the test, it needs the SemanticsBinding .

There was something wrong with my previous revision, you can check out my latest comment .
https://github.com/flutter/flutter/pull/82781/files#r671808835

I will revert the previous changes. And I think the problem is just that handleEventLoopCallback .

If locked is true, this method will return fasle at the first step and the event will not be executed