Skip to content

Commit 42ab5d4

Browse files
jnarebgitster
authored andcommitted
gitweb.js: use setTimeout rather than setInterval in blame_incremental.js
If there is a possibility that your logic could take longer to execute than the interval time, it is recommended that you recursively call a named function using window.setTimeout rather than window.setInterval. Therefore instead of using setInterval as an alternate way of invoking handleResponse (because some web browsers call onreadystatechange only once per each distinct state, and not for each server flush), use setTimeout and reset it from handleResponse. As a bonus this allows us to get rid of timer if it turns out that web browser calls onreadystatechange on each server flush. While at it get rid of `xhr' global variable, creating it instead as local variable in startBlame and passing it as parameter, and of `pollTimer' global variable, passing it as member of xhr object (xhr.pollTimer). Signed-off-by: Jakub Narebski <jnareb@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent e8dd0e4 commit 42ab5d4

File tree

1 file changed

+40
-15
lines changed

1 file changed

+40
-15
lines changed

gitweb/static/js/blame_incremental.js

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
/* ............................................................ */
3030
/* utility/helper functions (and variables) */
3131

32-
var xhr; // XMLHttpRequest object
3332
var projectUrl; // partial query + separator ('?' or ';')
3433

3534
// 'commits' is an associative map. It maps SHA1s to Commit objects.
@@ -431,8 +430,6 @@ var endRe = /^END ?([^ ]*) ?(.*)/;
431430
var curCommit = new Commit();
432431
var curGroup = {};
433432

434-
var pollTimer = null;
435-
436433
/**
437434
* Parse output from 'git blame --incremental [...]', received via
438435
* XMLHttpRequest from server (blamedataUrl), and call handleLine
@@ -533,26 +530,34 @@ function processData(unprocessed, nextReadPos) {
533530
* Handle XMLHttpRequest errors
534531
*
535532
* @param {XMLHttpRequest} xhr: XMLHttpRequest object
533+
* @param {Number} [xhr.pollTimer] ID of the timeout to clear
536534
*
537-
* @globals pollTimer, commits
535+
* @globals commits
538536
*/
539537
function handleError(xhr) {
540538
errorInfo('Server error: ' +
541539
xhr.status + ' - ' + (xhr.statusText || 'Error contacting server'));
542540

543-
clearInterval(pollTimer);
541+
if (typeof xhr.pollTimer === "number") {
542+
clearTimeout(xhr.pollTimer);
543+
delete xhr.pollTimer;
544+
}
544545
commits = {}; // free memory
545546
}
546547

547548
/**
548549
* Called after XMLHttpRequest finishes (loads)
549550
*
550-
* @param {XMLHttpRequest} xhr: XMLHttpRequest object (unused)
551+
* @param {XMLHttpRequest} xhr: XMLHttpRequest object
552+
* @param {Number} [xhr.pollTimer] ID of the timeout to clear
551553
*
552-
* @globals pollTimer, commits
554+
* @globals commits
553555
*/
554556
function responseLoaded(xhr) {
555-
clearInterval(pollTimer);
557+
if (typeof xhr.pollTimer === "number") {
558+
clearTimeout(xhr.pollTimer);
559+
delete xhr.pollTimer;
560+
}
556561

557562
fixColorsAndGroups();
558563
writeTimeInterval();
@@ -563,9 +568,13 @@ function responseLoaded(xhr) {
563568
* handler for XMLHttpRequest onreadystatechange event
564569
* @see startBlame
565570
*
566-
* @globals xhr
571+
* @param {XMLHttpRequest} xhr: XMLHttpRequest object
572+
* @param {Number} xhr.prevDataLength: previous value of xhr.responseText.length
573+
* @param {Number} xhr.nextReadPos: start of unread part of xhr.responseText
574+
* @param {Number} [xhr.pollTimer] ID of the timeout (to reset or cancel)
575+
* @param {Boolean} fromTimer: if handler was called from timer
567576
*/
568-
function handleResponse() {
577+
function handleResponse(xhr, fromTimer) {
569578

570579
/*
571580
* xhr.readyState
@@ -614,6 +623,19 @@ function handleResponse() {
614623
// did we finish work?
615624
if (xhr.readyState === 4) {
616625
responseLoaded(xhr);
626+
return;
627+
}
628+
629+
// if we get from timer, we have to restart it
630+
// otherwise onreadystatechange gives us partial response, timer not needed
631+
if (fromTimer) {
632+
setTimeout(function () {
633+
handleResponse(xhr, true);
634+
}, 1000);
635+
636+
} else if (typeof xhr.pollTimer === "number") {
637+
clearTimeout(xhr.pollTimer);
638+
delete xhr.pollTimer;
617639
}
618640
}
619641

@@ -629,11 +651,11 @@ function handleResponse() {
629651
* Called from 'blame_incremental' view after loading table with
630652
* file contents, a base for blame view.
631653
*
632-
* @globals xhr, t0, projectUrl, div_progress_bar, totalLines, pollTimer
654+
* @globals t0, projectUrl, div_progress_bar, totalLines
633655
*/
634656
function startBlame(blamedataUrl, bUrl) {
635657

636-
xhr = createRequestObject();
658+
var xhr = createRequestObject();
637659
if (!xhr) {
638660
errorInfo('ERROR: XMLHttpRequest not supported');
639661
return;
@@ -652,16 +674,19 @@ function startBlame(blamedataUrl, bUrl) {
652674
xhr.prevDataLength = -1; // used to detect if we have new data
653675
xhr.nextReadPos = 0; // where unread part of response starts
654676

655-
xhr.onreadystatechange = handleResponse;
656-
//xhr.onreadystatechange = function () { handleResponse(xhr); };
677+
xhr.onreadystatechange = function () {
678+
handleResponse(xhr, false);
679+
};
657680

658681
xhr.open('GET', blamedataUrl);
659682
xhr.setRequestHeader('Accept', 'text/plain');
660683
xhr.send(null);
661684

662685
// not all browsers call onreadystatechange event on each server flush
663686
// poll response using timer every second to handle this issue
664-
pollTimer = setInterval(xhr.onreadystatechange, 1000);
687+
xhr.pollTimer = setTimeout(function () {
688+
handleResponse(xhr, true);
689+
}, 1000);
665690
}
666691

667692
/* end of blame_incremental.js */

0 commit comments

Comments
 (0)