Skip to content

Commit

Permalink
Bug 1809848 - [devtools] Remove TargetMixin.isLocalTab/localTab attri…
Browse files Browse the repository at this point in the history
…butes. r=devtools-reviewers,nchevobbe

They should rather be queried on the descriptor front.

Differential Revision: https://phabricator.services.mozilla.com/D166647
  • Loading branch information
ochameau committed Jan 17, 2023
1 parent ad1bf27 commit a4b4266
Show file tree
Hide file tree
Showing 15 changed files with 44 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ class ParentDevToolsPanel extends BaseDevToolsPanel {
// panelLabel is used to set the aria-label attribute (See Bug 1570645).
panelLabel: title,
tooltip: `DevTools Panel added by "${extensionName}" add-on.`,
isToolSupported: toolbox => toolbox.target.isLocalTab,
isToolSupported: toolbox => toolbox.commands.descriptorFront.isLocalTab,
build: (window, toolbox) => {
if (toolbox !== this.toolbox) {
throw new Error(
Expand Down
6 changes: 4 additions & 2 deletions devtools/client/definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,9 @@ Tools.performance = {
// use the performance panel; about:debugging provides a "Profile performance" button
// which can be used instead, without having the overhead of starting a remote toolbox.
// Also accept the Browser Toolbox, so that we can profile its process via a second browser toolbox.
return toolbox.target.isLocalTab || toolbox.isBrowserToolbox;
return (
toolbox.commands.descriptorFront.isLocalTab || toolbox.isBrowserToolbox
);
},
build(frame, toolbox, commands) {
return new NewPerformancePanel(frame, toolbox, commands);
Expand Down Expand Up @@ -490,7 +492,7 @@ exports.ToolboxButtons = [
"toolbox.buttons.responsive",
osString == "Darwin" ? "Cmd+Opt+M" : "Ctrl+Shift+M"
),
isToolSupported: toolbox => toolbox.target.isLocalTab,
isToolSupported: toolbox => toolbox.commands.descriptorFront.isLocalTab,
onClick(event, toolbox) {
const { localTab } = toolbox.commands.descriptorFront;
const browserWindow = localTab.ownerDocument.defaultView;
Expand Down
12 changes: 11 additions & 1 deletion devtools/client/framework/test/browser_commands_from_url.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ SimpleTest.registerCleanupFunction(() => {

function assertTarget(target, url, chrome = false) {
is(target.url, url);
is(target.isLocalTab, false);
is(target.chrome, chrome);
is(target.isBrowsingContext, true);
}
Expand All @@ -45,7 +44,18 @@ add_task(async function() {
);
// Descriptor's getTarget will only work if the TargetCommand watches for the first top target
await commands.targetCommand.startListening();

// For now, we can't spawn a commands flagged as 'local tab' via URL query params
// The only way to has isLocalTab is to create the toolbox via showToolboxForTab
// and spawn the command via CommandsFactory.forTab.
is(
commands.descriptorFront.isLocalTab,
false,
"Even if we refer to a local tab, isLocalTab is false (for now)"
);

target = await commands.descriptorFront.getTarget();

assertTarget(target, TEST_URI);
await commands.destroy();

Expand Down
19 changes: 14 additions & 5 deletions devtools/client/framework/test/browser_target_events.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,18 @@ add_task(async function() {
gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
await BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);

const target = await createAndAttachTargetForTab(gBrowser.selectedTab);
is(target.localTab, gBrowser.selectedTab, "Target linked to the right tab.");
const commands = await CommandsFactory.forTab(gBrowser.selectedTab);
is(
commands.descriptorFront.localTab,
gBrowser.selectedTab,
"Target linked to the right tab."
);

const willNavigate = once(target, "will-navigate");
const navigate = once(target, "navigate");
await commands.targetCommand.startListening();
const { targetFront } = commands.targetCommand;

const willNavigate = once(targetFront, "will-navigate");
const navigate = once(targetFront, "navigate");
SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
content.location = "data:text/html,<meta charset='utf8'/>test navigation";
});
Expand All @@ -29,8 +36,10 @@ add_task(async function() {
await navigate;
ok(true, "navigate event received");

const onTargetDestroyed = once(target, "target-destroyed");
const onTargetDestroyed = once(targetFront, "target-destroyed");
gBrowser.removeCurrentTab();
await onTargetDestroyed;
ok(true, "target destroyed received");

await commands.destroy();
});
3 changes: 2 additions & 1 deletion devtools/client/framework/toolbox-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,8 @@ OptionsPanel.prototype = {
visibilityswitch: pref,

// Only local tabs are currently supported as targets.
isToolSupported: toolbox => toolbox.target.isLocalTab,
isToolSupported: toolbox =>
toolbox.commands.descriptorFront.isLocalTab,
})
);
}
Expand Down
1 change: 0 additions & 1 deletion devtools/client/fronts/descriptors/tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,6 @@ class TabDescriptorFront extends DescriptorMixin(
this._targetFront.off("target-destroyed", this._onTargetDestroyed);
}
this._targetFront = targetFront;
targetFront.setDescriptor(this);

targetFront.on("target-destroyed", this._onTargetDestroyed);

Expand Down
54 changes: 0 additions & 54 deletions devtools/client/fronts/targets/target-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,42 +120,6 @@ function TargetMixin(parentClass) {
return true;
}

/**
* Get the descriptor front for this target.
*
* TODO: Should be removed. This is misleading as only the top level target should have a descriptor.
* This will return null for targets created by the Watcher actor and will still be defined
* by targets created by RootActor methods (listSomething methods).
*/
get descriptorFront() {
if (this.isDestroyed()) {
throw new Error("Descriptor already destroyed for target: " + this);
}

if (this.isWorkerTarget) {
return this;
}

if (this._descriptorFront) {
return this._descriptorFront;
}

if (this.parentFront.typeName.endsWith("Descriptor")) {
return this.parentFront;
}
throw new Error("Missing descriptor for target: " + this);
}

/**
* Top-level targets created on the server will not be created and managed
* by a descriptor front. Instead they are created by the Watcher actor.
* On the client side we manually re-establish a link between the descriptor
* and the new top-level target.
*/
setDescriptor(descriptorFront) {
this._descriptorFront = descriptorFront;
}

get targetType() {
return this._targetType;
}
Expand Down Expand Up @@ -272,24 +236,6 @@ function TargetMixin(parentClass) {
return this.client.traits[traitName];
}

get isLocalTab() {
// Worker Target is also the Descriptor,
// so avoid infinite loop.
if (this.isWorkerTarget) {
return false;
}
return !!this.descriptorFront?.isLocalTab;
}

get localTab() {
// Worker Target is also the Descriptor,
// so avoid infinite loop.
if (this.isWorkerTarget) {
return null;
}
return this.descriptorFront?.localTab || null;
}

// Get a promise of the RootActor's form
get root() {
return this.client.mainRoot.rootForm;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ async function testToolboxInitialization(tab) {

ok(true, "Inspector started, and notification received.");
ok(inspector, "Inspector instance is accessible.");
is(inspector.currentTarget.localTab, tab, "Valid target.");

await selectNode("p", inspector);
await testMarkupView("p", inspector);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ const TEST_URL =

add_task(async function() {
info("Creating the test tab and opening the rule-view");
let { toolbox, inspector, highlighterTestFront } = await openInspectorForURL(
TEST_URL
);
let {
tab,
toolbox,
inspector,
highlighterTestFront,
} = await openInspectorForURL(TEST_URL);

info("Selecting the ruleview sidebar");
inspector.sidebar.select("ruleview");
Expand Down Expand Up @@ -80,7 +83,6 @@ add_task(async function() {
);

info("Destroying the toolbox");
const tab = toolbox.target.localTab;
await toolbox.destroy();

// As the toolbox get destroyed, we need to fetch a new test-actor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ add_task(async function() {
async function testToolboxInitialization(tab, inspector, toolbox) {
ok(true, "Inspector started, and notification received.");
ok(inspector, "Inspector instance is accessible.");
is(inspector.currentTarget.localTab, tab, "Valid target.");

await selectNode("#p", inspector);
await testMarkupView("#p", inspector);
Expand Down
2 changes: 1 addition & 1 deletion devtools/client/netmonitor/test/head.js
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ function restartNetMonitor(monitor, { requestCount }) {
info("Restarting the specified network monitor.");

return (async function() {
const tab = monitor.toolbox.target.localTab;
const tab = monitor.commands.descriptorFront.localTab;
const url = tab.linkedBrowser.currentURI.spec;

await waitForAllNetworkUpdateEvents();
Expand Down
4 changes: 2 additions & 2 deletions devtools/client/styleeditor/StyleEditorUI.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1446,8 +1446,8 @@ export class StyleEditorUI extends EventEmitter {
* Object with width or/and height properties.
*/
async #launchResponsiveMode(options = {}) {
const tab = this.currentTarget.localTab;
const win = this.currentTarget.localTab.ownerDocument.defaultView;
const tab = this.#commands.descriptorFront.localTab;
const win = tab.ownerDocument.defaultView;

await lazy.ResponsiveUIManager.openIfNeeded(win, tab, {
trigger: "style_editor",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ add_task(async function() {
for (const tab of tabs) {
// Open the console in tab${i}.
const hud = await openConsole(tab);
const browser = hud.currentTarget.localTab.linkedBrowser;
const browser = hud.commands.descriptorFront.localTab.linkedBrowser;
const message = "message for tab " + tabs.indexOf(tab);

// Log a message in the newly opened console.
Expand Down
2 changes: 1 addition & 1 deletion devtools/docs/user/devtoolsapi/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ Register a tool
},
isToolSupported: function(toolbox) {
return toolbox.target.isLocalTab;
return toolbox.commands.descriptorFront.isLocalTab;
},
build: function(iframeWindow, toolbox, node) {
Expand Down
3 changes: 1 addition & 2 deletions toolkit/components/extensions/ExtensionParent.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -881,9 +881,8 @@ class DevToolsExtensionPageContextParent extends ExtensionPageContextParent {
for (const resource of resources) {
const { targetFront } = resource;
if (targetFront.isTopLevel && resource.name === "dom-complete") {
const url = targetFront.localTab.linkedBrowser.currentURI.spec;
for (const listener of this._onNavigatedListeners) {
listener(url);
listener(targetFront.url);
}
}
}
Expand Down

0 comments on commit a4b4266

Please sign in to comment.