Consider allowing profiling in private browsing sessions
Categories
(DevTools :: Performance Tools (Profiler/Timeline), enhancement, P3)
Tracking
(firefox98 fixed)
Tracking | Status | |
---|---|---|
firefox98 | --- | fixed |
People
(Reporter: Harald, Assigned: julienw)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-needed)
Attachments
(12 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
Private browsing is a convenient way for profiling as it resets cache/cookies and cuts out addons impact.
The reasoning that PB disables profiling historically is preventing the accidental profiling of private browsing sessions.
I would argue that the obvious risk isn't recording and viewing the profiles in the Profiler – but sharing the data.
With that in mind, more fine-grained mitigation would be to warn/confirm with users when uploading a profile recorded in PB. Ongoing work on single-tab recording combined with the existing sanitization (removal of hidden data) the risk is further mitigated for general audience.
Comment 2•4 years ago
|
||
(In reply to :Harald Kirschner :digitarald from comment #0)
The reasoning that PB disables profiling historically is preventing the accidental profiling of private browsing sessions.
I would argue that the obvious risk isn't recording and viewing the profiles in the Profiler – but sharing the data.neral audience.
My impression is that the biggest risk is starting the profiler from a normal window when a Private browsing window that should have remained private was still in the background. I don't think starting the profiler from a private browsing window should be a problem, as long as the user is intentionally doing it.
Comment 3•3 years ago
|
||
Considering that the old performance panel works on Private Browsing sessions, and that web developers tend to use this regularly for development, we might want to address this before we enable the new panel by default.
Julien, Honza what do you think?
Assignee | ||
Comment 4•3 years ago
|
||
Yeah I believe we could make it work, but then we should probably record it in the profile metadata and show it in the frontend, especially when the user will share it. I believe there's a small bit of work.
Comment 5•3 years ago
|
||
Some comments / thoughts:
- I agree that we should support profiling of Private Browsing sessions
- The "Share Performance Profile" popup can show a warning when the profile contains data from Private Browsing Sessions
- [optional] The DevTools Performance panel could show the same (visually similar) warning informing the user that there are Private Browsing sessions running in the backround that will be included in profiling.
- Should the default settings in the Performance panel be "Web Developer" as opposed to "Firefox Platform"?
Also, the "Web Developer" settings isn't equal to "single-tab recording", correct? Do we have a bug for "single-tab recording"?
It feels like regular DevTools users are mostly interested in debugging the current tab, which automatically excludes any other running browser sessions/tabs.
Comment 6•3 years ago
|
||
The DevTools Performance panel could show the same (visually similar) warning informing the user that there are Private Browsing sessions running in the backround that will be included in profiling.
So from DevTools' perspective, the preferred option is to allow recording sessions with private browsing windows and just display a warning. We'll need some analysis for what needs to be done outside of the simple UI changes in the DevTools panel. Also need to check with other stakeholders that this is an acceptable change.
Should the default settings in the Performance panel be "Web Developer" as opposed to "Firefox Platform"?
Oh good point, we should file a bug for this!
Comment 7•3 years ago
|
||
(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #5)
Also, the "Web Developer" settings isn't equal to "single-tab recording", correct? Do we have a bug for "single-tab recording"?
It feels like regular DevTools users are mostly interested in debugging the current tab, which automatically excludes any other running browser sessions/tabs.
Redirecting the question to Julien. I think it's correct that WebDeveloper differs from single-tab recording, even though by default the "active tab" should be selected in the profile (currently there is a small issue with this, see https://bugzilla.mozilla.org/show_bug.cgi?id=1704546). This view should be focused on the active tab used when profiling.
Do we plan to go beyond this "active tab" view?
Assignee | ||
Comment 8•3 years ago
•
|
||
Thanks Julian, I was gonna answer this actually.
Indeed we don't plan to do something different for now, we expect that the "active tab" view will work good enough. In the future we'll want a tab switcher in the Profiler UI, I believe that will make it even easier. I also believe we'll want a new sanitization process to remove other tab's information. But nothing that prevents us from releasing !
(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #5)
Some comments / thoughts:
- The "Share Performance Profile" popup can show a warning when the profile contains data from Private Browsing Sessions
Doing that is what I outlined in comment 4. This isn't difficult but there's some work.
- [optional] The DevTools Performance panel could show the same (visually similar) warning informing the user that there are Private Browsing sessions running in the backround that will be included in profiling.
This is a lot easier, so I believe we should do this.
- Should the default settings in the Performance panel be "Web Developer" as opposed to "Firefox Platform"?
The default is "Firefox Platform" only in nightly or local builds, otherwise it's "Web Developer".
see https://searchfox.org/mozilla-central/rev/9dceacf3d761eb91237108ec438d64099a56f442/modules/libpref/init/all.js#850-857
Is that acceptable?
Also, the "Web Developer" settings isn't equal to "single-tab recording", correct? Do we have a bug for "single-tab recording"?
It feels like regular DevTools users are mostly interested in debugging the current tab, which automatically excludes any other running browser sessions/tabs.
(see above)
Comment 9•3 years ago
|
||
The default is "Firefox Platform" only in nightly or local builds, otherwise it's "Web Developer".
Makes sense to me, I think it's fine.
Actually yesterday I tested in DevEdition, where I probably had Web Developer by default. So I was a bit surprised when reading the question :)
Comment 10•3 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #8)
The default is "Firefox Platform" only in nightly or local builds, otherwise it's "Web Developer".
see https://searchfox.org/mozilla-central/rev/9dceacf3d761eb91237108ec438d64099a56f442/modules/libpref/init/all.js#850-857
Is that acceptable?
I agree, this makes sense, yes.
Thank you, Julien
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 13•3 years ago
|
||
Assignee | ||
Comment 14•3 years ago
|
||
Depends on D129416
Assignee | ||
Comment 15•3 years ago
|
||
Depends on D129417
Assignee | ||
Comment 16•3 years ago
|
||
Depends on D129418
Assignee | ||
Comment 17•3 years ago
|
||
Depends on D129419
Assignee | ||
Comment 18•3 years ago
|
||
Depends on D129420
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Comment 19•3 years ago
|
||
Depends on D129421
Assignee | ||
Comment 20•3 years ago
|
||
Depends on D129518
Assignee | ||
Comment 21•3 years ago
|
||
Depends on D129818
Assignee | ||
Comment 22•3 years ago
|
||
Windows Try: https://treeherder.mozilla.org/jobs?repo=try&revision=26374704eb62bdf8999a01af8b540e7246ab120b
Github work (tests still missing): https://github.com/firefox-devtools/profiler/pull/3671
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Comment 23•3 years ago
|
||
Depends on D129421
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Comment 24•3 years ago
|
||
Depends on D133375
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Comment 25•3 years ago
|
||
Depends on D129819
Assignee | ||
Comment 26•3 years ago
|
||
Final try before landing: https://treeherder.mozilla.org/jobs?repo=try&revision=589f37548072c3a2d92e1c3a5878cb5544125960
Comment 27•3 years ago
|
||
Comment 28•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d7b679fcef1f
https://hg.mozilla.org/mozilla-central/rev/b6301f4ca68c
https://hg.mozilla.org/mozilla-central/rev/53b4be41c530
https://hg.mozilla.org/mozilla-central/rev/9aa662b8da5e
https://hg.mozilla.org/mozilla-central/rev/001820edfacc
https://hg.mozilla.org/mozilla-central/rev/b32fc155af27
https://hg.mozilla.org/mozilla-central/rev/0585facaeb83
https://hg.mozilla.org/mozilla-central/rev/0747f5dd266b
https://hg.mozilla.org/mozilla-central/rev/3ea70d53ef4e
https://hg.mozilla.org/mozilla-central/rev/a2e43e51b6ca
https://hg.mozilla.org/mozilla-central/rev/de1c260f94f5
https://hg.mozilla.org/mozilla-central/rev/a615447107f1
Assignee | ||
Comment 29•3 years ago
|
||
dev-doc-needed for developer-oriented documentation
Description
•