Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

feat: allow tasks to be canceled and rescheduled on different zone in… #629

Merged
merged 1 commit into from
Feb 14, 2017

Conversation

mhevery
Copy link
Contributor

@mhevery mhevery commented Feb 10, 2017

… a zone delegate

@mhevery
Copy link
Contributor Author

mhevery commented Feb 10, 2017

TODO

  • Needs a better commit message
  • Add test for task cancelation in ZoneDelegate, which shows how a Task can be rescheduled on a different zone.

@JiaLiPassion
Copy link
Collaborator

JiaLiPassion commented Feb 10, 2017

@mhevery, could you explain a little more about the motivation of this PR, I would like to learn more deeply about it.

in the commit message, you said that the feature is 'allow tasks to be canceled and rescheduled on different zone', I don't understand when we need to do that.

thank you very much.

@mhevery mhevery force-pushed the tasks branch 3 times, most recently from cff7b67 to 0db62e9 Compare February 14, 2017 05:01
@mhevery
Copy link
Contributor Author

mhevery commented Feb 14, 2017

@JiaLiPassion sorry for the delayed response.

The issue can best be described by this test

Imagine that you want to write a ZoneSpec which would redirect the scheduling of a task to a different zone. For example you want to make sure that all addEventListeners on scroll would run outside of some Zone. What you need to do is to intercept the scheduling of the task and than reschedule it on a different zone. However, that could not have been done, since there was no way to tell the ZoneDelegates that the current task, is not part of the current request. We now have Task. cancelScheduleRequest() which does just that.

In addition it was hard to keep track of in which state the task was, so I made the Task state explicit, which helps with the error messages.

@@ -39,7 +39,7 @@ module.exports = function (config) {

logLevel: config.LOG_INFO,

browsers: ['Firefox'],
browsers: ['ChromeCanary'],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

@@ -71,8 +71,8 @@ export function patchTimer(window: any, setName: string, cancelName: string, nam
patchMethod(window, cancelName, (delegate: Function) => function(self: any, args: any[]) {
const task: Task = typeof args[0] === 'number' ? tasksByHandleId[args[0]] : args[0];
if (task && typeof task.type === 'string') {
if (task.cancelFn && task.data.isPeriodic || task.runCount === 0) {
// Do not cancel already canceled functions
if (task.state !== 'notScheduled' && task.cancelFn ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

target: any, name: string,
patchFn: (delegate: Function, delegateName: string, name: string) => (self: any, args: any[]) =>
any): Function {
target: any, name: string, patchFn: (delegate: Function, delegateName: string, name: string) =>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

lib/zone.ts Outdated
@@ -229,6 +229,7 @@ interface Zone {
scheduleEventTask(
source: string, callback: Function, data: TaskData, customSchedule: (task: Task) => void,
customCancel: (task: Task) => void): EventTask;
scheduleTask<T extends Task>(task: T): T;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add docs

lib/zone.ts Outdated
@@ -516,6 +527,8 @@ interface Task {
* Number of times the task has been executed, or -1 if canceled.
*/
runCount: number;

cancelScheduleRequest(): void;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add docs

lib/zone.ts Outdated
@@ -622,13 +639,13 @@ const Zone: ZoneType = (function(global: any) {
}

public fork(zoneSpec: ZoneSpec): AmbientZone {
if (!zoneSpec) throw new Error('ZoneSpec required!');
if (!zoneSpec) throw Error('ZoneSpec required!');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

@mhevery mhevery force-pushed the tasks branch 4 times, most recently from 87b5350 to 60db4f2 Compare February 14, 2017 05:46
… a zone delegate

1) Adds states to the Task: `notScheduled`, `scheduling`, `scheduled`, `running`, `canceling`
2) Adds `cancelScheduleRequest` method to Task.
3) Allows canceling of task scheduling and subsequent re-scheduling from ZoneSpec. This
   allows a Zone to intercept the scheduling of a Task and redirecting it to another Task.
@mhevery mhevery merged commit 76c6ebf into angular:master Feb 14, 2017
@JiaLiPassion
Copy link
Collaborator

@mhevery , Thank you for explaining, I will read the source and learn how it works!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants