Bug 1485660 - Switch from listening from DebuggerClient to TargetFront. r=jdescottes

* debugger-controller and events.js are special and require to support two cases because this is
the only production codepath that can have a TabTarget or a WorkerTarget.
Thus, leading to either TargetFront or WorkerClient on target.activeTab.
* webide.js doesn't need to listen for tabNavigated, this is redundant with tabListChanged.
* application's initializer. In case you are wondering this code can't be spawn against a WorkerTarget.
The application panel doesn't work in worker toolboxes.
* The code modified in target is in TabTarget, so we don't have to support the WorkerClient case, we always have a TargetFront here.
* I tried to update the doc file the best I can but this all feel outdated.

MozReview-Commit-ID: 2hGchebfIub

Depends on D7458

Differential Revision: https://phabricator.services.mozilla.com/D7459

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Alexandre Poirot 2018-10-15 08:36:10 +00:00
parent e7998a50ec
commit bbd1bcdcbe
13 changed files with 162 additions and 125 deletions

View File

@ -43,8 +43,7 @@ window.Application = {
return toolbox.selectTool(toolId);
}
};
this.client.addListener("workerListChanged", this.updateWorkers);
this.toolbox.target.activeTab.on("workerListChanged", this.updateWorkers);
this.client.addListener("serviceWorkerRegistrationListChanged", this.updateWorkers);
this.client.addListener("registration-changed", this.updateWorkers);
this.client.addListener("processListChanged", this.updateWorkers);
@ -89,7 +88,7 @@ window.Application = {
},
destroy() {
this.client.removeListener("workerListChanged", this.updateWorkers);
this.toolbox.target.activeTab.off("workerListChanged", this.updateWorkers);
this.client.removeListener("serviceWorkerRegistrationListChanged",
this.updateWorkers);
this.client.removeListener("registration-changed", this.updateWorkers);

View File

@ -478,11 +478,22 @@ Workers.prototype = {
}
this._updateWorkerList();
this._tabClient.addListener("workerListChanged", this._onWorkerListChanged);
// `_tabClient` can be BrowsingContextTargetFront (protocol.js front) or
// WorkerClient/DebuggerClient (old fashion client)
if (typeof(this._tabClient.on) == "function") {
this._tabClient.on("workerListChanged", this._onWorkerListChanged);
} else {
this._tabClient.addListener("workerListChanged", this._onWorkerListChanged);
}
},
disconnect: function () {
this._tabClient.removeListener("workerListChanged", this._onWorkerListChanged);
if (typeof(this._tabClient.on) == "function") {
this._tabClient.off("workerListChanged", this._onWorkerListChanged);
} else {
this._tabClient.removeListener("workerListChanged", this._onWorkerListChanged);
}
},
_updateWorkerList: function () {

View File

@ -41,7 +41,13 @@ function setupEvents(dependencies) {
});
if (threadClient._parent) {
threadClient._parent.addListener("workerListChanged", workerListChanged);
// Parent may be BrowsingContextTargetFront and be protocol.js.
// Or DebuggerClient/WorkerClient and still be old fashion actor.
if (threadClient._parent.on) {
threadClient._parent.on("workerListChanged", workerListChanged);
} else {
threadClient._parent.addListener("workerListChanged", workerListChanged);
}
}
}
}
@ -113,4 +119,4 @@ const clientEvents = {
newSource
};
exports.setupEvents = setupEvents;
exports.clientEvents = clientEvents;
exports.clientEvents = clientEvents;

View File

@ -180,8 +180,8 @@ function getAddonActorForId(aClient, aAddonId) {
async function attachTargetActorForUrl(aClient, aUrl) {
let grip = await getTargetActorForUrl(aClient, aUrl);
let [ response ] = await aClient.attachTarget(grip.actor);
return [grip, response];
let [ response, front ] = await aClient.attachTarget(grip.actor);
return [grip, response, front];
}
async function attachThreadActorForUrl(aClient, aUrl) {
@ -1109,14 +1109,9 @@ function attachWorker(tabClient, worker) {
return tabClient.attachWorker(worker.actor);
}
function waitForWorkerListChanged(tabClient) {
function waitForWorkerListChanged(targetFront) {
info("Waiting for worker list to change.");
return new Promise(function (resolve) {
tabClient.addListener("workerListChanged", function listener() {
tabClient.removeListener("workerListChanged", listener);
resolve();
});
});
return targetFront.once("workerListChanged");
}
function attachThread(workerClient, options) {

View File

@ -194,6 +194,7 @@ const TargetFactory = exports.TargetFactory = {
function TabTarget({ form, client, chrome, tab = null }) {
EventEmitter.decorate(this);
this.destroy = this.destroy.bind(this);
this._onTabNavigated = this._onTabNavigated.bind(this);
this.activeTab = this.activeConsole = null;
this._form = form;
@ -518,6 +519,12 @@ TabTarget.prototype = {
const [response, tabClient] = await this._client.attachTarget(this._form.actor);
this.activeTab = tabClient;
this.threadActor = response.threadActor;
this.activeTab.on("tabNavigated", this._onTabNavigated);
this._onFrameUpdate = packet => {
this.emit("frame-update", packet);
};
this.activeTab.on("frameUpdate", this._onFrameUpdate);
};
// Attach the console actor
@ -549,8 +556,6 @@ TabTarget.prototype = {
this._title = form.title;
}
this._setupRemoteListeners();
// AddonActor and chrome debugging on RootActor don't inherit from
// BrowsingContextTargetActor (i.e. this.isBrowsingContext=false) and don't need
// to be attached.
@ -558,6 +563,10 @@ TabTarget.prototype = {
await attachTarget();
}
// _setupRemoteListeners has to be called after the potential call to `attachTarget`
// as it depends on `activeTab` which is set by this method.
this._setupRemoteListeners();
// But all target actor have a console actor to attach
return attachConsole();
})();
@ -585,70 +594,95 @@ TabTarget.prototype = {
this._tab.removeEventListener("TabRemotenessChange", this);
},
/**
* Event listener for tabNavigated packet sent by activeTab's front.
*/
_onTabNavigated: function(packet) {
const event = Object.create(null);
event.url = packet.url;
event.title = packet.title;
event.nativeConsoleAPI = packet.nativeConsoleAPI;
event.isFrameSwitching = packet.isFrameSwitching;
// Keep the title unmodified when a developer toolbox switches frame
// for a tab (Bug 1261687), but always update the title when the target
// is a WebExtension (where the addon name is always included in the title
// and the url is supposed to be updated every time the selected frame changes).
if (!packet.isFrameSwitching || this.isWebExtension) {
this._url = packet.url;
this._title = packet.title;
}
// Send any stored event payload (DOMWindow or nsIRequest) for backwards
// compatibility with non-remotable tools.
if (packet.state == "start") {
event._navPayload = this._navRequest;
this.emit("will-navigate", event);
this._navRequest = null;
} else {
event._navPayload = this._navWindow;
this.emit("navigate", event);
this._navWindow = null;
}
},
/**
* Setup listeners for remote debugging, updating existing ones as necessary.
*/
_setupRemoteListeners: function() {
this.client.addListener("closed", this.destroy);
this._onTabDetached = (type, packet) => {
// We have to filter message to ensure that this detach is for this tab
if (packet.from == this._form.actor) {
this.destroy();
}
};
this.client.addListener("tabDetached", this._onTabDetached);
// For now, only browsing-context inherited actors are using a front,
// for which events have to be listened on the front itself.
// For other actors (ContentProcessTargetActor and AddonTargetActor), events should
// still be listened directly on the client. This should be ultimately cleaned up to
// only listen from a front by bug 1465635.
if (this.activeTab) {
this.activeTab.on("tabDetached", this.destroy);
this._onTabNavigated = (type, packet) => {
const event = Object.create(null);
event.url = packet.url;
event.title = packet.title;
event.nativeConsoleAPI = packet.nativeConsoleAPI;
event.isFrameSwitching = packet.isFrameSwitching;
// These events should be ultimately listened from the thread client as
// they are coming from it and no longer go through the Target Actor/Front.
this._onSourceUpdated = packet => this.emit("source-updated", packet);
this.activeTab.on("newSource", this._onSourceUpdated);
this.activeTab.on("updatedSource", this._onSourceUpdated);
} else {
this._onTabDetached = (type, packet) => {
// We have to filter message to ensure that this detach is for this tab
if (packet.from == this._form.actor) {
this.destroy();
}
};
this.client.addListener("tabDetached", this._onTabDetached);
// Keep the title unmodified when a developer toolbox switches frame
// for a tab (Bug 1261687), but always update the title when the target
// is a WebExtension (where the addon name is always included in the title
// and the url is supposed to be updated every time the selected frame changes).
if (!packet.isFrameSwitching || this.isWebExtension) {
this._url = packet.url;
this._title = packet.title;
}
// Send any stored event payload (DOMWindow or nsIRequest) for backwards
// compatibility with non-remotable tools.
if (packet.state == "start") {
event._navPayload = this._navRequest;
this.emit("will-navigate", event);
this._navRequest = null;
} else {
event._navPayload = this._navWindow;
this.emit("navigate", event);
this._navWindow = null;
}
};
this.client.addListener("tabNavigated", this._onTabNavigated);
this._onFrameUpdate = (type, packet) => {
this.emit("frame-update", packet);
};
this.client.addListener("frameUpdate", this._onFrameUpdate);
this._onSourceUpdated = (event, packet) => this.emit("source-updated", packet);
this.client.addListener("newSource", this._onSourceUpdated);
this.client.addListener("updatedSource", this._onSourceUpdated);
this._onSourceUpdated = (type, packet) => this.emit("source-updated", packet);
this.client.addListener("newSource", this._onSourceUpdated);
this.client.addListener("updatedSource", this._onSourceUpdated);
}
},
/**
* Teardown listeners for remote debugging.
*/
_teardownRemoteListeners: function() {
// Remove listeners set in _setupRemoteListeners
this.client.removeListener("closed", this.destroy);
this.client.removeListener("tabNavigated", this._onTabNavigated);
this.client.removeListener("tabDetached", this._onTabDetached);
this.client.removeListener("frameUpdate", this._onFrameUpdate);
this.client.removeListener("newSource", this._onSourceUpdated);
this.client.removeListener("updatedSource", this._onSourceUpdated);
if (this.activeTab) {
this.activeTab.off("tabDetached", this.destroy);
this.activeTab.off("newSource", this._onSourceUpdated);
this.activeTab.off("updatedSource", this._onSourceUpdated);
} else {
this.client.removeListener("tabDetached", this._onTabDetached);
this.client.removeListener("newSource", this._onSourceUpdated);
this.client.removeListener("updatedSource", this._onSourceUpdated);
}
// Remove listeners set in attachTarget
if (this.activeTab) {
this.activeTab.off("tabNavigated", this._onTabNavigated);
this.activeTab.off("frameUpdate", this._onFrameUpdate);
}
// Remove listeners set in attachConsole
if (this.activeConsole && this._onInspectObject) {
this.activeConsole.off("inspectObject", this._onInspectObject);
}

View File

@ -37,7 +37,7 @@ add_task(async function() {
const toolbox = await onToolboxReady;
const onToolboxDestroyed = gDevTools.once("toolbox-destroyed");
const onTabDetached = once(toolbox.target.client, "tabDetached");
const onTabDetached = toolbox.target.activeTab.once("tabDetached");
info("Removing the iframes");
toolboxIframe.remove();

View File

@ -39,10 +39,10 @@ function test() {
});
}
function testNavigate([aGrip, aResponse]) {
function testNavigate(targetFront) {
const outstanding = [promise.defer(), promise.defer()];
gClient.addListener("tabNavigated", function onTabNavigated(event, packet) {
targetFront.on("tabNavigated", function onTabNavigated(packet) {
is(packet.url.split("/").pop(), TAB2_FILE,
"Got a tab navigation notification.");
@ -54,27 +54,27 @@ function testNavigate([aGrip, aResponse]) {
outstanding[0].resolve();
} else {
ok(true, "Tab finished navigating.");
gClient.removeListener("tabNavigated", onTabNavigated);
targetFront.off("tabNavigated", onTabNavigated);
outstanding[1].resolve();
}
});
BrowserTestUtils.loadURI(gBrowser.selectedBrowser, TAB2_URL);
return promise.all(outstanding.map(e => e.promise))
.then(() => aGrip.actor);
.then(() => targetFront);
}
function testDetach(actor) {
const deferred = promise.defer();
gClient.addOneTimeListener("tabDetached", (type, packet) => {
ok(true, "Got a tab detach notification.");
is(packet.from, actor, "tab detach message comes from the expected actor");
deferred.resolve(gClient.close());
});
async function testDetach(targetFront) {
const onDetached = targetFront.once("tabDetached");
removeTab(gBrowser.selectedTab);
return deferred.promise;
const packet = await onDetached;
ok(true, "Got a tab detach notification.");
is(packet.from, targetFront.actorID,
"tab detach message comes from the expected actor");
return gClient.close();
}
registerCleanupFunction(function() {
@ -83,8 +83,8 @@ registerCleanupFunction(function() {
async function attachTargetActorForUrl(client, url) {
const grip = await getTargetActorForUrl(client, url);
const [ response ] = await client.attachTarget(grip.actor);
return [grip, response];
const [, targetFront] = await client.attachTarget(grip.actor);
return targetFront;
}
function getTargetActorForUrl(client, url) {

View File

@ -61,15 +61,11 @@ TabStore.prototype = {
// Watch for changes to remote browser tabs
this._connection.client.addListener("tabListChanged",
this._onTabListChanged);
this._connection.client.addListener("tabNavigated",
this._onTabNavigated);
this.listTabs();
} else {
if (this._connection.client) {
this._connection.client.removeListener("tabListChanged",
this._onTabListChanged);
this._connection.client.removeListener("tabNavigated",
this._onTabNavigated);
}
this._resetStore();
}

View File

@ -21,8 +21,6 @@ function start() {
// Start the client.
client = new DebuggerClient(transport);
// Attach listeners for client events.
client.addListener("tabNavigated", onTab);
client.connect((type, traits) => {
// Now the client is conected to the server.
debugTab();
@ -51,9 +49,6 @@ async function startClient() {
// Start the client.
client = new DebuggerClient(transport);
// Attach listeners for client events.
client.addListener("tabNavigated", onTab);
client.connect((type, traits) => {
// Now the client is conected to the server.
debugTab();
@ -89,6 +84,9 @@ function attachToTab() {
}
// Now the tabClient is ready and can be used.
// Attach listeners for client events.
tabClient.addListener("tabNavigated", onTab);
});
});
}
@ -105,7 +103,7 @@ async function onTab() {
// Detach from the previous thread.
await client.activeThread.detach();
// Detach from the previous tab.
await client.activeTab.detach();
await tabClient.activeTab.detach();
// Start debugging the new tab.
start();
}
@ -169,8 +167,6 @@ function startDebugger() {
// Start the client.
client = new DebuggerClient(transport);
// Attach listeners for client events.
client.addListener("tabNavigated", onTab);
client.connect((type, traits) => {
// Now the client is conected to the server.
debugTab();

View File

@ -102,12 +102,12 @@ async function connectAndAttachTab() {
// Connect to this tab
const transport = DebuggerServer.connectPipe();
const client = new DebuggerClient(transport);
client.addListener("tabNavigated", function(event, packet) {
assertEvent("tabNavigated", packet);
});
const form = await connectDebuggerClient(client);
const actorID = form.actor;
await client.attachTarget(actorID);
const [, targetFront ] = await client.attachTarget(actorID);
targetFront.on("tabNavigated", function(packet) {
assertEvent("tabNavigated", packet);
});
return { client, actorID };
}

View File

@ -26,7 +26,7 @@ async function setup(pageUrl) {
const { client, form } = target;
const [, tabClient] = await client.attachTarget(form.actor);
const [, targetFront] = await client.attachTarget(form.actor);
const [, consoleClient] = await client.attachConsole(form.consoleActor, []);
@ -34,7 +34,7 @@ async function setup(pageUrl) {
return {
client, form,
tabClient, consoleClient,
targetFront, consoleClient,
inspectedWindowFront,
extension, fakeExtCallerInfo,
};
@ -47,11 +47,11 @@ async function teardown({client, extension}) {
await extension.unload();
}
function waitForNextTabNavigated(client) {
function waitForNextTabNavigated(targetFront) {
return new Promise(resolve => {
client.addListener("tabNavigated", function tabNavigatedListener(evt, pkt) {
targetFront.on("tabNavigated", function tabNavigatedListener(pkt) {
if (pkt.state == "stop" && !pkt.isFrameSwitching) {
client.removeListener("tabNavigated", tabNavigatedListener);
targetFront.off("tabNavigated", tabNavigatedListener);
resolve();
}
});
@ -227,12 +227,12 @@ add_task(async function test_exception_inspectedWindowEval_result() {
add_task(async function test_exception_inspectedWindowReload() {
const {
client, consoleClient, inspectedWindowFront,
extension, fakeExtCallerInfo,
extension, fakeExtCallerInfo, targetFront,
} = await setup(`${TEST_RELOAD_URL}?test=cache`);
// Test reload with bypassCache=false.
const waitForNoBypassCacheReload = waitForNextTabNavigated(client);
const waitForNoBypassCacheReload = waitForNextTabNavigated(targetFront);
const reloadResult = await inspectedWindowFront.reload(fakeExtCallerInfo,
{ignoreCache: false});
@ -248,7 +248,7 @@ add_task(async function test_exception_inspectedWindowReload() {
// Test reload with bypassCache=true.
const waitForForceBypassCacheReload = waitForNextTabNavigated(client);
const waitForForceBypassCacheReload = waitForNextTabNavigated(targetFront);
await inspectedWindowFront.reload(fakeExtCallerInfo, {ignoreCache: true});
await waitForForceBypassCacheReload;
@ -265,12 +265,12 @@ add_task(async function test_exception_inspectedWindowReload() {
add_task(async function test_exception_inspectedWindowReload_customUserAgent() {
const {
client, consoleClient, inspectedWindowFront,
extension, fakeExtCallerInfo,
extension, fakeExtCallerInfo, targetFront,
} = await setup(`${TEST_RELOAD_URL}?test=user-agent`);
// Test reload with custom userAgent.
const waitForCustomUserAgentReload = waitForNextTabNavigated(client);
const waitForCustomUserAgentReload = waitForNextTabNavigated(targetFront);
await inspectedWindowFront.reload(fakeExtCallerInfo,
{userAgent: "Customized User Agent"});
@ -284,7 +284,7 @@ add_task(async function test_exception_inspectedWindowReload_customUserAgent() {
// Test reload with no custom userAgent.
const waitForNoCustomUserAgentReload = waitForNextTabNavigated(client);
const waitForNoCustomUserAgentReload = waitForNextTabNavigated(targetFront);
await inspectedWindowFront.reload(fakeExtCallerInfo, {});
await waitForNoCustomUserAgentReload;
@ -301,12 +301,12 @@ add_task(async function test_exception_inspectedWindowReload_customUserAgent() {
add_task(async function test_exception_inspectedWindowReload_injectedScript() {
const {
client, consoleClient, inspectedWindowFront,
extension, fakeExtCallerInfo,
extension, fakeExtCallerInfo, targetFront,
} = await setup(`${TEST_RELOAD_URL}?test=injected-script&frames=3`);
// Test reload with an injectedScript.
const waitForInjectedScriptReload = waitForNextTabNavigated(client);
const waitForInjectedScriptReload = waitForNextTabNavigated(targetFront);
await inspectedWindowFront.reload(fakeExtCallerInfo,
{injectedScript: `new ${injectedScript}`});
await waitForInjectedScriptReload;
@ -321,7 +321,7 @@ add_task(async function test_exception_inspectedWindowReload_injectedScript() {
// Test reload without an injectedScript.
const waitForNoInjectedScriptReload = waitForNextTabNavigated(client);
const waitForNoInjectedScriptReload = waitForNextTabNavigated(targetFront);
await inspectedWindowFront.reload(fakeExtCallerInfo, {});
await waitForNoInjectedScriptReload;
@ -339,13 +339,13 @@ add_task(async function test_exception_inspectedWindowReload_injectedScript() {
add_task(async function test_exception_inspectedWindowReload_multiple_calls() {
const {
client, consoleClient, inspectedWindowFront,
extension, fakeExtCallerInfo,
extension, fakeExtCallerInfo, targetFront,
} = await setup(`${TEST_RELOAD_URL}?test=user-agent`);
// Test reload with custom userAgent three times (and then
// check that only the first one has affected the page reload.
const waitForCustomUserAgentReload = waitForNextTabNavigated(client);
const waitForCustomUserAgentReload = waitForNextTabNavigated(targetFront);
inspectedWindowFront.reload(fakeExtCallerInfo, {userAgent: "Customized User Agent 1"});
inspectedWindowFront.reload(fakeExtCallerInfo, {userAgent: "Customized User Agent 2"});
@ -360,7 +360,7 @@ add_task(async function test_exception_inspectedWindowReload_multiple_calls() {
// Test reload with no custom userAgent.
const waitForNoCustomUserAgentReload = waitForNextTabNavigated(client);
const waitForNoCustomUserAgentReload = waitForNextTabNavigated(targetFront);
await inspectedWindowFront.reload(fakeExtCallerInfo, {});
await waitForNoCustomUserAgentReload;
@ -377,12 +377,12 @@ add_task(async function test_exception_inspectedWindowReload_multiple_calls() {
add_task(async function test_exception_inspectedWindowReload_stopped() {
const {
client, consoleClient, inspectedWindowFront,
extension, fakeExtCallerInfo,
extension, fakeExtCallerInfo, targetFront,
} = await setup(`${TEST_RELOAD_URL}?test=injected-script&frames=3`);
// Test reload on a page that calls window.stop() immediately during the page loading
const waitForPageLoad = waitForNextTabNavigated(client);
const waitForPageLoad = waitForNextTabNavigated(targetFront);
await inspectedWindowFront.eval(fakeExtCallerInfo,
"window.location += '&stop=windowStop'");
@ -390,7 +390,7 @@ add_task(async function test_exception_inspectedWindowReload_stopped() {
await waitForPageLoad;
info("Starting a reload with an injectedScript");
const waitForInjectedScriptReload = waitForNextTabNavigated(client);
const waitForInjectedScriptReload = waitForNextTabNavigated(targetFront);
await inspectedWindowFront.reload(fakeExtCallerInfo,
{injectedScript: `new ${injectedScript}`});
await waitForInjectedScriptReload;
@ -408,7 +408,7 @@ add_task(async function test_exception_inspectedWindowReload_stopped() {
// Reload again with no options.
info("Reload the tab again without any reload options");
const waitForNoInjectedScriptReload = waitForNextTabNavigated(client);
const waitForNoInjectedScriptReload = waitForNextTabNavigated(targetFront);
await inspectedWindowFront.reload(fakeExtCallerInfo, {});
await waitForNoInjectedScriptReload;

View File

@ -51,19 +51,19 @@ function setWebExtensionOOPMode(oopMode) {
});
}
function waitForFramesUpdated({client}, matchFn) {
function waitForFramesUpdated(target, matchFn) {
return new Promise(resolve => {
const listener = (evt, data) => {
const listener = data => {
if (typeof matchFn === "function" && !matchFn(data)) {
return;
} else if (!data.frames) {
return;
}
client.removeListener("frameUpdate", listener);
target.activeTab.off("frameUpdate", listener);
resolve(data.frames);
};
client.addListener("frameUpdate", listener);
target.activeTab.on("frameUpdate", listener);
});
}

View File

@ -44,7 +44,7 @@ function test_simple_source_map() {
expectedSources.delete(packet.source.url);
if (expectedSources.size === 0) {
gClient.removeListener("newSource", _onNewSource);
gThreadClient.removeListener("newSource", _onNewSource);
finishClient(gClient);
}
});