"PrivateBrowsingUtils is not defined" in nonbrowser-mac.js

VERIFIED FIXED in Firefox 67

Status

()

defect
P1
normal
VERIFIED FIXED
5 months ago
4 months ago

People

(Reporter: mixedpuppy, Assigned: Felipe)

Tracking

(Regression, {regression})

67 Branch
Firefox 68
Unspecified
macOS
Points:
---

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox66 unaffected, firefox67 verified, firefox68 verified)

Details

Attachments

(1 attachment)

Started the profile manager to create a clean profile:

$ ./mach run -P
obj-x86_64-apple-darwin18.2.0/dist/Nightly.app/Contents/MacOS/firefox -P -no-remote -foreground
JavaScript error: resource://gre/modules/XULStore.jsm, line 60: Error: Can't find profile directory.
JavaScript error: resource://gre/modules/osfile/ospath_unix.jsm, line 89: TypeError: invalid path component
JavaScript error: resource://gre/modules/XULStore.jsm, line 60: Error: Can't find profile directory.
JavaScript error: chrome://browser/content/nonbrowser-mac.js, line 76: ReferenceError: PrivateBrowsingUtils is not defined
JavaScript error: resource://gre/modules/XULStore.jsm, line 60: Error: Can't find profile directory.
JavaScript error: resource://gre/modules/XULStore.jsm, line 60: Error: Can't find profile directory.
JavaScript error: chrome://browser/content/nonbrowser-mac.js, line 112: TypeError: BrowserOffline is undefined

Unsure if it's related, but after clicking "create" I get a blank sheet. Was able to escape out and do that again, got normal UI and was able to create the profile.

Did a hg pull just a short while prior to this.

I can confirm this is happening and is new (within the past day or so). (The first can't find profile directory was always there).

Have there been any new changes with the profile stuff, Mossop?

Marking P1 because it's a regression and might point to something major being broken.

Flags: needinfo?(dtownsend)
Priority: -- → P1

LOTS of changes with the profile stuff!

The XULStore.jsm error though has been around for a long time and I think harmless (XULStore is trying to load persisted UI values for the profile manager but there is no store because we don't have a profile yet).

The other stuff does indeed look new though it doesn't seem like something my changes would have caused. I'll take a quick look though.

Flags: needinfo?(dtownsend)

(In reply to Shane Caraveo (:mixedpuppy) from comment #0)

Unsure if it's related, but after clicking "create" I get a blank sheet. Was able to escape out and do that again, got normal UI and was able to create the profile.

This is bug 1533396.

Looks like bug 827976 introduced these new errors. Not sure if they are actual problems or not, they may be similar to the XULStore issue.

Blocks: 827976
Flags: needinfo?(felipc)

So this happens because the hidden window getter now triggers the creation of the hidden window [1], and there's some code that is running that is triggering it (probably nsCocoaUtils::GetHiddenWindowWidget() [2] called by the nsMenuBar code).

Previously, for a profile manager run, the CreateHiddenWindow call inside XREMain::XRE_mainRun() would never be reached, so the getter would just return null and the profile manager wouldn't initialize the hidden window from the menu bar code.

We had to add a similar fix for calls to GetHiddenWindow from process that are not the parent process [3]. Mossop: Is there a simple a simple way to detect "this is a profile manager run, and not the full browser starting up"? (to be called from [3])

I guess I should also add a similar guard for the shutting down case, in case the window has already been destroyed and it gets recreated by accident. (I haven't seen this in the wild, but an old comment in [2] says this is possible)

[1] https://searchfox.org/mozilla-central/rev/7abb9117c8500ed20833746c9f8e800fce3a4688/xpfe/appshell/nsAppShellService.cpp#97
[2] https://searchfox.org/mozilla-central/rev/7abb9117c8500ed20833746c9f8e800fce3a4688/widget/cocoa/nsCocoaUtils.mm#277
[3] https://searchfox.org/mozilla-central/rev/7abb9117c8500ed20833746c9f8e800fce3a4688/xpfe/appshell/nsAppShellService.cpp#108-110

Flags: needinfo?(felipc) → needinfo?(dtownsend)

(In reply to :Felipe Gomes (needinfo me!) from comment #6)

So this happens because the hidden window getter now triggers the creation of the hidden window [1], and there's some code that is running that is triggering it (probably nsCocoaUtils::GetHiddenWindowWidget() [2] called by the nsMenuBar code).

Previously, for a profile manager run, the CreateHiddenWindow call inside XREMain::XRE_mainRun() would never be reached, so the getter would just return null and the profile manager wouldn't initialize the hidden window from the menu bar code.

We had to add a similar fix for calls to GetHiddenWindow from process that are not the parent process [3]. Mossop: Is there a simple a simple way to detect "this is a profile manager run, and not the full browser starting up"? (to be called from [3])

Not specifically (though we could add one) but there are two ways I can think of that identify this as early in startup:

nsIAppStartup.startingUp will return true early in startup. That changes to false here which is pretty close to where the hidden window was created previously. Might cause issues for any UI opened from final-ui-startup. I can't spot any with a quick skim though.
https://searchfox.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4523

Just try and get ProfD from the directory service, if it fails then you're in early startup (changes here: https://searchfox.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4135). I'm guessing the errors we're seeing are exactly because ProfD is not set yet so maybe that is the most accurate check?

Flags: needinfo?(dtownsend)
Assignee: nobody → felipc
Status: NEW → ASSIGNED
Version: 65 Branch → 67 Branch
Pushed by fgomes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/eab09d859167
Do not create the hidden window if it's too early on startup or during shutdown. r=mossop
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68

This is a P1 and 67 is marked as affected, Felipe is that something that you think you be uplifted to beta?

Flags: needinfo?(felipc)

(In reply to Pascal Chevrel:pascalc from comment #11)

This is a P1 and 67 is marked as affected, Felipe is that something that you
think you be uplifted to beta?

Yeah. To be honest these logged error messages are harmless, but the patch is very low risky and I don't want people to be confused by them

Flags: needinfo?(felipc)

Comment on attachment 9052376 [details]
Bug 1533405 - Do not create the hidden window if it's too early on startup or during shutdown. r=mossop

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 827976
  • User impact if declined: Error message gets logged in the "Profile Selection" window on Mac
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce: I don't think uplift needs to block on QE, but reproducing is simple: just open Nightly with the -P to get the Profile Selection window and see the error messages
  • List of other uplifts needed: none
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Simple patch and the what caused these error messages it is understood
  • String changes made/needed: none
Attachment #9052376 - Flags: approval-mozilla-beta?
Has Regression Range: --- → yes
Has STR: --- → yes
Keywords: regression
Regressed by: 827976
No longer blocks: 827976

Comment on attachment 9052376 [details]
Bug 1533405 - Do not create the hidden window if it's too early on startup or during shutdown. r=mossop

Fix for a recent minor regression in 67, low risk but might be noticed by some mac users. Uplift approved for 67 beta 8, thanks.

Attachment #9052376 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Confirming that issue is no longer reproducing using:

  • latest Nightly, build ID: 20190404215521
  • Fx 67.0b8, build ID: 20190404130536.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.