From f9f3e1275b1aef69d79304d2b3910a7e45883b65 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 6 Apr 2017 23:00:22 -0500 Subject: [PATCH] Bug 1352157 - Avoid listTabs for global actors. r=ochameau Adds a new `getRoot` request to the root actor which lists the global actors only (leaving out the tabs). This is a much better fit for callers who want to access some global actor only, since it avoids visiting every tab, which could be a very expensive operation. MozReview-Commit-ID: 1lIAuaV7zoF --HG-- extra : rebase_source : bf64ee1298591f26ffa4c6a660cca52145084b66 --- devtools/client/framework/target.js | 4 +- devtools/server/actors/root.js | 102 ++++++++++++++++------------ devtools/shared/client/main.js | 7 ++ 3 files changed, 66 insertions(+), 47 deletions(-) diff --git a/devtools/client/framework/target.js b/devtools/client/framework/target.js index dea04777e40a..9efaa1aee590 100644 --- a/devtools/client/framework/target.js +++ b/devtools/client/framework/target.js @@ -277,7 +277,7 @@ TabTarget.prototype = { return this._form; }, - // Get a promise of the root form returned by a listTabs request. This promise + // Get a promise of the root form returned by a getRoot request. This promise // is cached. get root() { if (!this._root) { @@ -288,7 +288,7 @@ TabTarget.prototype = { _getRoot: function () { return new Promise((resolve, reject) => { - this.client.listTabs(response => { + this.client.mainRoot.getRoot(response => { if (response.error) { reject(new Error(response.error + ": " + response.message)); return; diff --git a/devtools/server/actors/root.js b/devtools/server/actors/root.js index 8e71f5b251ed..773551262d7d 100644 --- a/devtools/server/actors/root.js +++ b/devtools/server/actors/root.js @@ -244,11 +244,40 @@ RootActor.prototype = { this._processActors.clear(); }, + /** + * Gets the "root" form, which lists all the global actors that affect the entire + * browser. This can replace usages of `listTabs` that only wanted the global actors + * and didn't actually care about tabs. + */ + onGetRoot: function () { + let reply = { + from: this.actorID, + }; + + // Create global actors + if (!this._globalActorPool) { + this._globalActorPool = new ActorPool(this.conn); + this.conn.addActorPool(this._globalActorPool); + } + this._createExtraActors(this._parameters.globalActorFactories, this._globalActorPool); + + // List the global actors + this._appendExtraActors(reply); + + return reply; + }, + /* The 'listTabs' request and the 'tabListChanged' notification. */ /** * Handles the listTabs request. The actors will survive until at least * the next listTabs request. + * + * ⚠ WARNING ⚠ This can be a very expensive operation, especially if there are many + * open tabs. It will cause us to visit every tab, load a frame script, start a + * debugger server, and read some data. With lazy tab support (bug 906076), this + * would trigger any lazy tabs to be loaded, greatly increasing resource usage. Avoid + * this method whenever possible. */ onListTabs: async function () { let tabList = this._parameters.tabList; @@ -257,19 +286,14 @@ RootActor.prototype = { message: "This root actor has no browser tabs." }; } - /* - * Now that a client has requested the list of tabs, we reattach the onListChanged - * listener in order to be notified if the list of tabs changes again in the future. - */ + // Now that a client has requested the list of tabs, we reattach the onListChanged + // listener in order to be notified if the list of tabs changes again in the future. tabList.onListChanged = this._onTabListChanged; - /* - * Walk the tab list, accumulating the array of tab actors for the - * reply, and moving all the actors to a new ActorPool. We'll - * replace the old tab actor pool with the one we build here, thus - * retiring any actors that didn't get listed again, and preparing any - * new actors to receive packets. - */ + // Walk the tab list, accumulating the array of tab actors for the reply, and moving + // all the actors to a new ActorPool. We'll replace the old tab actor pool with the + // one we build here, thus retiring any actors that didn't get listed again, and + // preparing any new actors to receive packets. let newActorPool = new ActorPool(this.conn); let tabActorList = []; let selected; @@ -287,36 +311,23 @@ RootActor.prototype = { newActorPool.addActor(tabActor); tabActorList.push(tabActor); } - /* DebuggerServer.addGlobalActor support: create actors. */ - if (!this._globalActorPool) { - this._globalActorPool = new ActorPool(this.conn); - this.conn.addActorPool(this._globalActorPool); - } - this._createExtraActors(this._parameters.globalActorFactories, - this._globalActorPool); - /* - * Drop the old actorID -> actor map. Actors that still mattered were - * added to the new map; others will go away. - */ + + // Start with the root reply, which includes the global actors for the whole browser. + let reply = this.onGetRoot(); + + // Drop the old actorID -> actor map. Actors that still mattered were added to the + // new map; others will go away. if (this._tabActorPool) { this.conn.removeActorPool(this._tabActorPool); } this._tabActorPool = newActorPool; this.conn.addActorPool(this._tabActorPool); - let reply = { - "from": this.actorID, - "selected": selected || 0, - "tabs": tabActorList.map(actor => actor.form()) - }; - - /* If a root window is accessible, include its URL. */ - if (this.url) { - reply.url = this.url; - } - - /* DebuggerServer.addGlobalActor support: name actors in 'listTabs' reply. */ - this._appendExtraActors(reply); + // We'll extend the reply here to also mention all the tabs. + Object.assign(reply, { + selected: selected || 0, + tabs: tabActorList.map(actor => actor.form()), + }); return reply; }, @@ -586,16 +597,17 @@ RootActor.prototype = { }; RootActor.prototype.requestTypes = { - "listTabs": RootActor.prototype.onListTabs, - "getTab": RootActor.prototype.onGetTab, - "getWindow": RootActor.prototype.onGetWindow, - "listAddons": RootActor.prototype.onListAddons, - "listWorkers": RootActor.prototype.onListWorkers, - "listServiceWorkerRegistrations": RootActor.prototype.onListServiceWorkerRegistrations, - "listProcesses": RootActor.prototype.onListProcesses, - "getProcess": RootActor.prototype.onGetProcess, - "echo": RootActor.prototype.onEcho, - "protocolDescription": RootActor.prototype.onProtocolDescription + getRoot: RootActor.prototype.onGetRoot, + listTabs: RootActor.prototype.onListTabs, + getTab: RootActor.prototype.onGetTab, + getWindow: RootActor.prototype.onGetWindow, + listAddons: RootActor.prototype.onListAddons, + listWorkers: RootActor.prototype.onListWorkers, + listServiceWorkerRegistrations: RootActor.prototype.onListServiceWorkerRegistrations, + listProcesses: RootActor.prototype.onListProcesses, + getProcess: RootActor.prototype.onGetProcess, + echo: RootActor.prototype.onEcho, + protocolDescription: RootActor.prototype.onProtocolDescription }; exports.RootActor = RootActor; diff --git a/devtools/shared/client/main.js b/devtools/shared/client/main.js index 9e1123bc1af9..c0fd9cc436d2 100644 --- a/devtools/shared/client/main.js +++ b/devtools/shared/client/main.js @@ -1652,6 +1652,13 @@ RootClient.prototype = { constructor: RootClient, /** + * Gets the "root" form, which lists all the global actors that affect the entire + * browser. This can replace usages of `listTabs` that only wanted the global actors + * and didn't actually care about tabs. + */ + getRoot: DebuggerClient.requester({ type: "getRoot" }), + + /** * List the open tabs. * * @param function onResponse