Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-16 14:55:47 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1485676 - Tweak RDM manage to support new async forTab. r=yulia

Browse Source 
Summary:
Fetching any target is now asynchronous. But RDM setup/destroy codepath is very fragile
and introduce many low level exception when trying to restore the original browser element
if any timing changes.
So this patch prevents trying to fetch the target object if a toolbox isn't already opened.
The target object is being used only for Telemetry purpose for now.

Depends On D4538

Reviewers: yulia!

Tags: #secure-revision

Bug #: 1485676

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

MozReview-Commit-ID: 2QDUNqentMP
This commit is contained in:
Alexandre Poirot 2018-08-29 06:11:53 -07:00
parent 7b7f2b7ebd
commit 57c47d0e7a
2 changed files with 62 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

102
devtools/client/responsive.html/manager.js
View File

@ -87,36 +87,18 @@ const ResponsiveUIManager = exports.ResponsiveUIManager = {
*/
async openIfNeeded(window, tab, options = {}) {
if (!tab.linkedBrowser.isRemoteBrowser) {
this.showRemoteOnlyNotification(window, tab, options);
await this.showRemoteOnlyNotification(window, tab, options);
return promise.reject(new Error("RDM only available for remote tabs."));
}
if (!this.isActiveForTab(tab)) {
this.initMenuCheckListenerFor(window);
// Track whether a toolbox was opened before RDM was opened.
const toolbox = gDevTools.getToolbox(TargetFactory.forTab(tab));
const hostType = toolbox ? toolbox.hostType : "none";
const hasToolbox = !!toolbox;
const tel = this._telemetry;
if (hasToolbox) {
tel.scalarAdd("devtools.responsive.toolbox_opened_first", 1);
}
tel.recordEvent("devtools.main", "activate", "responsive_design", null, {
"host": hostType,
"width": Math.ceil(window.outerWidth / 50) * 50,
"session_id": toolbox ? toolbox.sessionId : -1
});
// Track opens keyed by the UI entry point used.
let { trigger } = options;
if (!trigger) {
trigger = "unknown";
}
tel.keyedScalarAdd("devtools.responsive.open_trigger", trigger, 1);
const ui = new ResponsiveUI(window, tab);
this.activeTabs.set(tab, ui);
// Explicitly not await on telemetry to avoid delaying RDM opening
this.recordTelemetryOpen(window, tab, options);
await this.setMenuCheckFor(tab, window);
await ui.inited;
this.emit("on", { tab });
@ -125,6 +107,38 @@ const ResponsiveUIManager = exports.ResponsiveUIManager = {
return this.getResponsiveUIForTab(tab);
},
/**
* Record all telemetry probes related to RDM opening.
*/
async recordTelemetryOpen(window, tab, options) {
// Track whether a toolbox was opened before RDM was opened.
const isKnownTab = TargetFactory.isKnownTab(tab);
let toolbox;
if (isKnownTab) {
const target = await TargetFactory.forTab(tab);
toolbox = gDevTools.getToolbox(target);
}
const hostType = toolbox ? toolbox.hostType : "none";
const hasToolbox = !!toolbox;
const tel = this._telemetry;
if (hasToolbox) {
tel.scalarAdd("devtools.responsive.toolbox_opened_first", 1);
}
tel.recordEvent("devtools.main", "activate", "responsive_design", null, {
"host": hostType,
"width": Math.ceil(window.outerWidth / 50) * 50,
"session_id": toolbox ? toolbox.sessionId : -1
});
// Track opens keyed by the UI entry point used.
let { trigger } = options;
if (!trigger) {
trigger = "unknown";
}
tel.keyedScalarAdd("devtools.responsive.open_trigger", trigger, 1);
},
/**
* Closes the responsive UI, if not already closed.
*
@ -144,15 +158,6 @@ const ResponsiveUIManager = exports.ResponsiveUIManager = {
*/
async closeIfNeeded(window, tab, options = {}) {
if (this.isActiveForTab(tab)) {
const isKnownTab = TargetFactory.isKnownTab(tab);
const target = TargetFactory.forTab(tab);
const toolbox = gDevTools.getToolbox(target);
if (!toolbox && !isKnownTab) {
// Destroy the tabTarget to avoid a memory leak.
target.destroy();
}
const ui = this.activeTabs.get(tab);
const destroyed = await ui.destroy(options);
if (!destroyed) {
@ -160,14 +165,6 @@ const ResponsiveUIManager = exports.ResponsiveUIManager = {
return;
}
const hostType = toolbox ? toolbox.hostType : "none";
const t = this._telemetry;
t.recordEvent("devtools.main", "deactivate", "responsive_design", null, {
"host": hostType,
"width": Math.ceil(window.outerWidth / 50) * 50,
"session_id": toolbox ? toolbox.sessionId : -1
});
this.activeTabs.delete(tab);
if (!this.isActiveForWindow(window)) {
@ -175,9 +172,29 @@ const ResponsiveUIManager = exports.ResponsiveUIManager = {
}
this.emit("off", { tab });
await this.setMenuCheckFor(tab, window);
// Explicitly not await on telemetry to avoid delaying RDM closing
this.recordTelemetryClose(window, tab);
}
},
async recordTelemetryClose(window, tab) {
const isKnownTab = TargetFactory.isKnownTab(tab);
let toolbox;
if (isKnownTab) {
const target = await TargetFactory.forTab(tab);
toolbox = gDevTools.getToolbox(target);
}
const hostType = toolbox ? toolbox.hostType : "none";
const t = this._telemetry;
t.recordEvent("devtools.main", "deactivate", "responsive_design", null, {
"host": hostType,
"width": Math.ceil(window.outerWidth / 50) * 50,
"session_id": toolbox ? toolbox.sessionId : -1
});
},
/**
* Returns true if responsive UI is active for a given tab.
*
@ -238,7 +255,7 @@ const ResponsiveUIManager = exports.ResponsiveUIManager = {
},
showRemoteOnlyNotification(window, tab, { trigger } = {}) {
showNotification(window, tab, {
return showNotification(window, tab, {
toolboxButton: trigger == "toolbox",
msg: l10n.getStr("responsive.remoteOnly"),
priority: PriorityLevels.PRIORITY_CRITICAL_MEDIUM,
@ -374,7 +391,8 @@ ResponsiveUI.prototype = {
// If our tab is about to be closed, there's not enough time to exit
// gracefully, but that shouldn't be a problem since the tab will go away.
// So, skip any waiting when we're about to close the tab.
const isWindowClosing = options && options.reason === "unload";
const isTabDestroyed = !this.tab.linkedBrowser;
const isWindowClosing = (options && options.reason === "unload") || isTabDestroyed;
const isTabContentDestroying =
isWindowClosing || (options && (options.reason === "TabClose" ||
options.reason === "BeforeTabRemotenessChange"));

4
devtools/client/responsive.html/utils/notification.js
View File

@ -22,7 +22,7 @@ loader.lazyRequireGetter(this, "gDevTools", "devtools/client/framework/devtools"
* - `priority`: Priority level for the notification, which affects the icon and
* overall appearance.
*/
function showNotification(window, tab, { toolboxButton, msg, priority } = {}) {
async function showNotification(window, tab, { toolboxButton, msg, priority } = {}) {
// Default to using the browser's per-tab notification box
let nbox = window.gBrowser.getNotificationBox(tab.linkedBrowser);
@ -30,7 +30,7 @@ function showNotification(window, tab, { toolboxButton, msg, priority } = {}) {
// toolbox for the tab. If one exists, use the toolbox's notification box so that the
// message is placed closer to the action taken by the user.
if (toolboxButton) {
const target = TargetFactory.forTab(tab);
const target = await TargetFactory.forTab(tab);
const toolbox = gDevTools.getToolbox(target);
if (toolbox) {
nbox = toolbox.notificationBox;