fix(patch): fix #869, should not patch readonly method (#871)

JiaLiPassion · mhevery · commit 31d38c14bc0d · 2017-08-08T09:00:56.000-07:00
diff --git a/lib/common/utils.ts b/lib/common/utils.ts
@@ -283,15 +283,21 @@ export function patchMethod(
     // somehow we did not find it, but we can see it. This happens on IE for Window properties.
     proto = target;
   }
+
   const delegateName = zoneSymbol(name);
   let delegate: Function;
   if (proto && !(delegate = proto[delegateName])) {
     delegate = proto[delegateName] = proto[name];
-    const patchDelegate = patchFn(delegate, delegateName, name);
-    proto[name] = function() {
-      return patchDelegate(this, arguments as any);
-    };
-    attachOriginToPatched(proto[name], delegate);
+    // check whether proto[name] is writable
+    // some property is readonly in safari, such as HtmlCanvasElement.prototype.toBlob
+    const desc = proto && Object.getOwnPropertyDescriptor(proto, name);
+    if (isPropertyWritable(desc)) {
+      const patchDelegate = patchFn(delegate, delegateName, name);
+      proto[name] = function() {
+        return patchDelegate(this, arguments as any);
+      };
+      attachOriginToPatched(proto[name], delegate);
+    }
   }
   return delegate;
 }
diff --git a/test/common/util.spec.ts b/test/common/util.spec.ts
@@ -241,5 +241,92 @@ describe('utils', function() {
       });
       expect(log).toEqual(['property1patch', 'property2<root>']);
     });
+
+    it('non writable method should not be patched', () => {
+      'use strict';
+      const TestFunction: any = function() {};
+      const log: string[] = [];
+      Object.defineProperties(TestFunction.prototype, {
+        'property2': {
+          value: function Property2(callback: Function) {
+            Zone.root.run(callback);
+          },
+          writable: false,
+          configurable: true,
+          enumerable: true
+        }
+      });
+
+      const zone = Zone.current.fork({name: 'patch'});
+
+      zone.run(() => {
+        const instance = new TestFunction();
+        instance.property2(() => {
+          log.push('property2' + Zone.current.name);
+        });
+      });
+      expect(log).toEqual(['property2<root>']);
+      log.length = 0;
+
+      patchMethod(
+          TestFunction.prototype, 'property2',
+          function(delegate: Function, delegateName: string, name: string) {
+            return function(self: any, args: any) {
+              log.push('patched property2');
+            };
+          });
+
+      zone.run(() => {
+        const instance = new TestFunction();
+        instance.property2(() => {
+          log.push('property2' + Zone.current.name);
+        });
+      });
+      expect(log).toEqual(['property2<root>']);
+    });
+
+    it('readonly method should not be patched', () => {
+      'use strict';
+      const TestFunction: any = function() {};
+      const log: string[] = [];
+      Object.defineProperties(TestFunction.prototype, {
+        'property2': {
+          get: function() {
+            return function Property2(callback: Function) {
+              Zone.root.run(callback);
+            };
+          },
+          configurable: true,
+          enumerable: true
+        }
+      });
+
+      const zone = Zone.current.fork({name: 'patch'});
+
+      zone.run(() => {
+        const instance = new TestFunction();
+        instance.property2(() => {
+          log.push('property2' + Zone.current.name);
+        });
+      });
+      expect(log).toEqual(['property2<root>']);
+      log.length = 0;
+
+      patchMethod(
+          TestFunction.prototype, 'property2',
+          function(delegate: Function, delegateName: string, name: string) {
+            return function(self: any, args: any) {
+              log.push('patched property2');
+            };
+          });
+
+      zone.run(() => {
+        const instance = new TestFunction();
+        instance.property2(() => {
+          log.push('property2' + Zone.current.name);
+        });
+      });
+      expect(log).toEqual(['property2<root>']);
+    });
   });
 });
