Skip to content

Commit

Permalink
Bug 1835081 - Expose the "profilerstacks" option in the about:logging…
Browse files Browse the repository at this point in the history 
… UI r=padenot,fluent-reviewers,desktop-theme-reviewers,sfoster,flod

Differential Revision: https://phabricator.services.mozilla.com/D179114
  • Loading branch information
@julienw
julienw committed May 30, 2023
1 parent cf05c60 commit 6f9afe6
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 47 deletions.
10 changes: 8 additions & 2 deletions toolkit/content/aboutLogging.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ <h2 data-l10n-id="about-logging-log-modules-selection"></h2>
></span>
</div>
<h2 data-l10n-id="about-logging-logging-output-selection"></h2>
<div id="logging-output-profiler" class="radio-entry">
<div id="logging-output-profiler" class="form-entry">
<input
type="radio"
id="radio-logging-profiler"
Expand All @@ -103,7 +103,13 @@ <h2 data-l10n-id="about-logging-logging-output-selection"></h2>
data-l10n-id="about-logging-logging-to-profiler"
></label>
</div>
<div id="logging-output-file" class="radio-entry">
<div id="profiler-configuration" class="form-entry">
<input type="checkbox" id="with-profiler-stacks-checkbox" /><label
for="with-profiler-stacks-checkbox"
data-l10n-id="about-logging-with-profiler-stacks-checkbox"
></label>
</div>
<div id="logging-output-file" class="form-entry">
<input
type="radio"
id="radio-logging-file"
Expand Down
112 changes: 70 additions & 42 deletions toolkit/content/aboutLogging.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,21 +96,27 @@ const gLoggingSettings = {
// If non-null, the threads that will be recorded by the Firefox Profiler. If
// null, the threads from the profiler presets are going to be used.
profilerThreads: null,
// If non-null, stack traces will be recorded for MOZ_LOG profiler markers.
// This is set only when coming from the URL, not when the user changes the UI.
profilerStacks: null,
};

// When the profiler has been started, this holds the promise the
// Services.profiler.StartProfiler returns, to ensure the profiler has
// effectively started.
let gProfilerPromise = null;

// Used in tests
function presets() {
return gLoggingPresets;
}

// Used in tests
function settings() {
return gLoggingSettings;
}

// Used in tests
function profilerPromise() {
return gProfilerPromise;
}
Expand Down Expand Up @@ -157,19 +163,24 @@ function populatePresets() {

function updateLoggingOutputType(profilerOutputType) {
gLoggingSettings.loggingOutputType = profilerOutputType;

if (gLoggingSettings.loggingOutputType === "profiler") {
// hide options related to file output for clarity
$("#log-file-configuration").hidden = true;
} else if (gLoggingSettings.loggingOutputType === "file") {
$("#log-file-configuration").hidden = false;
$("#no-log-file").hidden = !!$("#current-log-file").innerText.length;
Services.prefs.setCharPref("logging.config.output_type", profilerOutputType);
$(`input[type=radio][value=${profilerOutputType}]`).checked = true;

switch (profilerOutputType) {
case "profiler":
if (!gLoggingSettings.profilerStacks) {
// If this value is set from the URL, do not allow to change it.
$("#with-profiler-stacks-checkbox").disabled = false;
}
// hide options related to file output for clarity
$("#log-file-configuration").hidden = true;
break;
case "file":
$("#with-profiler-stacks-checkbox").disabled = true;
$("#log-file-configuration").hidden = false;
$("#no-log-file").hidden = !!$("#current-log-file").innerText.length;
break;
}

Services.prefs.setCharPref(
"logging.config.output_type",
gLoggingSettings.loggingOutputType
);
}

function displayErrorMessage(error) {
Expand Down Expand Up @@ -207,7 +218,8 @@ function parseURL() {
outputTypeOverriden = null,
loggingPresetOverriden = null,
threadsOverriden = null,
profilerPresetOverriden = null;
profilerPresetOverriden = null,
profilerStacksOverriden = null;
try {
for (let [k, v] of options) {
switch (k) {
Expand Down Expand Up @@ -239,6 +251,9 @@ function parseURL() {
}
profilerPresetOverriden = v;
break;
case "profilerstacks":
profilerStacksOverriden = true;
break;
default:
throw new ParseError("about-logging-unknown-option", k, v);
}
Expand Down Expand Up @@ -286,12 +301,18 @@ function parseURL() {
updateLogModules();
}
if (outputTypeOverriden) {
$$("input[type=radio]").forEach(e => {
e.setAttribute("disabled", true);
someElementsDisabled = true;
e.checked = e.value == outputTypeOverriden;
});
$$("input[type=radio]").forEach(e => (e.disabled = true));
someElementsDisabled = true;
updateLoggingOutputType(outputTypeOverriden);
}
if (profilerStacksOverriden) {
const checkbox = $("#with-profiler-stacks-checkbox");
checkbox.disabled = true;
someElementsDisabled = true;
Services.prefs.setBoolPref("logging.config.profilerstacks", true);
gLoggingSettings.profilerStacks = true;
}

if (loggingPresetOverriden) {
gLoggingSettings.loggingPreset = loggingPresetOverriden;
}
Expand Down Expand Up @@ -335,17 +356,27 @@ function init() {
};
});

try {
let loggingOutputType = Services.prefs.getCharPref(
"logging.config.output_type"
$("#with-profiler-stacks-checkbox").addEventListener("change", e => {
Services.prefs.setBoolPref(
"logging.config.profilerstacks",
e.target.checked
);
if (loggingOutputType.length) {
updateLoggingOutputType(loggingOutputType);
}
} catch {
updateLoggingOutputType("profiler");
updateLogModules();
});

let loggingOutputType = Services.prefs.getCharPref(
"logging.config.output_type",
"profiler"
);
if (loggingOutputType.length) {
updateLoggingOutputType(loggingOutputType);
}

$("#with-profiler-stacks-checkbox").checked = Services.prefs.getBoolPref(
"logging.config.profilerstacks",
false
);

try {
let loggingPreset = Services.prefs.getCharPref("logging.config.preset");
gLoggingSettings.loggingPreset = loggingPreset;
Expand Down Expand Up @@ -452,22 +483,6 @@ function updateLogModules() {
setLogModulesButton.disabled = true;
} else {
let activeLogModules = [];
try {
if (Services.prefs.getBoolPref("logging.config.add_timestamp")) {
activeLogModules.push("timestamp");
}
} catch (e) {}
try {
if (Services.prefs.getBoolPref("logging.config.sync")) {
activeLogModules.push("sync");
}
} catch (e) {}
try {
if (Services.prefs.getBoolPref("logging.config.profilerstacks")) {
activeLogModules.push("profilerstacks");
}
} catch (e) {}

let children = Services.prefs.getBranch("logging.").getChildList("");

for (let pref of children) {
Expand All @@ -483,6 +498,19 @@ function updateLogModules() {
}
}

if (activeLogModules.length) {
// Add some options only if some modules are present.
if (Services.prefs.getBoolPref("logging.config.add_timestamp", false)) {
activeLogModules.push("timestamp");
}
if (Services.prefs.getBoolPref("logging.config.sync", false)) {
activeLogModules.push("sync");
}
if (Services.prefs.getBoolPref("logging.config.profilerstacks", false)) {
activeLogModules.push("profilerstacks");
}
}

if (activeLogModules.length !== 0) {
currentLogModules.innerText = activeLogModules.join(",");
currentLogModules.hidden = false;
Expand Down
74 changes: 72 additions & 2 deletions toolkit/content/tests/browser/browser_about_logging.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,32 @@ add_task(async function testURLParameters() {
});
}
);
await BrowserTestUtils.withNewTab(PAGE + "?profilerstacks", async browser => {
await SpecialPowers.spawn(browser, [], async () => {
let $ = content.document.querySelector.bind(content.document);
Assert.ok(
!$("#some-elements-unavailable").hidden,
"If the profiler stacks config is set via URL, a warning should be displayed."
);
Assert.ok(
$("#with-profiler-stacks-checkbox").disabled,
"If the profiler stacks config is set via URL, its checkbox should be disabled."
);

Assert.ok(
Services.prefs.getBoolPref("logging.config.profilerstacks"),
"The preference for profiler stacks is set initially, as a result of parsing the URL parameter"
);

$("#radio-logging-file").click();
$("#radio-logging-profiler").click();

Assert.ok(
$("#with-profiler-stacks-checkbox").disabled,
"If the profiler stacks config is set via URL, its checkbox should be disabled even after clicking around."
);
});
});
await BrowserTestUtils.withNewTab(
{
gBrowser,
Expand Down Expand Up @@ -296,8 +322,51 @@ add_task(async function testAboutLoggingPresets() {
clearLoggingPrefs();
});

// // Here we test that starting and stopping log collection to the Firefox
// // Profiler opens a new tab. We don't actually check the content of the profile.
// Test various things around the profiler stacks feature
add_task(async function testProfilerStacks() {
// Check the initial state before changing anything.
Assert.ok(
!Services.prefs.getBoolPref("logging.config.profilerstacks", false),
"The preference for profiler stacks isn't set initially"
);
await BrowserTestUtils.withNewTab(PAGE, async browser => {
await SpecialPowers.spawn(browser, [], async () => {
let $ = content.document.querySelector.bind(content.document);
const checkbox = $("#with-profiler-stacks-checkbox");
Assert.ok(
!checkbox.checked,
"The profiler stacks checkbox isn't checked at load time."
);
checkbox.checked = true;
checkbox.dispatchEvent(new content.Event("change"));
Assert.ok(
Services.prefs.getBoolPref("logging.config.profilerstacks"),
"The preference for profiler stacks is now set to true"
);
checkbox.checked = false;
checkbox.dispatchEvent(new content.Event("change"));
Assert.ok(
!Services.prefs.getBoolPref("logging.config.profilerstacks"),
"The preference for profiler stacks is now back to false"
);

$("#radio-logging-file").click();
Assert.ok(
checkbox.disabled,
"The profiler stacks checkbox is disabled when the output type is 'file'"
);
$("#radio-logging-profiler").click();
Assert.ok(
!checkbox.disabled,
"The profiler stacks checkbox is enabled when the output type is 'profiler'"
);
});
});
clearLoggingPrefs();
});

// Here we test that starting and stopping log collection to the Firefox
// Profiler opens a new tab. We don't actually check the content of the profile.
add_task(async function testProfilerOpens() {
await BrowserTestUtils.withNewTab(PAGE, async browser => {
let profilerOpenedPromise = BrowserTestUtils.waitForNewTab(
Expand Down Expand Up @@ -391,4 +460,5 @@ add_task(async function testLogFileFound() {
}
Assert.ok(foundNonEmptyLogFile, "Found at least one non-empty log file.");
});
clearLoggingPrefs();
});
1 change: 1 addition & 0 deletions toolkit/locales/en-US/toolkit/about/aboutLogging.ftl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ about-logging-logging-to-profiler = Logging to the { -profiler-brand-name }
about-logging-no-log-modules = None
about-logging-no-log-file = None
about-logging-logging-preset-selector-text = Logging preset:
about-logging-with-profiler-stacks-checkbox = Enable stack traces for log messages
## Logging presets

Expand Down
11 changes: 10 additions & 1 deletion toolkit/themes/shared/aboutLogging.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,17 @@
margin-bottom: 2em;
}

.radio-entry {
.form-entry {
/* Center the labels with their checkboxes */
display: flex;
align-items: center;
margin: 0.3em 0;
}

:disabled + label {
opacity: 0.5;
}

#current-log-modules,
#no-log-modules {
font-family: monospace;
Expand All @@ -33,12 +37,17 @@
font-family: monospace;
}

#profiler-configuration,
#log-file-configuration {
/* 16px is the size of the radio button, 6px is its margin
* Then it's properly aligned with the text above. */
padding-inline-start: calc(16px + 6px);
}

label {
line-height: 1.8em;
}

input[type=text] {
box-sizing: border-box;
width: 100%;
Expand Down

0 comments on commit 6f9afe6

Please sign in to comment.