From e1316ea48b51810bf1fdb431f749d523e744a127 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= <gael.metais@gmail.com>
Date: Sun, 10 Jul 2016 15:11:34 +0800
Subject: [PATCH] Add a new rule for Synchronous XMLHttpRequest (#178)

* Fix bug in spying function
* New rule Synchronous XHR
* Add the synchronousXHR rule to the dashboard
---
 front/src/views/rule.html                     |  2 +-
 lib/metadata/policies.js                      |  9 ++++++
 lib/metadata/scoreProfileGeneric.json         |  3 +-
 .../custom_modules/core/scopeYLT/scopeYLT.js  |  5 ++--
 .../modules/ajaxReqYLT/ajaxReqYLT.js          | 30 +++++++++++++++++++
 lib/tools/phantomas/phantomasWrapper.js       |  9 +++---
 test/www/jquery-page.html                     |  5 ++++
 7 files changed, 55 insertions(+), 8 deletions(-)
 create mode 100644 lib/tools/phantomas/custom_modules/modules/ajaxReqYLT/ajaxReqYLT.js

diff --git a/front/src/views/rule.html b/front/src/views/rule.html
index eb14e77..ee8fa78 100644
--- a/front/src/views/rule.html
+++ b/front/src/views/rule.html
@@ -155,7 +155,7 @@
                         (<ng-pluralize count="offender.requests" when="{'one':'1 request','other':'{} requests'}"></ng-pluralize>)
                     </div>
 
-                    <div ng-if="policyName === 'globalVariables' || policyName === 'jQueryVersionsLoaded'">
+                    <div ng-if="policyName === 'globalVariables' || policyName === 'jQueryVersionsLoaded' || policyName === 'synchronousXHR'">
                         {{offender}}
                     </div>
 
diff --git a/lib/metadata/policies.js b/lib/metadata/policies.js
index f94495c..869f50a 100644
--- a/lib/metadata/policies.js
+++ b/lib/metadata/policies.js
@@ -241,6 +241,15 @@ var policies = {
             };
         }
     },
+    "synchronousXHR": {
+        "tool": "phantomas",
+        "label": "Synchronous Ajax requests",
+        "message": "<p>Making an XMLHttpRequest with the <i>async</i> option set to <i>false</i> is deprecated due to the negative effect to performances. The browser's main thread needs to stop everything until the response is received.</p>",
+        "isOkThreshold": 0,
+        "isBadThreshold": 1,
+        "isAbnormalThreshold": 1,
+        "hasOffenders": true
+    },
     "consoleMessages": {
         "tool": "phantomas",
         "label": "Console messages",
diff --git a/lib/metadata/scoreProfileGeneric.json b/lib/metadata/scoreProfileGeneric.json
index bf050f7..565ead7 100644
--- a/lib/metadata/scoreProfileGeneric.json
+++ b/lib/metadata/scoreProfileGeneric.json
@@ -50,6 +50,7 @@
             "policies": {
                 "jsErrors": 1,
                 "documentWriteCalls": 2,
+                "synchronousXHR": 5,
                 "consoleMessages": 0.5,
                 "globalVariables": 0.5
             }
@@ -108,7 +109,7 @@
         "domComplexity": 2,
         "domManipulations": 2,
         "scroll": 1,
-        "badJavascript": 1,
+        "badJavascript": 2,
         "jQuery": 1,
         "cssSyntaxError": 1,
         "cssComplexity": 1,
diff --git a/lib/tools/phantomas/custom_modules/core/scopeYLT/scopeYLT.js b/lib/tools/phantomas/custom_modules/core/scopeYLT/scopeYLT.js
index aa0aaa7..322f504 100644
--- a/lib/tools/phantomas/custom_modules/core/scopeYLT/scopeYLT.js
+++ b/lib/tools/phantomas/custom_modules/core/scopeYLT/scopeYLT.js
@@ -49,7 +49,7 @@ exports.module = function(phantomas) {
                             var err;
                             
                             // Before
-                            if (enabled) {
+                            if (enabled && callbackBefore) {
                                 callbackBefore.apply(this, arguments);
                             }
 
@@ -71,7 +71,8 @@ exports.module = function(phantomas) {
 
                                 // After
                                 if (enabled && callbackAfter) {
-                                    callbackAfter.call(this, result, arguments);
+                                    var args = Array.prototype.slice.call(arguments);
+                                    callbackAfter.apply(this, [result].concat(args));
                                 }
 
                                 if (err) {
diff --git a/lib/tools/phantomas/custom_modules/modules/ajaxReqYLT/ajaxReqYLT.js b/lib/tools/phantomas/custom_modules/modules/ajaxReqYLT/ajaxReqYLT.js
new file mode 100644
index 0000000..a1577e8
--- /dev/null
+++ b/lib/tools/phantomas/custom_modules/modules/ajaxReqYLT/ajaxReqYLT.js
@@ -0,0 +1,30 @@
+/**
+ * Analyzes AJAX requests
+ */
+/* global window: true */
+
+exports.version = '0.2.a';
+
+exports.module = function(phantomas) {
+    'use strict';
+
+    phantomas.setMetric('ajaxRequests'); // @desc number of AJAX requests
+    phantomas.setMetric('synchronousXHR'); // @desc number of synchronous 
+
+    phantomas.on('init', function() {
+        phantomas.evaluate(function() {
+            (function(phantomas) {
+                phantomas.spy(window.XMLHttpRequest.prototype, 'open', null, function(result, method, url, async) {
+                    phantomas.incrMetric('ajaxRequests');
+                    phantomas.addOffender('ajaxRequests', '<%s> [%s]', url, method);
+
+                    if (async === false) {
+                        phantomas.incrMetric('synchronousXHR');
+                        phantomas.addOffender('synchronousXHR', url);
+                        phantomas.log('ajaxRequests: synchronous XMLHttpRequest call to <%s>', url);
+                    }
+                }, true);
+            })(window.__phantomas);
+        });
+    });
+};
\ No newline at end of file
diff --git a/lib/tools/phantomas/phantomasWrapper.js b/lib/tools/phantomas/phantomasWrapper.js
index e6a0bc8..910d3d5 100644
--- a/lib/tools/phantomas/phantomasWrapper.js
+++ b/lib/tools/phantomas/phantomasWrapper.js
@@ -39,18 +39,19 @@ var PhantomasWrapper = function() {
             'analyze-css': true,
             'ignore-ssl-errors': true,
             'skip-modules': [
-                'domHiddenContent', // overriden
+                'ajaxRequests', // overridden
+                'domHiddenContent', // overridden
                 'domMutations', // not compatible with webkit
-                'domQueries', // overriden
+                'domQueries', // overridden
                 'events', // overridden
                 'filmStrip', // not needed
                 'har', // not needed for the moment
                 'javaScriptBottlenecks', // needs to be launched after custom module scopeYLT
                 'jQuery', // overridden
                 'jserrors', // overridden
-                'lazyLoadableImages', //overriden
+                'lazyLoadableImages', //overridden
                 'pageSource', // not needed
-                'windowPerformance' // overriden
+                'windowPerformance' // overridden
             ].join(','),
             'include-dirs': [
                 path.join(__dirname, 'custom_modules/core'),
diff --git a/test/www/jquery-page.html b/test/www/jquery-page.html
index f2e3f16..ff03ef8 100644
--- a/test/www/jquery-page.html
+++ b/test/www/jquery-page.html
@@ -190,6 +190,11 @@
         $li.parentsUntil('body', 'div');
         $li.siblings();
         $li.siblings('li');
+
+        $.ajax({
+            url: 'xml.xml',
+            async: false
+        });
     </script>
 </body>
 </html>
\ No newline at end of file