Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3637)

Unified Diff: chrome/browser/resources/extensions/extension_list.js

Issue 1049483003: [Extensions] Update extensions UI to observe events and add test (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/resources/extensions/extension_list.js
diff --git a/chrome/browser/resources/extensions/extension_list.js b/chrome/browser/resources/extensions/extension_list.js
index 15832f38b4e0277a4b4cb6a14875f192bf169eaf..ff76a2b21bd23ba9fd5e4a5067b21237456d2478 100644
--- a/chrome/browser/resources/extensions/extension_list.js
+++ b/chrome/browser/resources/extensions/extension_list.js
@@ -146,8 +146,7 @@ cr.define('extensions', function() {
function ExtensionList() {
var div = document.createElement('div');
div.__proto__ = ExtensionList.prototype;
- /** @private {!Array<ExtensionInfo>} */
- div.extensions_ = [];
+ div.initialize();
return div;
}
@@ -206,6 +205,43 @@ cr.define('extensions', function() {
enableAppInfoDialog_: false,
/**
+ * Initializes the list.
+ */
+ initialize: function() {
+ /** @private {!Array<ExtensionInfo>} */
+ this.extensions_ = [];
+
+ /** @private {Promise} */
+ this.extensionsUpdated_ = null;
Dan Beam 2015/04/06 23:53:12 1) why are you keeping a |this.extensionsUpdated_|
Dan Beam 2015/04/06 23:55:48 ok, figured out 1) (using in tests), but why 2)?
Devlin 2015/04/07 16:07:54 2 was because I thought it was best practice to in
+
+ chrome.developerPrivate.onItemStateChanged.addListener(
+ function(eventData) {
+ var EventType = chrome.developerPrivate.EventType;
+ switch (eventData.event_type) {
+ case EventType.VIEW_REGISTERED:
+ case EventType.VIEW_UNREGISTERED:
+ // For now, view notifications are handled through the WebUI.
+ // TODO(devlin): Transition these.
+ break;
+ case EventType.INSTALLED:
+ case EventType.LOADED:
+ case EventType.UNLOADED:
+ case EventType.ERROR_ADDED:
+ if (eventData.extensionInfo)
Dan Beam 2015/04/06 23:53:12 when will |eventData.extensionInfo| not exist?
Devlin 2015/04/07 16:07:54 In the case of it being uninstalled, for one. The
+ this.updateExtension_(eventData.extensionInfo);
+ break;
+ case EventType.UNINSTALLED:
+ var childNode = $(eventData.item_id);
+ if (childNode)
Dan Beam 2015/04/06 23:53:12 when will childNode not exist?
Devlin 2015/04/07 16:07:54 In theory, never. Removed.
+ childNode.parentNode.removeChild(childNode);
+ break;
+ default:
+ assertNotReached();
+ }
+ }.bind(this));
+ },
+
+ /**
* Updates the extensions on the page.
* @param {boolean} incognitoAvailable Whether or not incognito is allowed.
* @param {boolean} enableAppInfoDialog Whether or not the app info dialog
@@ -218,7 +254,7 @@ cr.define('extensions', function() {
// consider passing in the full object from the ExtensionSettings.
this.incognitoAvailable_ = incognitoAvailable;
this.enableAppInfoDialog_ = enableAppInfoDialog;
- return new Promise(function(resolve, reject) {
+ this.extensionsUpdated_ = new Promise(function(resolve, reject) {
chrome.developerPrivate.getExtensionsInfo(
{includeDisabled: true, includeTerminated: true},
function(extensions) {
@@ -241,6 +277,7 @@ cr.define('extensions', function() {
resolve();
}.bind(this));
}.bind(this));
+ return this.extensionsUpdated_;
},
/** @return {number} The number of extensions being displayed. */
@@ -268,15 +305,9 @@ cr.define('extensions', function() {
var seenIds = [];
// Iterate over the extension data and add each item to the list.
- this.extensions_.forEach(function(extension, i) {
- var nextExt = this.extensions_[i + 1];
- var node = $(extension.id);
+ this.extensions_.forEach(function(extension) {
seenIds.push(extension.id);
-
- if (node)
- this.updateNode_(extension, node);
- else
- this.createNode_(extension, nextExt ? $(nextExt.id) : null);
+ this.updateExtension_(extension);
}, this);
// Remove extensions that are no longer installed.
@@ -982,6 +1013,22 @@ cr.define('extensions', function() {
// after its showing animation? Makes very little sense to me.
overlay.setInitialFocus();
},
+
+ /**
+ * Updates the node for the extension.
+ * @param {!ExtensionInfo} extension The information about the extension to
+ * update.
+ * @private
+ */
+ updateExtension_: function(extension) {
+ var node = /** @type {ExtensionFocusRow} */ ($(extension.id));
+ if (node) {
+ this.updateNode_(extension, node);
+ } else {
+ var nextExt = this.extensions_[this.extensions_.indexOf(extension) + 1];
+ this.createNode_(extension, nextExt ? $(nextExt.id) : null);
+ }
+ }
};
return {

Powered by Google App Engine
This is Rietveld 408576698