1267007 - New tabs are initially blocked with Electrolysis enabled (TypeError: Cc[aContract] is undefined)

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Marco Perez [:mpk]]

This is probably not the best category/component for which to file this bug.
Please change as appropriate.

On FreeBSD (10-stable, maybe other BSDs too) there's a problem with Electrolysis enabled.
Opening a new tab will always open the tab in the background. Clicking on it won't activate it. The only way to break that lock is by clicking on the currently active tab first. Once that's done, it'll work fine for the rest of the session.

A bigger problem occurs when this is not done. As soon as the user clicks on a link which would normally open in a new tab, Firefox crashes.

Here's a stacktrace for the non-crashing case:

CustomizableUI:TypeError: Cc[aContract] is undefined
Stack trace:
XPCU_serviceLambda@resource://gre/modules/XPCOMUtils.jsm:230:7
XPCU_defineLazyGetter/<.get@resource://gre/modules/XPCOMUtils.jsm:198:21
_getSwitcher/switcher.addMarker@chrome://browser/content/tabbrowser.xml:3771:1
_getSwitcher/switcher.startTabSwitch@chrome://browser/content/tabbrowser.xml:3724:15
_getSwitcher/switcher.requestTab@chrome://browser/content/tabbrowser.xml:3668:15
set_selectedIndex@chrome://browser/content/tabbrowser.xml:6771:11
set_selectedPanel@chrome://global/content/bindings/tabbox.xml:687:13
set_selectedIndex@chrome://global/content/bindings/tabbox.xml:403:15
set_selectedItem@chrome://global/content/bindings/tabbox.xml:435:34
set_selectedTab@chrome://global/content/bindings/tabbox.xml:114:15
set_selectedTab@chrome://browser/content/tabbrowser.xml:2913:11
loadOneTab@chrome://browser/content/tabbrowser.xml:1504:15
openLinkIn@chrome://browser/content/utilityOverlay.js:358:5
openUILinkIn@chrome://browser/content/utilityOverlay.js:196:3
switchToTabHavingURI@chrome://browser/content/browser.js:7370:7
openPreferences@chrome://browser/content/utilityOverlay.js:567:16
preferencesButton.onCommand@resource:///modules/CustomizableWidgets.jsm:1108:7
CustomizableUIInternal.handleWidgetCommand@resource:///modules/CustomizableUI.jsm:1488:11
EventListener.handleEvent*CustomizableUIInternal.buildWidget@resource:///modules/CustomizableUI.jsm:1380:7
CustomizableUIInternal.getWidgetNode@resource:///modules/CustomizableUI.jsm:895:16
CustomizableUIInternal.buildArea@resource:///modules/CustomizableUI.jsm:721:32
CustomizableUIInternal.registerMenuPanel@resource:///modules/CustomizableUI.jsm:922:5
this.CustomizableUI.registerMenuPanel@resource:///modules/CustomizableUI.jsm:3058:5
PanelUI.ensureReady/this._readyPromise<@chrome://browser/content/customizableui/panelUI.js:268:11
TaskImpl_run@resource://gre/modules/Task.jsm:319:40
promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:395:7
TaskImpl_run@resource://gre/modules/Task.jsm:327:13
TaskImpl@resource://gre/modules/Task.jsm:280:3
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:254:14
Task_spawn@resource://gre/modules/Task.jsm:168:12
PanelUI.ensureReady@chrome://browser/content/customizableui/panelUI.js:228:26
PanelUI.show/<@chrome://browser/content/customizableui/panelUI.js:129:7
Promise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:388:5
PanelUI.show@chrome://browser/content/customizableui/panelUI.js:128:12
PanelUI.toggle@chrome://browser/content/customizableui/panelUI.js:116:7
PanelUI.handleEvent@chrome://browser/content/customizableui/panelUI.js:200:11
EventListener.handleEvent*PanelUI.init@chrome://browser/content/customizableui/panelUI.js:51:5
gBrowserInit._delayedStartup@chrome://browser/content/browser.js:1175:5
EventListener.handleEvent*gBrowserInit.onLoad@chrome://browser/content/browser.js:1028:5
onload@chrome://browser/content/browser.xul:1:1
1CustomizableUI.jsm:1490

Comment 1

[:Gijs (he/him)]

What build of Firefox are you using? Does this happen on an official mozilla.org build? In the crashing case, can you provide a link to a crashreport from about:crashes ?

Comment 2

[Marco Perez [:mpk]]

This is a home-built nightly on FreeBSD 10-stable (with tests disabled and alsa enabled).
I have witnessed this behaviour on FreeBSD only, and there are no official BSD builds from mozilla.org (or are there?). I'll try to provide the requested crash info once I have access to my home box.

Comment 3

[:Gijs (he/him)]

(In reply to Marco Perez from comment #2)
> This is a home-built nightly on FreeBSD 10-stable (with tests disabled and
> alsa enabled).

Then it sounds like an issue with your build... what does your mozconfig file look like ?

Based on the JS stack, it looks like you've disabled profiling, and trying to get Services.profiler throws an exception:

https://dxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#3743-3747

I expect that check needs to be more cautious. (cc mconley + felipe)


A crash stack and your full mozconfig would still be interesting, because I don't think the crash will be the same issue - though fixing this one might wallpaper over whatever's crashing.

Comment 4

[:Gijs (he/him)]

The profiler check is supposed to work - it broke when bug 1151598 replaced an #ifdef with a check with AppConstants, which doesn't work because MOZ_ENABLE_PROFILER_SPS is actually not defined there, and the default value in Services.jsm's initTable checking is 'true', which will be used for an explicitly passed undefined, apparently...

Comment 5

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #4)
> The profiler check is supposed to work - it broke when bug 1151598 replaced
> an #ifdef with a check with AppConstants, which doesn't work because
> MOZ_ENABLE_PROFILER_SPS is actually not defined there, and the default value
> in Services.jsm's initTable checking is 'true', which will be used for an
> explicitly passed undefined, apparently...

This check also broke the conditionals for MOZ_TOOLKIT_SEARCH (Services.search).

Comment 6

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1267007 - fix profiler and search conditional Services.jsm declarations, r?billm

Review commit: https://reviewboard.mozilla.org/r/48729/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48729/

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8744937 [details]
MozReview Request: Bug 1267007 - fix profiler and search conditional Services.jsm declarations, r?billm

https://reviewboard.mozilla.org/r/48729/#review45551

Comment 8

[Marco Perez [:mpk]]

(In reply to :Gijs Kruitbosch from comment #3)
>
> Then it sounds like an issue with your build... what does your mozconfig
> file look like ?
> 

It's actually quite short. I have alsa enabled in order to get audio working on FreeBSD. The tests broke just a few weeks ago. I currently also have to add https://bugzilla.mozilla.org/attachment.cgi?id=8739971 in order to unbreak building m-c on FreeBSD. Apart from that it's a standard trunk build.

mk_add_options MOZ_OBJDIR="/work/build"
mk_add_options MOZ_MAKE_FLAGS="-s -j4"
ac_add_options --disable-tests
ac_add_options --enable-alsa

Thanks for the patch. I'll apply it before trying to build again (to get the crash stack).

Comment 9

[Marco Perez [:mpk]]

Lo and behold! The bug is no more! \o/

It's either a secondary effect of your patch or a check-in during this timeframe:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2be765b599b2&tochange=2045bc8c9e90

Its' been bugging me for months and has prevented me from switching e10s on full-time.
I can try to narrow down whatever fixed it, but I'm inclined to just call it WFM.

Comment 10

[:Gijs (he/him)]

(In reply to Marco Perez from comment #9)
> Lo and behold! The bug is no more! \o/
> 
> It's either a secondary effect of your patch or a check-in during this
> timeframe:
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=2be765b599b2&tochange=2045bc8c9e90
> 
> Its' been bugging me for months and has prevented me from switching e10s on
> full-time.
> I can try to narrow down whatever fixed it, but I'm inclined to just call it
> WFM.

Well, as I said, I imagine that fixing the Services.profiler issue will wallpaper over the crash... if you unapply the patch and run:

./mach build faster

(no really, that's a thing - it'll take <15 seconds)

then you should be able to re-test and get a crash stack, which I would honestly still be interested to see. :-)

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e053673b0dd6

Comment 12

[Marco Perez [:mpk]]

Attachment: stack trace (bt full from gdb)

No kidding! :-) "faster" boosted the build time from 30s to less than 3s.

Here's the requested stack trace. If you want me to apply some special settings before the crash or to get additional debug info, just let me know.

Comment 13

[:Gijs (he/him)]

(In reply to Marco Perez from comment #12)
> Created attachment 8745079 [details]
> stack trace (bt full from gdb)
> 
> No kidding! :-) "faster" boosted the build time from 30s to less than 3s.
> 
> Here's the requested stack trace. If you want me to apply some special
> settings before the crash or to get additional debug info, just let me know.

That looks helpful - but definitely in e10s land, so I'm not the best person to look at it myself. Bill, anything you can make out of that?

Comment 14

[:Gijs (he/him)]

(In reply to Marco Perez from comment #12)
> Created attachment 8745079 [details]
> stack trace (bt full from gdb)
> 
> No kidding! :-) "faster" boosted the build time from 30s to less than 3s.

(Just to be clear - this only works because it builds a subset of what is built without passing 'faster'. If you update your tree tomorrow and want the C++ side of things rebuilding, you sadly can't use it. But it's very useful for small, JS/CSS/XUL-only changes like the patch on this bug.)

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e053673b0dd6

Comment 16

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Sorry, I'm not sure. I suspect something is going wrong before this happens. It might help to run a DEBUG build and attach any console output. You'll have to rebuild with --enable-debug.

Comment 17

[:Gijs (he/him)]

(In reply to Bill McCloskey (:billm) from comment #16)
> Sorry, I'm not sure. I suspect something is going wrong before this happens.
> It might help to run a DEBUG build and attach any console output. You'll
> have to rebuild with --enable-debug.

OK. At this stage it probably makes sense to split this off into a separate bug, if it happens after this fix... and I'll see about getting the fix uplifted. Marking this fixed and so on to get it on the right radars.

Comment 18

[:Gijs (he/him)]

Comment on attachment 8744937 [details]
MozReview Request: Bug 1267007 - fix profiler and search conditional Services.jsm declarations, r?billm

Approval Request Comment
[Feature/regressing bug #]: bug 1151598
[User impact if declined]: builds with e10s and without a profiler break very badly
[Describe test coverage new/current, TreeHerder]: general test coverage around these features, yes. Not for this case.
[Risks and why]: very low, simple fix to AppConstants so some of the build-time defines/configure variables are honoured correctly.
[String/UUID change made/needed]: no.

Comment 19

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8744937 [details]
MozReview Request: Bug 1267007 - fix profiler and search conditional Services.jsm declarations, r?billm

e10s related, Beta47+, Aurora48+

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7999c04a50fe

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/4076bba66266