Closed Bug 1218364 Opened 8 years ago Closed 8 years ago

Firefox crashes on opening "about:support" with an WebExtension addon installed


(WebExtensions :: Untriaged, defect)

Not set


(firefox45 fixed, firefox48 verified)

45.2 - Nov 30
Tracking Status
firefox45 --- fixed
firefox48 --- verified


(Reporter: rpl, Assigned: rpl)




(1 file, 2 obsolete files)

Step to reproduce:

- install a WebExtension addon
- open "about:support" in a tab

Expected behaviour:

- new tab opened with "about:support" content rendered

Actual behaviour:

- Firefox crashes
I'm attaching a patch with a testcase and a proposed fix.

The problem is obviously related to the windowless browser created by the WebExtension for the background page.

After a page is actually loaded in the windowless browser, its "layerManagerType" is NONE but the "ClientLayerManager::GetBackendName" method crash because it doesn't have a "NONE case branch" in the "" which resolve the type into a string name.

The above method ("ClientLayerManager::GetBackendName") is used by the "toolkit/modules/Troubleshoot.jsm" to calculate the number of the graphics  accelerate windows.

In the proposed fixes:

- add "LayersBackend::LAYERS_NONE" branch in the "ClientLayerManager::GetBackendName"'s switch-case
- change the Troubleshoot.jsm to recognize the "None" layerManagerType as non-accelerated

In the testcase:

- added browser chrome tests to the "gfx/test" dir 
  (apparently in the existent mochitest test suite I can't access the value sent by the "Troubleshoot.snapshot" method, it raise an exception because we can't access a privileged properties from the testcase context)
- create a new windowless browser and loads "about:blank" in it
- check the expected layerManagerType value
- check Troubleshoot.snapshot doesn't count the windowless browser as an accelerated window (maybe this part of the testcase should be moved elsewhere?)
Assignee: nobody → luca.greco
Attachment #8678804 - Flags: review?(wmccloskey)
Comment on attachment 8678804 [details] [diff] [review]

Review of attachment 8678804 [details] [diff] [review]:

Thanks for figuring this out Luca. I'm not sure this is the right approach though. Troubleshoot.jsm seems to take information about the compositor from the last window that it iterates over. If that happens to be one of these windowless browsers, then we'll get bad data.

I can think of two solutions. One is to make GetLayerManagerType throw for a windowless browser. Then Troubleshoot.jsm will skip it. The other is to have Troubleshoot.jsm skip non-browser windows. Perhaps it could use Services.wm.getEnumerator("navigator:browser").

Jeff, do you have any opinion here?
Attachment #8678804 - Flags: review?(wmccloskey) → review?(jmuizelaar)
(In reply to Bill McCloskey (:billm) from comment #3)
> Jeff, do you have any opinion here?

I'd prefer if Troubleshoot.jsm skipped non-browser windows.
Comment on attachment 8678804 [details] [diff] [review]

Review of attachment 8678804 [details] [diff] [review]:

It seems like not iterating over windowsless browser windows is a better solution
Attachment #8678804 - Flags: review?(jmuizelaar) → review-
Unfortunately enumerating "navigator:browser" doesn't skip the windowless browser.

How about skipping on windowLayerType == "None"'? can be a reasonable approach?

This updated patch apply the above solution (and add the related test steps in the test cas): 
- skip the windowless browser windows in the Troubleshoot.jsm graphics report by checking if windowLayerType == "None"
- Troubleshoot should not count a windowless browser in the numTotalWindow graphics report field, nor in the numAcceleratedWindow
- the presence of a windowless browser window should not set Troubleshoot's graphics.windowLayerManagerType to "None"
Attachment #8678804 - Attachment is obsolete: true
Attachment #8681724 - Flags: review?(wmccloskey)
Attachment #8681724 - Flags: review?(jmuizelaar)
Comment on attachment 8681724 [details] [diff] [review]

Review of attachment 8681724 [details] [diff] [review]:

This looks fine to me.
Attachment #8681724 - Flags: review?(jmuizelaar) → review+
Attachment #8681724 - Flags: review?(wmccloskey) → review+
I rebased this patch on a recent mozilla-central without have any conflicts or changes, follows the related try run (there are 10 intermittents which seem unrelated to this change):


"checkin-needed" keyword applied.
Keywords: checkin-needed
had to back this out for test failures like
Flags: needinfo?(luca.greco)
My apologies, I didn't notice that the last "try run" was too much restricted for this kind of change, which definitely deserves a broader "try run" before the checkin-needed (and I learned an important lesson: "always put a platform different from the one you are working on when you launch a 'try run' restricted to a single platform")

From the above log we can assume that on OSX (and Windows) the existent windows (that are: the mochi test dialog and the firefox windows which runs the test cases) are always accellerated using OpenGL (and DirectX on Windows).

In the updated patch I applied the following changes:

- check layerManagerType != "None" (instead of assuming that it should be 'Basic')
- do not check the number of accelerated windows

Follows a couple of try runs:

- restricted to linux, osx and windows:
- all platforms:
Attachment #8681724 - Attachment is obsolete: true
Flags: needinfo?(luca.greco)
Attachment #8686056 - Flags: review?(wmccloskey)
Attachment #8686056 - Flags: review?(jmuizelaar)
Attachment #8686056 - Flags: review?(jmuizelaar) → review+
Attachment #8686056 - Flags: review?(wmccloskey) → review+
the above try runs confirms that this test case now passes on windows and osx consistently, I'm applying the checkin-needed keyword.
Keywords: checkin-needed
The issue which has generated the backout doesn't seem to relate to this patch:
Keywords: checkin-needed
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Iteration: --- → 45.2 - Nov 30
I was able to reproduce the initial issue on Firefox 44.0a1 (2015-10-21) under Windows 10 64-bit.

Verified fixed on Firefox 48.0a1 (2016-03-24/25) under Windows 10 64-bit, Ubuntu 12.04 32-bit and Mac OS X 10.11.
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.