899064 - Profiler won't turn on because it thinks I'm in private browsing mode

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[:Gijs (he/him)]

STR:

1) Open Firefox with the gecko profiler installed
2) (Open a new tab / the default homepage)

AR:
The button gets disabled

ER:
The button does not get disabled.


Workaround: open and close a private window.

This is probably because the thumbnail service thingummywhatsit uses the private context that we're trying to detect here:

http://mxr.mozilla.org/mozilla-central/source/tools/profiler/nsProfiler.cpp#73

Comment 1

[Benoit Girard (:BenWa)]

Is it a recent change of the thumbnail service? I know I'm not using 'last-pb-context-exited' as it's intended but I wonder if other add-ons will also misbehave.

Comment 2

[(no longer active)]

Tom, are we supposed to get a chrome-document-gloabl-created for an OOP <xul:browser remote=true>?

Comment 3

[Tom S [:evilpie]]

I think this notification is fired in content and not send to the parent. We couldn't really use the domwindow pointer anyway.

Comment 4

[Mark Hammond [:markh] [:mhammond]]

It's worth noting that the "background" thumbnail service, which is used indirectly by about:newtab, does indeed use private-browsing - so it would be expected that private browsing would be considered active for some time after opening about:newtab (and FTR, only if any of the thumbnails displayed is > 2 days old).  I'm marking this as depending on bug 875986, which calls for us to avoid using PB mode.

The thumbnail service actually keeps the remote browser alive for 60 seconds after the last queued capture completes.  Given the interaction with PB mode, we've filed bug 899403 to reduce this timeout to zero - meaning PB mode should only be active while the thumbnails are actively being captured.  Not bothering to depend on that bug as it will not help solve it, just reduce the impact.

Given comment 2 and 3, it sounds like there is also an issue with chrome-document-global-created which we need to track.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

You can work around this issue locally by setting browser.pageThumbs.enabled=false, FWIW.

Comment 6

[(no longer active)]

I really really wished we just disabled the profile upload feature if PB was activated instead of breaking the profiler altogether.

Comment 7

[(no longer active)]

Actually, what happens when you have a private window open and then you open the Javascript Profiler in the devtools window?

Comment 8

[Benoit Girard (:BenWa)]

It wont work because the profiler component (shared between the two) locks itself.

Comment 9

[(no longer active)]

(In reply to comment #8)
> It wont work because the profiler component (shared between the two) locks
> itself.

That is unacceptable.  :(

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

SPS is completely unusable for me on central because of this bug.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #6)
> I really really wished we just disabled the profile upload feature if PB was
> activated instead of breaking the profiler altogether.

Yes, we should annotate the profile data if we were ever in PB mode during the profile, and if we were ever in PB mode then upload should be disabled or at least have a big fat warning.

Comment 12

[Benoit Girard (:BenWa)]

Workaround is to disable the thumb pref while we get this figured out.

Comment 13

[Mark Hammond [:markh] [:mhammond]]

(In reply to Benoit Girard (:BenWa) from comment #12)
> Workaround is to disable the thumb pref while we get this figured out.

To disable this via prefs, please set the boolean pref 'browser.pagethumbnails.capturing_disabled' to true.

Note that I or others may previously have suggested using the pref 'browser.pageThumbs.enabled' - in the next couple of days this preference is going to be removed (bug 897811) as it turns out we had 2 prefs which did almost identical things - 'browser.pagethumbnails.capturing_disabled' was widely (if informally) documented, so that's the one we will be sticking with.

Comment 14

[Anton Kovalyov (:anton)]

Was there any progress on this? This breaks our JavaScript Profiler in the DevTools and the workaround is not a solution for obvious reasons there.

Comment 15

[(no longer active)]

(In reply to comment #14)
> Was there any progress on this? This breaks our JavaScript Profiler in the
> DevTools and the workaround is not a solution for obvious reasons there.

Somebody needs to remove the code that disables the profiler in the product, and move it to the Gecko Profiler add-on or something.

Comment 16

[:Gavin Sharp [email: [e-mail]]]

That means backing out bug 822135, right?

What negative effects are there to just doing that and nothing else?

Comment 17

[Benoit Girard (:BenWa)]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #16)
> That means backing out bug 822135, right?
> 
> What negative effects are there to just doing that and nothing else?

Users will likely accidentally submit profile containing private browsing (profiles include URLs) and some opt-in features auto send in profiles on shutdown. Those will be send if there's a private window open.

Comment 18

[(no longer active)]

(In reply to comment #17)
> (In reply to :Gavin Sharp (use [e-mail] for email) from comment
> #16)
> > That means backing out bug 822135, right?
> > 
> > What negative effects are there to just doing that and nothing else?
> 
> Users will likely accidentally submit profile containing private browsing
> (profiles include URLs) and some opt-in features auto send in profiles on
> shutdown. Those will be send if there's a private window open.

Right.  FWIW I had an extensive chat with BenWa about this a few weeks ago, and I failed to update the bug.  Sorry about that!

Comment 19

[Benoit Girard (:BenWa)]

Working around the issue for now by having the addon disabling the thumbnail service :( for profiler users:
https://github.com/bgirard/Gecko-Profiler-Addon/commit/7d83c613d671e9cc78e6e6e1da0d2be0e0c35f9f

Update to 1.12.7 for work around.

Comment 20

[Mark Hammond [:markh] [:mhammond]]

Note that with bug 875986 landing, b/g thumbnails no longer use private-browsing - so I think this bug can probably be closed WFM?

Comment 21

[Benoit Girard (:BenWa)]

Ohh I need to verify and then I'll back out that nasty work around.

Comment 22

[:Gavin Sharp [email: [e-mail]]]

(In reply to Benoit Girard (:BenWa) from comment #21)
> Ohh I need to verify and then I'll back out that nasty work around.

Can you do the backout?

Comment 23

[Benoit Girard (:BenWa)]

done