doc/test(task): add task lifecycle doc and testcases to explain task state transition.

[JiaLiPassion]

• add doc and state transition graph to explain the ZoneTask's lifecycle.
• add test cases to test the possible transitions described in the doc.

@mhevery, could you help me to review the document and test cases, and I have several questions here.

1. should the ZoneTask can be only cancelled in the zone which created the task?

const task = Zone.current.scheduleMacoTask();
// currently it is ok that task being canceled by other zone.
Zone.current.fork({name:'anotherZone'}).cancelTask(task);

2. when overriding task.zone in onScheduleTask callback of ZoneSpec, the hasTask callback will still be called in original zoneSpec, is that the correct behavior?

https://github.com/JiaLiPassion/zone.js/blob/5fe8a83df6e5197a5a1fe98f59481d83909168d3/test/common/task.spec.ts#L921

3. in the test case you write here, https://github.com/angular/zone.js/blob/master/test/common/zone.spec.ts#L226, the zone of task is override in onScheduleTask callback, and in current implementation, I can also override it in onInvokeTask callback or onCancelTask callback, if I do so, is that the correct behavior?

4. when error occurs in onHasTask, should we just try/catch it here https://github.com/angular/zone.js/blob/master/lib/zone.ts#L1046, because I think it should not affect the state transition of task.

5. if zoneSpec.onCancelTask throw error, currently the task's state will be 'canceling', should it be reset to 'scheduled' or 'running'?
   https://github.com/JiaLiPassion/zone.js/blob/5fe8a83df6e5197a5a1fe98f59481d83909168d3/test/common/task.spec.ts#L148

6. for non periodical macroTask(such as setTimeout), should we allow the task be canceled after running? now the logic in common/timer.ts add the check logic to make sure the non periodical macro task can be only canceled before running. Should we move those logic into zone.ts?
   https://github.com/angular/zone.js/blob/master/lib/common/timers.ts#L74

7. Should we check when application want to cancel a microTask?

8. should we do some parent-child relation check when reschedule task, for example we have zoneParent, zoneChild, zoneGrandChild, and in zoneChild, application want to reschedule task to zoneGrandChild, it will cause infinite loop. should we check it in zone.ts?

Thank you.

[mhevery]

1. should the ZoneTask can be only cancelled in the zone which created the task?

Yes, we should add code which enforces that is the case.

2. when overriding task.zone in onScheduleTask callback of ZoneSpec, the hasTask callback will still be called in original zoneSpec, is that the correct behavior?

No, the hasTask should only be run on the zone which the task was scheduled. If the scheduling process was canceled than it should not run hasTask.

Your test is is assigning task.zone = ... which is not allowed. We should make that read only.

3. in the test case you write here, https://github.com/angular/zone.js/blob/master/test/common/zone.spec.ts#L226, the zone of task is override in onScheduleTask callback, and in current implementation, I can also override it in onInvokeTask callback or onCancelTask callback, if I do so, is that the correct behavior?

That test is wrong and should be fixed and we should make task.zone readonly. See above

4. when error occurs in onHasTask, should we just try/catch it here https://github.com/angular/zone.js/blob/master/lib/zone.ts#L1046, because I think it should not affect the state transition of task.

Good question. I am not clear on that. An error here should not prevent other hasTasks from running, so I think we should try catch and send the error to the current zone onError handler.

5. if zoneSpec.onCancelTask throw error, currently the task's state will be 'canceling', should it be reset to 'scheduled' or 'running'?
   https://github.com/JiaLiPassion/zone.js/blob/5fe8a83df6e5197a5a1fe98f59481d83909168d3/test/common/task.spec.ts#L148

Good catch. Not sure what the correct behavior is. Since we don't know if we succeeded in canceling it. So the task is in unknown state. I think we should add a new state unknown???.

6. for non periodical macroTask(such as setTimeout), should we allow the task be canceled after running? now the logic in common/timer.ts add the check logic to make sure the non periodical macro task can be only canceled before running. Should we move those logic into zone.ts?
   https://github.com/angular/zone.js/blob/master/lib/common/timers.ts#L74

So you are suggesting that canceling a canceled task should be a noop. In general I don't like silent ignores. I think it is an error to cancel a canceled task. So I would keep it the way it is even though the place where it matters setTimeout works the other way.

7. Should we check when application want to cancel a microTask?

Tell me more, I am not sure what you are getting to.

8. should we do some parent-child relation check when reschedule task, for example we have zoneParent, zoneChild, zoneGrandChild, and in zoneChild, application want to reschedule task to zoneGrandChild, it will cause infinite loop. should we check it in zone.ts?

I ran into it. Yes, that would be a useful check.

[JiaLiPassion]

should the ZoneTask can be only cancelled in the zone which created the task?
Yes, we should add code which enforces that is the case.
Ok, I will add check of it.

when overriding task.zone in onScheduleTask callback of ZoneSpec, the hasTask callback will still be called in original zoneSpec, is that the correct behavior?
No, the hasTask should only be run on the zone which the task was scheduled. If the scheduling process was canceled than it should not run hasTask.

Your test is is assigning task.zone = ... which is not allowed. We should make that read only.

in the test case you write here, https://github.com/angular/zone.js/blob/master/test/common/zone.spec.ts#L226, the zone of task is override in onScheduleTask callback, and in current implementation, I can also override it in onInvokeTask callback or onCancelTask callback, if I do so, is that the correct behavior?
That test is wrong and should be fixed and we should make task.zone readonly. See above

so zone.js don't support override zone, a task can only change zone by reschedule when task in scheduling state, is that right?

when error occurs in onHasTask, should we just try/catch it here https://github.com/angular/zone.js/blob/master/lib/zone.ts#L1046, because I think it should not affect the state transition of task.
Good question. I am not clear on that. An error here should not prevent other hasTasks from running, so I think we should try catch and send the error to the current zone onError handler.

Ok, got it ,I will implement it.

if zoneSpec.onCancelTask throw error, currently the task's state will be 'canceling', should it be reset to 'scheduled' or 'running'?
https://github.com/JiaLiPassion/zone.js/blob/5fe8a83df6e5197a5a1fe98f59481d83909168d3/test/common/task.spec.ts#L148
Good catch. Not sure what the correct behavior is. Since we don't know if we succeeded in canceling it. So the task is in unknown state. I think we should add a new state unknown???.

In fact, I also want to confirm if zoneSpec.onCancelTask throw error, can the task be cancel again?
if the task is in unknown state, it should not be able to be cancelled again.

for non periodical macroTask(such as setTimeout), should we allow the task be canceled after running? now the logic in common/timer.ts add the check logic to make sure the non periodical macro task can be only canceled before running. Should we move those logic into zone.ts?
https://github.com/angular/zone.js/blob/master/lib/common/timers.ts#L74
So you are suggesting that canceling a canceled task should be a noop. In general I don't like silent ignores. I think it is an error to cancel a canceled task. So I would keep it the way it is even though the place where it matters setTimeout works the other way.

Ok, got it.

Should we check when application want to cancel a microTask?
Tell me more, I am not sure what you are getting to.

Now microTask doesn't have customCancel function, and for the user scenario, microTask like promise, process.nextTick, should not be able to be cancelled, so I want to confirm should we add check that if user want to zone.cancelTask(microTask).

should we do some parent-child relation check when reschedule task, for example we have zoneParent, zoneChild, zoneGrandChild, and in zoneChild, application want to reschedule task to zoneGrandChild, it will cause infinite loop. should we check it in zone.ts?
I ran into it. Yes, that would be a useful check.

Ok, got it , I will add the check

[mhevery]

so zone.js don't support override zone, a task can only change zone by reschedule when task in scheduling state, is that right?

correct

In fact, I also want to confirm if zoneSpec.onCancelTask throw error, can the task be cancel again?
if the task is in unknown state, it should not be able to be cancelled again.

correct

Now microTask doesn't have customCancel function, and for the user scenario, microTask like promise, process.nextTick, should not be able to be cancelled, so I want to confirm should we add check that if user want to zone.cancelTask(microTask).

I think we should throw if you try to cancel a microtask, since microtask has no cancel function.

[mhevery]

Let me know when you think this PR is ready to be merged in, and I will merge it.

[JiaLiPassion]

@mhevery , I have updated the code, don't updateTaskCount when onCancelTask throw error, please review. the other comments are all done!