YF (Yang) Reporter
	Description • 10 years ago

Run Firefox 35+, install Enhanced Steam extension, open one or more Steam page, the Firefox may crashed.

The crashes seems to be intermittent, I did not find a steps to stable reproduce.


See https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3Anet%3A%3AHttpBaseChannel%3A%3ASetupReplacementChannel%28nsIURI*%2C+nsIChannel*%2C+bool%29#tab-comments.

	YF (Yang) Reporter
	Comment 1 • 10 years ago

[Tracking Requested - why for this release]: the causing crashes.

	Ruben Petersen
	Comment 2 • 10 years ago

This affects a good few users:
http://steamcommunity.com/groups/enhancedsteam/discussions/0/624075036457937895/

(and the extension's author has "not been able to pin down the particular issue that's causing Firefox 35 to crash when Enhanced Steam is enabled")

As per the thread, the issue might not exist on Firefox 36.

	Trunten
	Comment 3 • 10 years ago

I can confirm the crashing after upgrading to Firefox 35.
Firefox will crash on any of the "Steam" related web sites with the "Enhanced Steam" extension.
Disabling the extension will restore stability to Firefox on the Steam web sites again.
Never had this issue before with other versions of Firefox or the extension.

	kalviskajaks
	Comment 4 • 10 years ago

There are 2 crash signatures relevant to this bug
1)E10s disabled
https://crash-stats.mozilla.com/report/index/1970cdf5-4ecc-4126-a0a7-365842150115
2)E10S enabled 
https://crash-stats.mozilla.com/report/index/efc71dc6-9c0c-47fa-b48f-41e392150115

	Mikael Magnusson
	Comment 5 • 10 years ago

I get the crash in firefox 35 and also in seamonkey 2.32, in case that helps narrow it down more.

	Lukas Blakk [:lsblakk] use ?needinfo
	Comment 6 • 10 years ago

Tracking for now, though this may turn out to be a tech evangelism issue, it's not a driver for a 35 dot release as there is a workaround (disabling Enhanced Steam).

	Lukas Blakk [:lsblakk] use ?needinfo
	Comment 7 • 10 years ago

also added this to known issues on 35/36/37

	Lukas Blakk [:lsblakk] use ?needinfo
	Comment 8 • 10 years ago

Tyler - if there isn't already, would it make sense to put up a support article on disabling this extension for people who wonder about crashes?

	jshackles
	Comment 9 • 10 years ago

Enhanced Steam developer here.  I believe this is related to XMLHttpRequest calls done from the extension context (pageMod) that are performed on pages which have a 302 temporary redirect.  This could have been introduced by fixing bug #1073882 in Firefox 35, as this behavior works as expected in Firefox 34.

I have been able to reliably fix the issue by removing any XMLHttpRequest calls that are done to redirected files, but this limits some of the extension's functionality and could introduce additional browser crashes if files on the remote server are redirected in the future.

I'm happy to provide any additional information I can.

	emtilt
	Comment 10 • 10 years ago

I can confirm that I experience this crash when using Enhanced Steam 7.2.2 and Firefox 35. There are a large number of users that rely on this add on.

	Trunten
	Comment 11 • 10 years ago

Still crashes occasionally with 7.2.3 version of the extension.
(In reply to jshackles from comment #9)
> Enhanced Steam developer here...

	:Gijs (he/him)
	Comment 12 • 10 years ago

This looks like a nullptr crash, presumably because |uri| is null - meaning one of the redirects has a null URI, which seems odd. mmc/mayhemer, do you have ideas about what's going on here?

	Honza Bambas (:mayhemer)
	Comment 13 • 10 years ago

STR:
- Nightly debug
- install https://firefox.enhancedsteam.com/latest/enhanced-steam.xpi
- go to http://store.steampowered.com/
- attach the child process

At...

>	xul.dll!mozilla::ipc::PrincipalToPrincipalInfo(0x0a8def24, 0x00240588) Line 132	C++
 	xul.dll!mozilla::net::propagateLoadInfo(0x0d3e4970, {...}) Line 1305	C++
 	xul.dll!mozilla::net::HttpChannelChild::ContinueAsyncOpen() Line 1591	C++
 	xul.dll!mozilla::net::HttpChannelChild::AsyncOpen(0x0d4d9290, 0x00000000) Line 1486	C++
 	xul.dll!nsXMLHttpRequest::Send(0x00000000, {...}) Line 3009	C++
 	xul.dll!nsXMLHttpRequest::Send({...}) Line 423	C++
 	xul.dll!nsXMLHttpRequest::Send(0x04726040, {...}) Line 434	C++
 	xul.dll!nsXMLHttpRequest::Send(0x04726040, {...}, {...}) Line 461	C++
 	xul.dll!mozilla::dom::XMLHttpRequestBinding::send(0x04726040, {...}, 0x0ac4c300, {...}) Line 710	C++
 	xul.dll!mozilla::dom::GenericBindingMethod(0x04726040, 0x00000001, 0x04e1e5c0) Line 2485	C++


0 get_http(url = "http://api.enhancedsteam.com/early_access/", callback = [function]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/loader/sandbox.js -> resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js":285]
    this = [object Window]
1 anonymous() ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/loader/sandbox.js -> resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js":3315]
    this = [object Window]
2 <TOP LEVEL> ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/loader/sandbox.js -> resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js":3306]
    this = [object Window]
3 load(sandbox = [object Window], uri = "resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js") ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/loader/sandbox.js":66]
4 importScripts(workerSandbox = [object Object], urls = resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/jQuery.min.js,resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/localization.js,resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js, "resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/localization.js", "resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js") ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/content/sandbox.js":290]
    this = null
5 WorkerSandbox(worker = [object Object], window = [object Window]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/content/sandbox.js":252]
    this = [object Object]
6 constructor([object Object], [object Window]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/core/heritage.js":146]
7 initialize(options = [object Object]) ["resource://gre/modules/The thread 0x14bc has exited with code 0 (0x0).


...the aPrincipal is an [nsExpandedPrincipal] having no URI (GetURI returns null but doesn't fail).  Since propagateLoadInfo doesn't check result of mozilla::ipc::PrincipalToPrincipalInfo, we store an empty requestingPrincipalInfo and triggeringPrincipalInfo.  This later fails to serialize and I fail with NS_RUNTIMEABORT("unknown union type") - since it's 0 - at PNeckoChild::Write called as Write((__v).triggeringPrincipalInfo(), __msg) from PNeckoChild::Write(const HttpChannelOpenArgs& __v, Message* __msg).



It's the same reason why we fail to have a URI in the redirect chain later (since it's taken from the same principal).


I see following fixes here:
- fix the add-on to not produce XHR loads w/ url-less principals (?)
- fail the XHR load/creation when principals are url-less (jonas?)
- check URL presence in nsHttpChannel (just a logging code anyway, I can do it right now)

	vinod.chandru
	Comment 14 • 10 years ago

Thanks Honza.
This crashes an extension we've developed as well. Crash Report here: https://crash-stats.mozilla.com/report/index/e57e175b-4477-4741-85c3-ed1302150120 
Since the signature indicates it affects several users, any chance of a dot release with a fix?

	Sylvestre Ledru [:Sylvestre]
	Comment 15 • 10 years ago

Honza, are you able to write a fix for this? This is the last patch we need to do a dot release of 35. Thanks

	Honza Bambas (:mayhemer)
	Comment 16 • 10 years ago

(In reply to Sylvestre Ledru [:sylvestre] from comment #15)
> Honza, are you able to write a fix for this? This is the last patch we need
> to do a dot release of 35. Thanks

See bug 1123732.

	Mikael Magnusson
	Comment 17 • 10 years ago

Based on the patch, I tried compiling firefox with --disable-logging and that does seem to make the crash go away, or at least make it much less rare (I haven't seen it at all yet).

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 18 • 9 years ago

I don't know what our security model is on e10s-desktop. Do we trust child processes to use expanded principals? I'm guessing so since they are not that different from "normal" principals?

Mrbkap, can you verify that that is correct?

If so, I think we need to fix PrincipalToPrincipalInfo to be able to properly send Expanded principals across IPC.

	Blake Kaplan (:mrbkap) (inactive)
	Comment 19 • 9 years ago

(In reply to Jonas Sicking (:sicking) from comment #18)
> Mrbkap, can you verify that that is correct?
> 
> If so, I think we need to fix PrincipalToPrincipalInfo to be able to
> properly send Expanded principals across IPC.

Yeah, we definitely need to make PrincipalToPrincipalInfo handle expanded principals. Jetpack frame scripts are going to definitely want to do mashups, even in e10s.

	Trunten
	Comment 20 • 9 years ago

I no longer have any crashes with (version 7.2.3 of) this extension in Firefox 35.0.1

	Kamil Jozwiak [:kjozwiak]
	Comment 21 • 9 years ago

This is still happening every single time on fx38 (m-c) with e10s enabled. Once you disable e10s, the crash stops.

* Enhanced Steam v7.2.3
* m-c build used: [build id: 20150128101733]
* https://crash-stats.mozilla.com/report/index/5ccd144a-dddc-4eec-af22-d011c2150128

	kalviskajaks
	Comment 22 • 9 years ago

(In reply to Kamil Jozwiak [:kjozwiak] from comment #21)
> This is still happening every single time on fx38 (m-c) with e10s enabled.
> Once you disable e10s, the crash stops.
> 
> * Enhanced Steam v7.2.3
> * m-c build used: [build id: 20150128101733]
> *
> https://crash-stats.mozilla.com/report/index/5ccd144a-dddc-4eec-af22-
> d011c2150128

Can confirm. No crashing anymore with e10s disabled. With e10s enabled going to a steam store page, crashes tab instantly every single time. http://store.steampowered.com/app/218620 for example.

	alex_mayorga
	Comment 23 • 9 years ago

Looks pretty much confirmed and it is even on https://www.mozilla.org/en-US/firefox/36.0beta/releasenotes/ so marking NEW accordingly...

	Sylvestre Ledru [:Sylvestre]
	Comment 24 • 9 years ago

e10s only, stop tracking.

	YF (Yang) Reporter
	Comment 25 • 9 years ago

Note: It also occurs in without e10s.

	kalviskajaks
	Comment 26 • 9 years ago

(In reply to YF (Yang) from comment #25)
> Note: It also occurs in without e10s.

Im running latest enhanced steam and nightly i dont experience any crashes without e10s. As far as i know it was supposed to be fixed?

	YF (Yang) Reporter
	Comment 27 • 9 years ago

(In reply to kalviskajaks from comment #26)
> (In reply to YF (Yang) from comment #25)
> > Note: It also occurs in without e10s.
> 
> Im running latest enhanced steam and nightly i dont experience any crashes
> without e10s. As far as i know it was supposed to be fixed?

sorry, I forgot the should have been fixed by bug 1123732.

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 28 • 9 years ago

Dropping tracking and marking all versions > 38 as unaffected. 38 will also be unaffected once it moves to Aurora. This needs to be fixed specifically for when e10s is enabled.