Continuously update status in the buildbot extension popup window

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

Index: chrome/common/extensions/docs/examples/extensions/buildbot/active_issues.js
diff --git a/chrome/common/extensions/docs/examples/extensions/buildbot/active_issues.js b/chrome/common/extensions/docs/examples/extensions/buildbot/active_issues.js
new file mode 100644
index 0000000000000000000000000000000000000000..c576f19a2428e43f8c85887df8178d6f32731284
--- /dev/null
+++ b/chrome/common/extensions/docs/examples/extensions/buildbot/active_issues.js
@@ -0,0 +1,64 @@
+// Copyright (c) 2013 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.
+
+(function(){
+
+window.ActiveIssues = function(stateObject) {

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

ditto, buildbot.ActiveIssues

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

Done.

+  this.issues_ = stateObject;
+
+  if (window === chrome.extension.getBackgroundPage()) {
+    var thisObj = this;

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

idiomatically this is 'self'?

also if there was only 1 of these then it would't be needed.

also what happened to just implementing this message passing in-memory? instead
of a port and connectEventsPort allow the popup to set callbacks on the active
issues.

It would imply a bit of a memory leak I suppose... though you could make the
popup clear the existing listeners every time it opened. Or clear them on the
popup's beforeunload event? the listener-adding method here could take the frame
as an argument and add the beforeunload event listener itself.

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

I've moved to a singleton implementation with callback-based message passing.

As far as I can tell, beforeunload isn't invoked on the popup before it is
closed so it doesn't seem to be a viable mechanism for resetting the listener.
I've instead made the popup reset the listener on load, and added logic to only
run the callback code if the popup is open.

+    chrome.extension.onConnect.addListener(function(port) {
+      if (port.name == thisObj.portName_) {
+        thisObj.port_ = port;
+        port.onDisconnect.addListener(function() { thisObj.port_ = null; });
+      }
+    });
+  }
+};
+
+window.ActiveIssues.prototype = {
+  forEach: function(callback) {
+    for (var key in this.issues_)
+      callback(this.issues_[key]);

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

simpler as this.issues_.forEach(callback)?

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

this.issues_ is an Object rather than an Array. I don't think I can use forEach.

+  },
+
+  getIssue: function(number) {
+    return this.issues_[number];
+  },
+
+  updateIssue: function(issue) {
+    var eventType = this.issues_.hasOwnProperty(issue.issue) ?
+      "issueUpdated" : "issueAdded";
+    this.issues_[issue.issue] = issue;
+    if (this.port_)
+      this.port_.postMessage({event: eventType, issue: issue.issue});
+  },
+
+  removeIssue: function(issue) {
+    delete this.issues_[issue.issue];
+    if (this.port_)
+      this.port_.postMessage({event: "issueRemoved", issue: issue.issue});
+  },
+
+  connectEventsPort: function() {
+    return chrome.runtime.connect({name: this.portName_});
+  },
+
+  portName_: "ActiveIssues"
+};
+
+window.getActiveIssues = function() {
+  var stateObjectKey = "activeIssuesState";
+  var background = chrome.extension.getBackgroundPage();
+  if (!background.hasOwnProperty(stateObjectKey))
+    background[stateObjectKey] = {};
+
+  var key = "activeIssues";
+  if (!window.hasOwnProperty(key))
+    window[key] = new ActiveIssues(background[stateObjectKey]);
+  return window[key];

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

(responding to earlier comment)

Cool, yeah I get/got that this is so that the state of the popup could outlive
it. I still don't know why you need both stores, though, Like, make
getActiveIssues return the background page's active issues no matter what. That
gives you a singleton. The ActiveIssues object can then own its own store.

Anyhow, I also can't help thinking that this should use the storage API. Maybe I
already said that.

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

I've converted over to a singleton. I had misattributed an unrelated problem to
invocation of functions on the background page from the popup which led me to
using separate objects.

Is the storage API suggestion mainly to get the extension closer to an event
page implementation?

+};
+
+})();