fix(zone.js): patch child method that overrides an already patched method

[edusperoni]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• angular.io application / infrastructure changes
• Other... Please describe:

What is the current behavior?

When running patchMethod on a target that had it's parent already patched, the patch would not be applied.

Consider we have these 3 classes:

class Type {
  method(callback) {
    Zone.root.run(callback, null, ['type']);
    return 'Type';
  }
}
class OverrideType extends Type {
  method(callback) {
    Zone.root.run(callback, null, ['override']);
    return 'Override';
  }
}
class Dummy extends Type {
}
function patchType(t) {
  return patchMethod(t.prototype, 'method', (delegate) => {
    console.log('patched');
    return function(self, args) {
      args[0] = Zone.current.wrap(args[0]);
      return delegate.apply(self, args);
    };
  });
}
const cb = (arg) => {
  console.log(`${arg} ${Zone.current.name}`);
};
const typeInstance = new Type();
const overrideInstance= new OverrideType();
const dummyInstance= new Dummy();
const zone = Zone.current.fork({name: 'patch'});

As we can see, OverrideType overrides method, so we want to patch it as well as patching Type. I've added the helper function patchType.

patchType(Type); // outputs 'patched'. Returns the unpatched Type.prototype.method
patchType(Dummy); // no output as method is just inherited. Returns the unpatched Type.prototype.method
const nativeOverride = patchType(OverrideType); // no output as it checks for proto[__symbol__('method')], which is inherited, instead of checking for proto.hasOwnProperty(__symbol__('method')). Returns the unpatched Type.prototype.method instead of OverrideType.prototype.method
zone.run(() => {
  typeInstance.method(cb); // 'type patch'
  dummyInstance.method(cb); // 'type patch'
  overrideInstance.method(cb); // 'override <root>'
  nativeOverride.apply(overrideInstance, [cb]); // 'type <root>'
});

const nativeOverride = patchType(OverrideType); // outputs 'patched' because the parent class hasn't been patched so it does not inherit proto[__symbol__('method')] yet. Returns the unpatched OverrideType.prototype.method
patchType(Type); // outputs 'patched'. Returns the unpatched Type.prototype.method
patchType(Dummy); // no output. Returns the unpatched Type.prototype.method
zone.run(() => {
  typeInstance.method(cb); // 'type patch'
  dummyInstance.method(cb); // 'type patch'
  overrideInstance.method(cb); // 'override patch'
  nativeOverride.apply(overrideInstance, [cb]); // 'override <root>'
});

Issue Number: N/A

What is the new behavior?

patchMethod correctly patches a class that overrides a parent method. It now verifies if the __symbol__(delegateName) is defined in the prototype and not just inherited, resulting in the following behavior:

patchType(Type); // outputs 'patched'. Returns the unpatched Type.prototype.method
patchType(Dummy); // no output as method is just inherited. Returns the unpatched Type.prototype.method
const nativeOverride = patchType(OverrideType); // outputs 'patched' as proto.hasOwnProperty(__symbol__('method')) == false. Returns the unpatched OverrideType.prototype.method
zone.run(() => {
  typeInstance.method(cb); // 'type patch'
  dummyInstance.method(cb); // 'type patch'
  overrideInstance.method(cb); // 'override patch'
  nativeOverride.apply(overrideInstance, [cb]); // 'override <root>'
});

// this scenario is the same as before
const nativeOverride = patchType(OverrideType); // outputs 'patched'. Returns the unpatched OverrideType.prototype.method
patchType(Type); // outputs 'patched'. Returns the unpatched Type.prototype.method
patchType(Dummy); // no output. Returns the unpatched Type.prototype.method
zone.run(() => {
  typeInstance.method(cb); // 'type patch'
  dummyInstance.method(cb); // 'type patch'
  overrideInstance.method(cb); // 'override patch'
  nativeOverride.apply(overrideInstance, [cb]); // 'override <root>'
});

Does this PR introduce a breaking change?

• Yes
• No

Other information

This issue arose while providing patches to the NativeScript framework, which has an Observable class which emits events. View, which inherits Observable, overrides addEventListener and provides it's own implementation to emit gesture events, as it needs to keep track of how many observers there are to remove the listeners when there are none.

[JiaLiPassion]

@edusperoni , please write some details information to describe,

• What is the issue?
• How this PR resolve it?

And also please make lint task pass, please also add the issue and how this PR resolve the issue in the commit message.
Finally please squash the two commits.

Thanks.

[edusperoni]

@JiaLiPassion I've added more information in the description. Is this enough?

You want me to add that information in the commit as well, correct?

Edit: Also, yarn lint is not outputting any errors on my code, can you clarify the issue?

[JiaLiPassion]

LGTM, thanks.
And the lint is failing because the commit messages are not correct.
Please add the description you added in this PR to the commit message, and following the rule described here.

https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-commit-message-format

[mhevery]

reviewed-for: global-approvers

[mhevery]

presubmit

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}