Continuously update status in the buildbot extension popup window

chrome/common/extensions/docs/examples/extensions/buildbot/bg.js

Index: chrome/common/extensions/docs/examples/extensions/buildbot/bg.js
diff --git a/chrome/common/extensions/docs/examples/extensions/buildbot/bg.js b/chrome/common/extensions/docs/examples/extensions/buildbot/bg.js
index b5111f53a6f454fc202ba47ba7dc0f7ded20a006..63dfbf12639a1156d8890aaee4535f0d53229435 100644
--- a/chrome/common/extensions/docs/examples/extensions/buildbot/bg.js
+++ b/chrome/common/extensions/docs/examples/extensions/buildbot/bg.js
@@ -7,6 +7,8 @@
 // Notification object when event pages get reloaded.  See
 // http://crbug.com/165276.
 
+(function() {
+
 var statusURL = "http://chromium-status.appspot.com/current?format=raw";
 var statusHistoryURL =
   "http://chromium-status.appspot.com/allstatus?limit=20&format=json";
@@ -119,13 +121,12 @@ function requestStatus() {
   setTimeout(requestStatus, pollFrequencyInMs);
 }
 
-var activeIssues = {};
 // Record of the last defunct build number we're aware of on each builder.  If
 // the build number is less than or equal to this number, the buildbot
 // information is not available and a request will return a 404.
 var lastDefunctTryJob = {};
 
-function fetchTryJobResults(fullPatchset, builder, buildnumber) {
+function fetchTryJobResults(fullPatchset, builder, buildnumber, completed) {
   var tryJobURL =
     "http://build.chromium.org/p/tryserver.chromium/json/builders/" +
         builder + "/builds/" + buildnumber;
@@ -135,8 +136,11 @@ function fetchTryJobResults(fullPatchset, builder, buildnumber) {
     return;
 
   var onStatusError = function(status) {

[not at google - send to devlin, 2013/07/04 00:37:11]

can you inline this function, or un-inline the other one as well? looks a bit
odd to have one and not the other. both inlined preferable.

[Mike Wittman, 2013/07/16 17:53:19]

Done.

-    if (status == 404)
-      lastDefunctTryJob[builder] = buildnumber;
+    if (status == 404) {
+      lastDefunctTryJob[builder] =
+          Math.max(lastDefunctTryJob[builder] || 0, buildnumber);
+      completed();
+    }
   };
 
   requestURL(tryJobURL, "json", function(tryJobResult) {
@@ -145,11 +149,20 @@ function fetchTryJobResults(fullPatchset, builder, buildnumber) {
 
     var key = builder + "-" + buildnumber;
     fullPatchset.full_try_job_results[key] = tryJobResult;
+
+    completed();
   }, onStatusError);
 }
 
-function fetchPatches(issue, completed) {
+// Enums corresponding to how much state has been loaded for an issue.
+var PATCHES_COMPLETE = 0;
+var TRY_JOBS_COMPLETE = 1;
+
+function fetchPatches(issue, updatedCallback) {
+  // Notify updated once after receiving all patchsets, and a second time after
+  // receiving all try job results.
   var patchsetsRetrieved = 0;
+  var tryJobResultsOutstanding = 0;
   issue.patchsets.forEach(function(patchset) {
     var patchURL = "https://codereview.chromium.org/api/" + issue.issue +
         "/" + patchset;
@@ -161,45 +174,82 @@ function fetchPatches(issue, completed) {
       issue.full_patchsets[patch.patchset] = patch;
 
       patch.try_job_results.forEach(function(results) {
-        if (results.buildnumber)
-          fetchTryJobResults(patch, results.builder, results.buildnumber);
+        if (results.buildnumber) {
+          tryJobResultsOutstanding++;
+
+          fetchTryJobResults(
+            patch,
+            results.builder,
+            results.buildnumber,
+            function() {
+              if (--tryJobResultsOutstanding == 0)
+                updatedCallback(TRY_JOBS_COMPLETE);
+            });

[not at google - send to devlin, 2013/07/04 00:37:11]

looks a bit, odd, I'd probably cheat like

         fetchTryJobResults(patch, results.builder, results.buildnumber,
                            function() {
           if (--tryJobResultsOutstanding == 0)
             updatedCallback(TRY_JOBS_COMPLETE);
         });

looks less weird.

[Mike Wittman, 2013/07/16 17:53:19]

Done.

+        }
       });
 
       if (++patchsetsRetrieved == issue.patchsets.length)
-        completed(issue);
+        updatedCallback(PATCHES_COMPLETE);
     });
   });
 }
 
 function updateTryStatus(status) {
   var seen = {};
+  var activeIssues = getActiveIssues();
   status.results.forEach(function(result) {
     var issueURL = "https://codereview.chromium.org/api/" + result.issue;
 
     requestURL(issueURL, "json", function(issue) {
-      fetchPatches(issue, function() {activeIssues[issue.issue] = issue;});
+      fetchPatches(issue, function(state) {
+        // If the issue already exists, wait until all the issue state has
+        // loaded before updating the issue so we don't lose try job information
+        // from the display.
+        if (activeIssues.getIssue(issue.issue)) {
+          if (state == TRY_JOBS_COMPLETE)
+            activeIssues.updateIssue(issue);
+        } else {
+          activeIssues.updateIssue(issue);
+        }
+      });
     });
 
     seen[result.issue] = true;
   });
 
-  for (var issue in activeIssues)
-    if (!seen[issue])
-      delete activeIssues[issue];
+  activeIssues.forEach(function(issue) {
+    if (!seen[issue.issue])
+      activeIssues.removeIssue(issue);
+  });
 }
 
+var tryStatusTimeoutID;
+
 function requestTryStatus() {
+  var searchBaseURL = "https://codereview.chromium.org/search";
+
   prefs.getTryJobUsername(function(username) {
-    if (username) {
-      var url = "https://codereview.chromium.org/search" +
+    // Use the username if set and not consisting solely of whitespace.
+    if (username && username.trim()) {
+      var url = searchBaseURL +
           // commit=2 is CLs with commit bit set, commit=3 is CLs with commit
           // bit cleared, commit=1 is either.
           "?closed=3&commit=1&limit=100&order=-modified&format=json&owner=" +
-              username;
+              username.trim();
       requestURL(url, "json", updateTryStatus);
+    } else if (username == null) {
+      // Try scraping username from Rietveld if unset.
+      requestURL(searchBaseURL, "text", function(text) {
+        var match = /([^<>\s]+@\S+)\s+\(.+\)/.exec(text);
+        if (match) {
+          prefs.setTryJobUsername(match[1]);
+          clearTimeout(tryStatusTimeoutID);

[not at google - send to devlin, 2013/07/04 00:37:11]

I'm a bit confused by the flow here, with the clearTimeout only in this branch,
I don't see how it's related.

[Mike Wittman, 2013/07/16 17:53:19]

I've refactored this to avoid the clearTimeout and hopefully make it less
confusing.

+          requestTryStatus();
+        }
+      });
     }
 
-    setTimeout(requestTryStatus, tryPollFrequencyInMs);
+    tryStatusTimeoutID = setTimeout(requestTryStatus, tryPollFrequencyInMs);
   });
 }
 
@@ -209,3 +259,5 @@ function main() {
 }
 
 main();
+
+})();