[Sheriff-o-matic] Remove race condition on the commit list.

Tools/GardeningServer/model/ct-commit-log.html

 <!--
 Copyright 2014 The Chromium Authors. All rights reserved.
 Use of this source code is governed by a BSD-style license that can be
 found in the LICENSE file.
 -->
 
 <link rel="import" href="../lib/net.html">
 <link rel='import' href='ct-commit.html'>
 <link rel='import' href='ct-repositories.html'>
 
 <script>
 function CTCommitLog() {
   this.commits = {};
   this.firstRevision = {};
   this.lastRevision = {};
   this._repositories = new CTRepositories();
+
+  this._repositories.names().forEach(function(name) {
+    this.commits[name] = {};
+  }.bind(this));
 }
 
 CTCommitLog.prototype.update = function() {
   var requests = [];
   Object.keys(this._repositories.repositories, (function(name, repository) {
     var responseHandler = this._handleResponse.bind(this, repository.repositoryUrl, repository.name);
     // FIXME: Turn net.js into net.html and import it at the top of this file.
     requests.push(net.ajax({url: repository.dataUrl}).then(responseHandler));
   }.bind(this)));
   return Promise.all(requests);
 }
 
 CTCommitLog.prototype._handleResponse = function(repositoryUrl, repository, json) {
-  if (!this.commits[repository])
-    this.commits[repository] = {};
-
   // FIXME: Fix googlesource to not do this outdated JSON XSS mitigation.
   json = json.substring(')]}\n'.length);
   JSON.parse(json).log.forEach(function(entry) {
-    var commit = CTCommit.create(entry.author.name, entry.message, repositoryUrl, repository);
+    var author = entry.author.name;
+    var message = entry.message;
+    var commit = CTCommit.create(author, message, repositoryUrl, repository);

[michaelpg, 2014/08/20 19:32:32]

This is a weird pattern to me, you're creating a commit and then maybe not using
it. If you make CTCommit._findRevision public static you could just pass it the
message to find the revision here and see whether a commit already exists before
creating one. 

[Mathieu, 2014/08/21 14:08:21]

Done.

+
+    // Update the existing commit instead of creating a new one so that
+    // the data-binding properly updates all the <ct-commit>'s.
+    var existingCommit = this.commits[repository][commit.revision];
+    if (existingCommit && !existingCommit.isComplete) {
+      existingCommit.complete(author, message);
+      return;
+    }
+
     this.commits[repository][commit.revision] = commit;
   }.bind(this));
 
   this._findFirstAndLastRevisions(repository);
 }
 
 CTCommitLog.prototype._findFirstAndLastRevisions = function(repository) {
   var sortedCommits = Object.keys(this.commits[repository]).sort();
   this.firstRevision[repository] = parseInt(sortedCommits.first());
   this.lastRevision[repository] = parseInt(sortedCommits.last());
 }
 
 CTCommitLog.prototype.range = function(repository, first, last) {
   var commits = [];
   for (var revision = first; revision <= last; revision++) {
-    // Until we've seen a commit we have data for, fill in dummy data
-    // so that we show something. Assume that we have the correct data
-    // for all the commits after the first one we have data for.
-    if (revision >= this.firstRevision[repository]) {
-      var commit = this.commits[repository][revision];
-      if (commit)
-        commits.push(commit);
-    } else {
-      var url = this._repositories.repositories[repository].repositoryUrl;
-      commits.push(CTCommit.createIncomplete(url, revision, repository));
-    }
+     var commit = this.commits[repository][revision];
+     if (!commit) {
+       // FIXME: This is wrong. If we iterate through revisions and then later
+       // in handleReponse we realize that this revision doesn't actually exist
+       // in the repo (e.g. because it corresponds to a branch commit), then we
+       // don't remove the incomplete commit.
+       var url = this._repositories.repositories[repository].repositoryUrl;
+       commit = CTCommit.createIncomplete(url, revision, repository);
+       this.commits[repository][revision] = commit;
+     }
+
+     if (commit)
+       commits.push(commit);
   }
   return commits;
 }
 </script>