@dhoko opened this Issue on June 9th 2015

Hi,

Inside Piwik.js there is Date.prototype.getTimeAlias = Date.prototype.getTime; (L 5381) why ?

We can only find one call to this alias (L641):

       if (expireDateTime) {
                // the things we do for backwards compatibility...
                // in ECMA-262 5th ed., we could simply use:
                //     while (Date.now() < expireDateTime) { }
                do {
                    now = new Date();
                } while (now.getTimeAlias() < expireDateTime);
            }

Can we remove it and use getTime and keep the prototype clean ?

Edit:

We can find getTime :)

setHeartBeatTimer: function (minimumVisitLength, heartBeatDelay) {
    var now = new Date();

    configMinimumVisitTime = now.getTime() + minimumVisitLength * 1000;
    configHeartBeatTimer = heartBeatDelay * 1000;
},

lastActivityTime = now.getTime();
setTimeout(function heartBeat() {
    var requestPing;
    now = new Date();

    // there was activity during the heart beat period;
    // on average, this is going to overstate the visitDuration by configHeartBeatTimer/2
    if ((lastActivityTime + configHeartBeatTimer) > now.getTime()) {
        // send ping if minimum visit time has elapsed
        if (configMinimumVisitTime < now.getTime()) {
            requestPing = getRequest('ping=1', customData, 'ping');

            sendRequest(requestPing, configTrackerPause);
        }

 if (String(orderId).length) {
     request += '&ec_id=' + encodeWrapper(orderId);
     // Record date of order in the visitor cookie
     lastEcommerceOrderTs = Math.round(now.getTime() / 1000);
 }

...

@tsteur commented on June 9th 2015 Member

The blame says: "fixes https://github.com/piwik/piwik/issues/2929 - workaround Chrome suckage"

From http://forums.thedailywtf.com/forums/p/24374/253916.aspx#253916

I fiddled around a bit, it's really quite targeted. A while() loop alone doesn't trigger the behavior, nor does calling getTime() alone. But if you call getTime() in the condition or body of a while() loop, bam.

Can we remove it and use getTime and keep the prototype clean ?

Does it cause any problems for you or just in general?

@mattab commented on June 10th 2015 Member

@diosmosis is working on #2041 which is related in the sense, that maybe the Ping/heatbeat timer code will be cleaned up (ie. removed!) from the piwik.js. if this code can be completely removed then maybe it would be best way to solve the issue

@dhoko commented on June 10th 2015

Hum, it works as expected today with Chrome, (webkit too).

var s = e = new Date(); 

for( var i = 0; e.getTime() - s.getTime() < 500; i++) 
{ 
    e = new Date(); 
}
Wed Jun 10 2015 08:33:49 GMT+0200 (CEST)
var s = e = new Date(); 

while (e.getTime() - s.getTime() < 1) 
{ 
    e = new Date(); 
}
Wed Jun 10 2015 08:34:00 GMT+0200 (CEST)
var s = e = new Date(); 

while (e.getTime() - s.getTime() < 500) 
{ 
    e = new Date(); 
}
Wed Jun 10 2015 08:34:08 GMT+0200 (CEST)

@tsteur The promblem ? Date.prototype.getTimeAlias...
@mattab ok

@tsteur commented on June 10th 2015 Member

We'd need to figure out which Chrome versions are effected by that but since Chrome updates automatically does versions shouldn't be used anymore. I reckon we could also do something like

function getCurrentTime() {
    var now = new Date();
    now.getTimeAlias = now.getTime;
   return now.getTimeAlias();
}

do {
   // could be also done here instead of in getCurrentTime
} while (getCurrentTime() < expireDateTime);
@mattab commented on July 15th 2015 Member

Feel free to submit a pull request :+1:

@dhoko commented on August 25th 2015

Maybe we can follow the comment:

if (expireDateTime) {
    // the things we do for backwards compatibility...
    // in ECMA-262 5th ed., we could simply use:
    //     while (Date.now() < expireDateTime) { }
    do {
        now = new Date();
    } while (now.getTimeAlias() < expireDateTime);
}

And use Date.now cf Date.now compat table with a custom polyfill for IE8 ? ( Might not be the best idea if you need to support IE8 ).

Powered by GitHub Issue Mirror