879850 - Test failures due to mock nsIPluginHost

Status: NEW

(Toolkit :: Add-ons Manager, defect, P5)

Description

[:Irving Reid (No longer working on Firefox)]

An odd interaction between OS.File.writeAtomic and existing code is causing crashes in tests. I've tracked the crashes down to a conflict between unit tests that install a mock version of the @mozilla.org/plugin/host;1 service (nsIPluginHost), and core C++ code that assumes the plugin host service will always be the singleton C++ nsPluginHost.

From bug 657952:

According to DXR, tests that mock "@mozilla.org/plugin/host;1" are:

http://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js#l435

http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/xpcshell/test_pluginchange.js#l121

http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/xpcshell/test_bug455906.js#l182

http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/xpcshell/test_bug514327_3.js#l102

http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/xpcshell/test_bug449027.js#l430

http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/xpcshell/test_duplicateplugins.js#l96


See the comments on Bug 657952 beginning at https://bugzilla.mozilla.org/show_bug.cgi?id=657952#c5 for additional information.


I need to disable the crashing tests in my patches for bug 853388; I'll refer to this bug in the xpcshell.ini

Comment 1

[Dave Townsend [:mossop]]

(In reply to :Irving Reid from comment #0)
> An odd interaction between OS.File.writeAtomic and existing code is causing
> crashes in tests. I've tracked the crashes down to a conflict between unit
> tests that install a mock version of the @mozilla.org/plugin/host;1 service
> (nsIPluginHost), and core C++ code that assumes the plugin host service will
> always be the singleton C++ nsPluginHost.
> 
> From bug 657952:
> 
> According to DXR, tests that mock "@mozilla.org/plugin/host;1" are:
> 
> http://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/
> tests/unit/test_TelemetryPing.js#l435
> 
> http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> test/xpcshell/test_pluginchange.js#l121
> 
> http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> test/xpcshell/test_bug455906.js#l182
> 
> http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> test/xpcshell/test_bug514327_3.js#l102
> 
> http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> test/xpcshell/test_bug449027.js#l430
> 
> http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> test/xpcshell/test_duplicateplugins.js#l96
> 
> 
> See the comments on Bug 657952 beginning at
> https://bugzilla.mozilla.org/show_bug.cgi?id=657952#c5 for additional
> information.
> 
> 
> I need to disable the crashing tests in my patches for bug 853388; I'll
> refer to this bug in the xpcshell.ini

Can we just fix these as benjamin suggested. Set a pref in those tests with a new contract id and in PluginProvider.jsm have it check the pref and use that contract instead of the regular pluginhost.

Comment 2

[:Irving Reid (No longer working on Firefox)]

Is this something we do often? It feels rather dirty to me...

Do we need the pref, or could we just have the code in question try to get "@mozilla.org/plugin/host-TEST-MOCK;1" and fall back to ".../host;1" if the mock isn't there?

Comment 3

[Benjamin Smedberg]

Mocking in a complex interdependent system *is* dirty, let's not hide it too much. I don't think we should do automatic fallback unless there's some technical reason we have to.

Comment 4

[:Irving Reid (No longer working on Firefox)]

The fix for bug 881578 has caused the crashes to stop for the time being, because we no longer invoke the plugin handler code for .jsm files.