search

mozilla / lightbeam

Orignal unmaintained version of the Lightbeam extension. See lightbeam-we for the new one which works in modern versions of Firefox.
https://github.com/mozilla/lightbeam-we
Mozilla Public License 2.0
587 stars 149 forks source link

Fix logging websites in multiprocess Firefox #698

Closed koenkivits closed 8 years ago

koenkivits commented 8 years ago

Logging websites seems to be broken in multiprocess Firefox. Just run cfx run --e10s and you won't see any requests being tracked by Lightbeam (tested in Developer Edition 42.0a2 and Nightly 44.0a1).

The problem is in the part where the tab for each HTTP request is figured out in lib/tab/utils.js. It tries to do that through the load context's topWindow, which is unavailable in multiprocess Firefox. It crashes when you try to access it:

Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsILoadContext.topWindow]" nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)" location: "JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://jid1-f9uj2thwoam5gq-at-jetpack/lightbeam/lib/tab/utils.js :: getTabForChannel :: line 62" data: no

I fixed it by using loadContext.topFrameElement in browsers that support it, which gives us a chrome browser object which we can work with.

Note that getTabForChannel2 (a fallback function for getTabForChannel) should still always fail in multiprocess. The [alternative](https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox/Limitations_of_chrome_scripts#HTTP_requests suggested by MDN) needs a load context, and this function is a fallback for when no load context is available. I couldn't find out why the fallback was needed, so I left it alone.

fmarier commented 8 years ago

Thanks, I can confirm that it works in Firefox 44 both with and without e10s.

However, it breaks Lightbeam in Firefox 38 without e10s. Any chance you can resolve that? I'd love to merge this in.

koenkivits commented 8 years ago

No problem! Will update once I have a patch for this.

koenkivits commented 8 years ago

It should be fixed now. Although topFrameElement is defined in Firefox 38, the getTabForBrowser call for it fails. This is specific to Firefox 38, as the problem does not occur in version 39. The code now falls back to the old behavior when this happens. I also refactored the code a bit to be a more readable after the extra fallback. I tested most Firefox versions >= 36, and they all seemed to work.

fmarier commented 8 years ago

Awesome, thanks so much for looking into this and proposing a patch!