fix(websocket): fix #824, patch websocket onproperties correctly in P…

…hantomJS (#826)

* fix(websocket): fix #824, patch websocket onproperties correctly in phantomjs

* add phantomjs test for local and travis

.travis.yml

@@ -27,6 +27,7 @@ script:
   - node_modules/.bin/gulp build
   - scripts/closure/closure_compiler.sh
   - node_modules/.bin/gulp promisetest
+  - npm run test:phantomjs-single
   - node_modules/.bin/karma start karma-dist-sauce-jasmine.conf.js --single-run
   - node_modules/.bin/karma start karma-build-sauce-mocha.conf.js --single-run
   - node_modules/.bin/karma start karma-dist-sauce-selenium3-jasmine.conf.js --single-run

karma-build-jasmine-phantomjs.conf.js

@@ -0,0 +1,9 @@
+
+module.exports = function (config) {
+  require('./karma-build.conf.js')(config);
+
+  config.plugins.push(require('karma-jasmine'));
+  config.plugins.push(require('karma-phantomjs-launcher'));
+  config.frameworks.push('jasmine');
+  config.browsers.splice(0, 1, ['PhantomJS']);
+};

lib/browser/websocket.ts

@@ -21,10 +21,16 @@ export function apply(_global: any) {
     const socket = arguments.length > 1 ? new WS(a, b) : new WS(a);
     let proxySocket: any;
 
+    let proxySocketProto: any;
+
     // Safari 7.0 has non-configurable own 'onmessage' and friends properties on the socket instance
     const onmessageDesc = Object.getOwnPropertyDescriptor(socket, 'onmessage');
     if (onmessageDesc && onmessageDesc.configurable === false) {
       proxySocket = Object.create(socket);
+      // socket have own property descriptor 'onopen', 'onmessage', 'onclose', 'onerror'
+      // but proxySocket not, so we will keep socket as prototype and pass it to
+      // patchOnProperties method
+      proxySocketProto = socket;
       ['addEventListener', 'removeEventListener', 'send', 'close'].forEach(function(propName) {
         proxySocket[propName] = function() {
           return socket[propName].apply(socket, arguments);
@@ -35,7 +41,7 @@ export function apply(_global: any) {
       proxySocket = socket;
     }
 
-    patchOnProperties(proxySocket, ['close', 'error', 'message', 'open']);
+    patchOnProperties(proxySocket, ['close', 'error', 'message', 'open'], proxySocketProto);
 
     return proxySocket;
   };

package.json

@@ -19,8 +19,10 @@
     "closure:test": "scripts/closure/closure_compiler.sh",
     "format": "gulp format:enforce",
     "karma-jasmine": "karma start karma-build-jasmine.conf.js",
+    "karma-jasmine:phantomjs": "karma start karma-build-jasmine-phantomjs.conf.js --single-run",
     "karma-jasmine:single": "karma start karma-build-jasmine.conf.js --single-run",
     "karma-jasmine:autoclose": "npm run karma-jasmine:single && npm run ws-client",
+    "karma-jasmine-phantomjs:autoclose": "npm run karma-jasmine:phantomjs && npm run ws-client",
     "lint": "gulp lint",
     "prepublish": "tsc && gulp build",
     "promisetest": "gulp promisetest",
@@ -33,6 +35,8 @@
     "tsc": "tsc",
     "tsc:w": "tsc -w",
     "test": "npm run tsc && concurrently \"npm run tsc:w\" \"npm run ws-server\" \"npm run karma-jasmine\"",
+    "test:phantomjs": "npm run tsc && concurrently \"npm run tsc:w\" \"npm run ws-server\" \"npm run karma-jasmine:phantomjs\"",
+    "test:phantomjs-single": "concurrently \"npm run ws-server\" \"npm run karma-jasmine-phantomjs:autoclose\"",
     "test:single": "npm run tsc && concurrently \"npm run ws-server\" \"npm run karma-jasmine:autoclose\"",
     "test-dist": "concurrently \"npm run tsc:w\" \"npm run ws-server\" \"karma start karma-dist-jasmine.conf.js\"",
     "test-node": "gulp test/node",
@@ -74,11 +78,13 @@
     "karma-firefox-launcher": "^0.1.4",
     "karma-jasmine": "^0.3.6",
     "karma-mocha": "^1.2.0",
+    "karma-phantomjs-launcher": "^1.0.4",
     "karma-safari-launcher": "^0.1.1",
     "karma-sauce-launcher": "^0.2.10",
     "karma-sourcemap-loader": "^0.3.6",
     "mocha": "^3.1.2",
     "nodejs-websocket": "^1.2.0",
+    "phantomjs": "^2.1.7",
     "promises-aplus-tests": "^2.1.2",
     "pump": "^1.0.1",
     "selenium-webdriver": "^3.4.0",

test/browser/Notification.spec.ts

@@ -15,7 +15,7 @@ declare const window: any;
 function notificationSupport() {
   const desc = window['Notification'] &&
       Object.getOwnPropertyDescriptor(window['Notification'].prototype, 'onerror');
-  return window['Notification'] && window['Notification'].prototype && desc.configurable;
+  return window['Notification'] && window['Notification'].prototype && desc && desc.configurable;
 }
 
 (<any>notificationSupport).message = 'Notification Support';

test/test-util.ts

@@ -67,4 +67,22 @@ export function supportPatchXHROnProperty() {
     return false;
   }
   return true;
-}
+}
+
+let supportSetErrorStack = true;
+
+export function isSupportSetErrorStack() {
+  try {
+    throw new Error('test');
+  } catch (err) {
+    try {
+      err.stack = 'new stack';
+      supportSetErrorStack = err.stack === 'new stack';
+    } catch (error) {
+      supportSetErrorStack = false;
+    }
+  }
+  return supportSetErrorStack;
+}
+
+(isSupportSetErrorStack as any).message = 'supportSetErrorStack';

test/zone-spec/long-stack-trace-zone.spec.ts

@@ -7,136 +7,139 @@
  */
 
 import {zoneSymbol} from '../../lib/common/utils';
+import {ifEnvSupports, isSupportSetErrorStack} from '../test-util';
+
 const defineProperty = (Object as any)[zoneSymbol('defineProperty')] || Object.defineProperty;
 
-describe('longStackTraceZone', function() {
-  let log: Error[];
-  let lstz: Zone;
-  let longStackTraceZoneSpec = (Zone as any)['longStackTraceZoneSpec'];
+describe(
+    'longStackTraceZone', ifEnvSupports(isSupportSetErrorStack, function() {
+      let log: Error[];
+      let lstz: Zone;
+      let longStackTraceZoneSpec = (Zone as any)['longStackTraceZoneSpec'];
 
-  beforeEach(function() {
-    lstz = Zone.current.fork(longStackTraceZoneSpec).fork({
-      name: 'long-stack-trace-zone-test',
-      onHandleError: (parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone,
-                      error: any): boolean => {
-        parentZoneDelegate.handleError(targetZone, error);
-        log.push(error);
-        return false;
-      }
-    });
+      beforeEach(function() {
+        lstz = Zone.current.fork(longStackTraceZoneSpec).fork({
+          name: 'long-stack-trace-zone-test',
+          onHandleError: (parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone,
+                          error: any): boolean => {
+            parentZoneDelegate.handleError(targetZone, error);
+            log.push(error);
+            return false;
+          }
+        });
 
-    log = [];
-  });
+        log = [];
+      });
 
-  function expectElapsed(stack: string, expectedCount: number) {
-    try {
-      let actualCount = stack.split('_Elapsed_').length;
-      if (actualCount !== expectedCount) {
-        expect(actualCount).toEqual(expectedCount);
-        console.log(stack);
+      function expectElapsed(stack: string, expectedCount: number) {
+        try {
+          let actualCount = stack.split('_Elapsed_').length;
+          if (actualCount !== expectedCount) {
+            expect(actualCount).toEqual(expectedCount);
+            console.log(stack);
+          }
+        } catch (e) {
+          expect(e).toBe(null);
+        }
       }
-    } catch (e) {
-      expect(e).toBe(null);
-    }
-  }
 
-  it('should produce long stack traces', function(done) {
-    lstz.run(function() {
-      setTimeout(function() {
-        setTimeout(function() {
+      it('should produce long stack traces', function(done) {
+        lstz.run(function() {
           setTimeout(function() {
-            expectElapsed(log[0].stack, 3);
-            done();
+            setTimeout(function() {
+              setTimeout(function() {
+                expectElapsed(log[0].stack, 3);
+                done();
+              }, 0);
+              throw new Error('Hello');
+            }, 0);
           }, 0);
-          throw new Error('Hello');
-        }, 0);
-      }, 0);
-    });
-  });
+        });
+      });
 
-  it('should produce a long stack trace even if stack setter throws', (done) => {
-    let wasStackAssigned = false;
-    let error = new Error('Expected error');
-    defineProperty(error, 'stack', {
-      configurable: false,
-      get: () => 'someStackTrace',
-      set: (v: any) => {
-        throw new Error('no writes');
-      }
-    });
-    lstz.run(() => {
-      setTimeout(() => {
-        throw error;
+      it('should produce a long stack trace even if stack setter throws', (done) => {
+        let wasStackAssigned = false;
+        let error = new Error('Expected error');
+        defineProperty(error, 'stack', {
+          configurable: false,
+          get: () => 'someStackTrace',
+          set: (v: any) => {
+            throw new Error('no writes');
+          }
+        });
+        lstz.run(() => {
+          setTimeout(() => {
+            throw error;
+          });
+        });
+        setTimeout(() => {
+          const e = log[0];
+          expect((e as any).longStack).toBeTruthy();
+          done();
+        });
       });
-    });
-    setTimeout(() => {
-      const e = log[0];
-      expect((e as any).longStack).toBeTruthy();
-      done();
-    });
-  });
 
-  it('should produce long stack traces when has uncaught error in promise', function(done) {
-    lstz.runGuarded(function() {
-      setTimeout(function() {
-        setTimeout(function() {
-          let promise = new Promise(function(resolve, reject) {
+      it('should produce long stack traces when has uncaught error in promise', function(done) {
+        lstz.runGuarded(function() {
+          setTimeout(function() {
             setTimeout(function() {
-              reject(new Error('Hello Promise'));
+              let promise = new Promise(function(resolve, reject) {
+                setTimeout(function() {
+                  reject(new Error('Hello Promise'));
+                }, 0);
+              });
+              promise.then(function() {
+                fail('should not get here');
+              });
+              setTimeout(function() {
+                expectElapsed(log[0].stack, 5);
+                done();
+              }, 0);
             }, 0);
-          });
-          promise.then(function() {
-            fail('should not get here');
-          });
-          setTimeout(function() {
-            expectElapsed(log[0].stack, 5);
-            done();
           }, 0);
-        }, 0);
-      }, 0);
-    });
-  });
+        });
+      });
 
-  it('should produce long stack traces when handling error in promise', function(done) {
-    lstz.runGuarded(function() {
-      setTimeout(function() {
-        setTimeout(function() {
-          let promise = new Promise(function(resolve, reject) {
+      it('should produce long stack traces when handling error in promise', function(done) {
+        lstz.runGuarded(function() {
+          setTimeout(function() {
             setTimeout(function() {
-              try {
-                throw new Error('Hello Promise');
-              } catch (err) {
-                reject(err);
-              }
+              let promise = new Promise(function(resolve, reject) {
+                setTimeout(function() {
+                  try {
+                    throw new Error('Hello Promise');
+                  } catch (err) {
+                    reject(err);
+                  }
+                }, 0);
+              });
+              promise.catch(function(error) {
+                // should be able to get long stack trace
+                const longStackFrames: string = longStackTraceZoneSpec.getLongStackTrace(error);
+                expectElapsed(longStackFrames, 4);
+                done();
+              });
             }, 0);
-          });
-          promise.catch(function(error) {
-            // should be able to get long stack trace
-            const longStackFrames: string = longStackTraceZoneSpec.getLongStackTrace(error);
-            expectElapsed(longStackFrames, 4);
-            done();
-          });
-        }, 0);
-      }, 0);
-    });
-  });
+          }, 0);
+        });
+      });
 
-  it('should not produce long stack traces if Error.stackTraceLimit = 0', function(done) {
-    const originalStackTraceLimit = Error.stackTraceLimit;
-    lstz.run(function() {
-      setTimeout(function() {
-        setTimeout(function() {
+      it('should not produce long stack traces if Error.stackTraceLimit = 0', function(done) {
+        const originalStackTraceLimit = Error.stackTraceLimit;
+        lstz.run(function() {
           setTimeout(function() {
-            if (log[0].stack) {
-              expectElapsed(log[0].stack, 1);
-            }
-            Error.stackTraceLimit = originalStackTraceLimit;
-            done();
+            setTimeout(function() {
+              setTimeout(function() {
+                if (log[0].stack) {
+                  expectElapsed(log[0].stack, 1);
+                }
+                Error.stackTraceLimit = originalStackTraceLimit;
+                done();
+              }, 0);
+              Error.stackTraceLimit = 0;
+              throw new Error('Hello');
+            }, 0);
           }, 0);
-          Error.stackTraceLimit = 0;
-          throw new Error('Hello');
-        }, 0);
-      }, 0);
-    });
-  });
-});
+        });
+      });
+    }));

test/zone-spec/task-tracking.spec.ts

@@ -61,10 +61,11 @@ describe('TaskTrackingZone', function() {
         xhr.send();
         expect(taskTrackingZoneSpec.macroTasks.length).toBe(1);
         expect(taskTrackingZoneSpec.macroTasks[0].source).toBe('XMLHttpRequest.send');
-        expect(taskTrackingZoneSpec.eventTasks[0].source)
-            .toMatch(/\.addEventListener:readystatechange/);
+        if (supportPatchXHROnProperty()) {
+          expect(taskTrackingZoneSpec.eventTasks[0].source)
+              .toMatch(/\.addEventListener:readystatechange/);
+        }
       });
-
     });
   });