1159300 - GMP OpenH264 fails to decode on reload on Windows

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect)

Description

[Ethan Hugg [:ehugg]]

Running this test on Windows with FF38-40:
https://mozilla.github.io/webrtc-landing/pc_test.html
with "Require H.264 video" selected, hit start and it works, but if you hit stop and then start again it does not.  On the second time GMP logs that OpenH264 is encoding but not decoding so the video screen remain blank.

This does not happen with FF38-40 on OSX, only on Windows.

Comment 1

[Ethan Hugg [:ehugg]]

Just tested Linux64 and M-C works fine there.  I also built a debug version of mozilla-beta/38 on Windows.  It consistently has this problem and on one of the attempts I ran into this assertion - not sure yet if it's related:

>	xul.dll!mozilla::ipc::Shmem::AssertInvariants() Line 317	C++
 	xul.dll!mozilla::ipc::Shmem::Size<unsigned char>() Line 173	C++
 	xul.dll!mozilla::gmp::GMPSharedMemManager::MgrAllocShmem(mozilla::gmp::GMPSharedMem::GMPMemoryClasses aClass, unsigned int aSize, mozilla::ipc::SharedMemory::SharedMemoryType aType, mozilla::ipc::Shmem * aMem) Line 44	C++
 	xul.dll!mozilla::gmp::GMPVideoEncodedFrameImpl::CreateEmptyFrame(unsigned int aSize) Line 138	C++
 	xul.dll!mozilla::WebrtcGmpVideoDecoder::Decode_g(const webrtc::EncodedImage & aInputImage, bool aMissingFrames, const webrtc::RTPFragmentationHeader * aFragmentation, const webrtc::CodecSpecificInfo * aCodecSpecificInfo, __int64 aRenderTimeMs) Line 659	C++
 	xul.dll!mozilla::runnable_args_m_5_ret<mozilla::WebrtcGmpVideoDecoder *,int (__thiscall mozilla::WebrtcGmpVideoDecoder::*)(webrtc::EncodedImage const &,bool,webrtc::RTPFragmentationHeader const *,webrtc::CodecSpecificInfo const *,__int64),webrtc::EncodedImage,bool,webrtc::RTPFragmentationHeader const *,webrtc::CodecSpecificInfo const *,__int64,int>::Run() Line 481	C++
 	xul.dll!mozilla::SyncRunnable::Run() Line 77	C++
 	xul.dll!nsThread::ProcessNextEvent(bool aMayWait, bool * aResult) Line 855	C++
 	xul.dll!NS_ProcessNextEvent(nsIThread * aThread, bool aMayWait) Line 265	C++
 	xul.dll!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate * aDelegate) Line 368	C++
 	xul.dll!MessageLoop::RunInternal() Line 234	C++
 	xul.dll!MessageLoop::RunHandler() Line 227	C++
 	xul.dll!MessageLoop::Run() Line 201	C++
 	xul.dll!nsThread::ThreadFunc(void * aArg) Line 358	C++
 	nss3.dll!_PR_NativeRunThread(void * arg) Line 397	C
 	nss3.dll!pr_root(void * arg) Line 90	C
 	[External Code]

Comment 2

[Ethan Hugg [:ehugg]]

I got this to work now by adding a return at the beginning of GMPParent::CloseIfUnused() so it never unloads OpenH264.  Perhaps we're unloading it prematurely, or counting wrong since OpenH264 is doing both encode and decode.

Comment 3

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

It's interesting. It looks like the first load returns an openh264 decoder and encoder. On the second load we return an openh264 encoder, but an Adobe CDM decoder.

This probably has something to do with my SelectPluginForAPI changes. I'll try to get this fixed today when I get to my Windows box in the office.

Comment 4

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

I think we got lucky that this ever worked.

The webrtc code is requesting a decode-video[h264] GMP, which not only matches OpenH264, but also the ClearKey and Adobe CDMs.

When the OpenH264 GMP is added back onto the plugin list after shutdown, it is added to the end of the GMPService::mPlugins array, so it's prioritised last.

We *could* make GMPParent::CanBeSharedCrossNodeIds to return |false| for CDMs, but IIRC we plan on using CDMs for non-encrypted decoding at some point.

I think the best thing to do here is to add a new GMP tag to the |APIs| line of gmpopenh264.info so that we can tell it apart.

Comment 5

[Ethan Hugg [:ehugg]]

We need a solution that works with the existing v1.3 and v1.4 plugins and that can be released with FF38 or everyone using H264 on Windows will break with FF38 releases.

Longer term we could release a v1.4.1 that changes the .info file but we are already in the middle of the v1.4 rollout.

Comment 6

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback

Comment 7

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Comment on attachment 8599082 [details] [diff] [review]
Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback

Approval Request Comment
[Feature/regressing bug #]: Adobe CDM; ClearKey CDM.
[User impact if declined]: Total WebRTC breakage on Windows.
[Describe test coverage new/current, TreeHerder]: Local testing.
[Risks and why]: Very low.
[String/UUID change made/needed]: None.

Requesting mozilla-release approval for 38; not 37.

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

[Tracking Requested - why for this release]:
I reckon this should be tracked by relman; breaking WebRTC/Firefox Hello is not something we want to let slip through the cracks.

Comment 9

[Pulsebot]

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

Comment 10

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: [Aurora39] Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback

Carry r+

Comment 11

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

attachment 8599082 [details] [diff] [review] applies on "release" and beta; attachment 8599103 [details] [diff] [review] applies on aurora.

Comment 12

[Ethan Hugg [:ehugg]]

I just made at opt build of mozilla-beta(38) on Win8 with this patch applied.  I was able to start/stop pc_test successfully 10 times, so it appears fixed.

Comment 13

[Carsten Book [:Tomcat]]

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=9392621&repo=mozilla-inbound

Comment 14

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b79c422f85e

Comment 15

[Maire Reavy [:mreavy]]

Edwin wrote the patch and is the owner for this, but we'll look into why it bounced and try to re-land it for him before he gets in later today (in Auckland). (We = WebRTC team)

Comment 16

[Ethan Hugg [:ehugg]]

From what I can tell, this patch tries to select only the OpenH264 plugin for WebRTC by eliminating the CDM encoders from the list.  This is working well is real situations, but these tests do not use OpenH264 unfortunately.   I believe they are using gmp-fake and if you look at dist/bin/gmp-fake/1.0/fake.info it lists APIs other than [h264] which means it gets eliminated from the list so we end up with no codecs to decode on this test.   This is my first guess at least, I haven't thought of a solution yet.

Comment 17

[Randell Jesup [:jesup] (needinfo me)]

We can duplicate the plugin (and clean this up later), removing the decrypt API.

Comment 18

[Randell Jesup [:jesup] (needinfo me)]

Attachment: WIP patch to add a clone of gmp-fake that doesn't do decryption

Comment 19

[Randell Jesup [:jesup] (needinfo me)]

Attachment: WIP patch to add a clone of gmp-fake that doesn't do decryption

didn't hit one Makefile.in

Comment 20

[Randell Jesup [:jesup] (needinfo me)]

Attachment: WIP patch to add a clone of gmp-fake that doesn't do decryption

forgot to hg add the new files

Comment 21

[Randell Jesup [:jesup] (needinfo me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b224c21aa99

Comment 22

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8599502 [details] [diff] [review]
WIP patch to add a clone of gmp-fake that doesn't do decryption

Sorry for the firedrill, but this bug is a 38 blocker.  If people could review ASAP, everyone would appreciate it.

Comment 23

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8599502 [details] [diff] [review]
WIP patch to add a clone of gmp-fake that doesn't do decryption

Review of attachment 8599502 [details] [diff] [review]:
-----------------------------------------------------------------

r+

I'm fine with you landing as is, but if you have time I think you should remove the stuff related to decryption from gmp-fakeopenh264, since you're not using it anyway because the .info file doesn't advertise that capability. That will make the patch smaller.

At a later date, we can remove the decoding from gmp-fake, and/or rename gmp-fake to gmp-fake-decryptor.

::: dom/media/gmp-plugin-openh264/fakeopenh264.info
@@ +1,5 @@
> +Name: fakeopenh264
> +Description: Fake GMP Plugin
> +Version: 1.0
> +APIs: encode-video[h264], decode-video[h264]
> +Libraries: dxva2.dll

The decode-video implementation doesn't use dxva.dll, so you can remove this line.

::: dom/media/gmp-plugin-openh264/gmp-fake.cpp
@@ +1,1 @@
> +/*!

Probably clearer if you renamed gmp-fake.cpp to gmp-fakeopenh264.cpp.

@@ +406,5 @@
> +      return GMPNoErr;
> +    } else if (!strcmp (aApiName, GMP_API_VIDEO_ENCODER)) {
> +      *aPluginApi = new FakeVideoEncoder (static_cast<GMPVideoHost*> (aHostAPI));
> +      return GMPNoErr;
> +    } else if (!strcmp (aApiName, GMP_API_DECRYPTOR)) {

Since your .info file doesn't have an eme-decrypt, I you can remove the GMP_API_DECRYPTOR and GMP_API_ASYNC_SHUTDOWN blocks as GMP_API_DECRYPTOR won't be hit, and GMP_API_ASYNC_SHUTDOWN is not necessary for your purposes and the real OpenH264 doesn't use it. Then you can delete gmp-test-decryptor.*, gmp-test-output-protection.h, gmp-test-storage.* from gmp-plugin-openh264/ directory, and simplify your patch. You'd then not need to include their headers too.

Comment 24

[Randell Jesup [:jesup] (needinfo me)]

Attachment: add a clone of gmp-fake that doesn't do decryption

tweak MOZ_GMP_PATH for mach and gtest

Comment 25

[Randell Jesup [:jesup] (needinfo me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b29dce41ca4 with the gtest fix (and os.pathsep)

Comment 26

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8599556 [details] [diff] [review]
add a clone of gmp-fake that doesn't do decryption

carry forward cpearce's r+

Comment 27

[Mike Hommey [:glandium]]

Comment on attachment 8599556 [details] [diff] [review]
add a clone of gmp-fake that doesn't do decryption

Review of attachment 8599556 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/gmp-plugin-openh264/Makefile.in
@@ +5,5 @@
> +
> +INSTALL_TARGETS += FAKE_GMP_OPENH264_PLUGIN
> +FAKE_GMP_OPENH264_PLUGIN_DEST = $(DEPTH)/dist/bin/gmp-fakeopenh264/1.0
> +FAKE_GMP_OPENH264_PLUGIN_FILES = \
> +  $(SHARED_LIBRARY) \

You don't have to put SHARED_LIBRARY here if you set FINAL_TARGET = 'dist/bin/gmp-fakeopenh264/1.0' in moz.build, and remove NO_DIST_INSTALL.

@@ +7,5 @@
> +FAKE_GMP_OPENH264_PLUGIN_DEST = $(DEPTH)/dist/bin/gmp-fakeopenh264/1.0
> +FAKE_GMP_OPENH264_PLUGIN_FILES = \
> +  $(SHARED_LIBRARY) \
> +  $(srcdir)/fakeopenh264.info \
> +  $(srcdir)/fakeopenh264.voucher

\
$(NULL)

@@ +9,5 @@
> +  $(SHARED_LIBRARY) \
> +  $(srcdir)/fakeopenh264.info \
> +  $(srcdir)/fakeopenh264.voucher
> +
> +include $(topsrcdir)/config/rules.mk

no need for this include.

::: dom/media/gmp-plugin-openh264/gmp-fake.cpp
@@ +1,1 @@
> +/*!

Don't copy the files, since they're identical, just use the ones in gmp-plugin. SOURCES can contain relative paths.
(and if we really wanted copies, they should be indicated as such in the patch; use hg cp)

::: dom/media/gmp-plugin-openh264/moz.build
@@ +22,5 @@
> +NO_VISIBILITY_FLAGS = True
> +# Don't use STL wrappers; this isn't Gecko code
> +DISABLE_STL_WRAPPING = True
> +
> +FAIL_ON_WARNINGS = True

This file should be indicated as a copy of dom/media/gmp-plugin/moz.build.

::: python/mozbuild/mozbuild/mach_commands.py
@@ +675,5 @@
>          gtest_env = {b'GTEST_FILTER': gtest_filter}
>  
>          xre_path = os.path.join(self.topobjdir, "dist", "bin")
>          gtest_env["MOZ_XRE_DIR"] = xre_path
> +        gtest_env["MOZ_GMP_PATH"] = os.path.join(xre_path, "gmp-fake", "1.0") + os.pathsep + os.path.join(xre_path, "gmp-fakeopenh264", "1.0")

os.pathsep.join(os.path.join(xre_path, p, '1.0') for p in ('gmp-fake', 'gmp-fakeopenh264'))

::: testing/gtest/rungtests.py
@@ +66,5 @@
>          Add environment variables likely to be used across all platforms, including remote systems.
>          """
>          env["MOZILLA_FIVE_HOME"] = self.xre_path
>          env["MOZ_XRE_DIR"] = self.xre_path
> +        env["MOZ_GMP_PATH"] = os.path.join(self.xre_path, "gmp-fake", "1.0") + os.pathsep + os.path.join(self.xre_path, "gmp-fakeopenh264", "1.0")

likewise

::: testing/mochitest/mach_commands.py
@@ +469,5 @@
> +                options.gmp_path += os.pathsep
> +                options.gmp_path = os.path.join(
> +                    os.path.dirname(bin_path),
> +                    'gmp-fakeopenh264',
> +                    '1.0')

This entire section could use some factoring, like:

gmp_modules = (
    ('gmp-fake', '1.0'),
    ('gmp-clearkey', '0.1'),
    ('gmp-fakeopenh264', '1.0'),
)

options.gmp_path = os.pathsep.join(os.path.join(os.path.dirname(bin_path), *p) for p in gmp_modules)

Comment 28

[Randell Jesup [:jesup] (needinfo me)]

Attachment: WIP patch to add a clone of gmp-fake that doesn't do decryption

incorporates glandium's and cpearce's reviews

Comment 29

[Randell Jesup [:jesup] (needinfo me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d765ea18d23

Comment 30

[Mike Hommey [:glandium]]

Comment on attachment 8599616 [details] [diff] [review]
WIP patch to add a clone of gmp-fake that doesn't do decryption

Review of attachment 8599616 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/gmp-plugin-openh264/moz.build
@@ +1,1 @@
> +# -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-

Still not marked as a copy in the patch. Please use hg cp.

@@ +5,5 @@
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +# largely a copy of dom/media/gmp-fake/moz.build
> +
> +FINAL_TARGET='dist/bin/gmp-fakeopenh264/1.0'

spaces around =

@@ +7,5 @@
> +# largely a copy of dom/media/gmp-fake/moz.build
> +
> +FINAL_TARGET='dist/bin/gmp-fakeopenh264/1.0'
> +SOURCES += [
> +        '../gmp-plugin/gmp-fake.cpp',

4-space indents

::: testing/gtest/rungtests.py
@@ +66,5 @@
>          Add environment variables likely to be used across all platforms, including remote systems.
>          """
>          env["MOZILLA_FIVE_HOME"] = self.xre_path
>          env["MOZ_XRE_DIR"] = self.xre_path
> +        env["MOZ_GMP_PATH"] = os.pathsep.join(os.path.join(self.xre_path, p, "1.0") for p in ('gmp-fake', 'gmp-fakeopenh264'))

might be better to add some carriage returns, like:

os.pathset.join(
  os.path.join(...)
  for ...
)

::: testing/mochitest/mach_commands.py
@@ +458,5 @@
>                  # shipped in the binary
>                  bin_path = self.get_binary_path()
> +                gmp_modules = (
> +                    ('gmp-fake', '1.0'),
> +                    ('gmp-clearkey', '1.0'),

0.1

Comment 31

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Patch2: Add a clone of gmp-fake that doesn't do decryption

Comment 32

[Mike Hommey [:glandium]]

Comment on attachment 8599631 [details] [diff] [review]
Patch2: Add a clone of gmp-fake that doesn't do decryption

Review of attachment 8599631 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/gmp-plugin-openh264/Makefile.in
@@ +7,5 @@
> +FAKE_GMP_OPENH264_PLUGIN_DEST = $(DEPTH)/dist/bin/gmp-fakeopenh264/1.0
> +FAKE_GMP_OPENH264_PLUGIN_FILES = \
> +  $(srcdir)/fakeopenh264.info \
> +  $(srcdir)/fakeopenh264.voucher \
> +  $(NULL)

As a followup, you can do the same cleanup in dom/media/gmp-plugin.

::: testing/gtest/rungtests.py
@@ +67,5 @@
>          """
>          env["MOZILLA_FIVE_HOME"] = self.xre_path
>          env["MOZ_XRE_DIR"] = self.xre_path
> +        env["MOZ_GMP_PATH"] =
> +            os.pathsep.join(

that needs to be on the same line as the =, otherwise, it's a SyntaxError.

@@ +68,5 @@
>          env["MOZILLA_FIVE_HOME"] = self.xre_path
>          env["MOZ_XRE_DIR"] = self.xre_path
> +        env["MOZ_GMP_PATH"] =
> +            os.pathsep.join(
> +                os.path.join(self.xre_path, p, "1.0")

and those dedented.

@@ +70,5 @@
> +        env["MOZ_GMP_PATH"] =
> +            os.pathsep.join(
> +                os.path.join(self.xre_path, p, "1.0")
> +                for p in ('gmp-fake', 'gmp-fakeopenh264')
> +            )

The same line wrapping should be done in mach_commands.py

Comment 33

[Randell Jesup [:jesup] (needinfo me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=221df2363a66

Comment 34

[Randell Jesup [:jesup] (needinfo me)]

Attachment: [Beta38] Patch 2: Add a clone of gmp-fake that doesn't do decryption (fx38 version)

Beta/38 patch

Comment 35

[Randell Jesup [:jesup] (needinfo me)]

Attachment: [Beta38] Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback

Comment 36

[Randell Jesup [:jesup] (needinfo me)]

Final "what the hell, waste some resources" try failed because   Bug 1155939 landed just before I did my try and breaks when there are two plugins that can decode video

edwin, cpearce: can you resolve this and land it ASAP on inbound?  We desperately need to get this in and uplifted to 38.

Comment 37

[Randell Jesup [:jesup] (needinfo me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f3038b2decb

Comment 38

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: [Uplift Not Required] Patch3: Fix GMP gtest

Problem is as jesup pointed out; now we have two fake plugins. The easiest fix is to make the GeckoMediaPlugins.RemoveAndDeleteForcedInUse gtest ask for a video decoder that also supports decrypting, that way we can narrow the test down to only try to use one specific fake gmp type.

This patch should only be required on m-i, not aurora or beta.

Comment 39

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7792f4778da8
https://hg.mozilla.org/integration/mozilla-inbound/rev/491bcd3dba9f
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7da86419656

Comment 40

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8599687 [details] [diff] [review]
[Beta38] Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback

Approval Request Comment
[Feature/regressing bug #]: Something in EME regressed this
[User impact if declined]: Users won't be able to make a second WebRTC call in the same Firefox instance using OpenH264 GMP.
[Describe test coverage new/current, TreeHerder]: We have plenty of EME tests, so this won't break EME. I don't think the coverage of the OpenH264 use cases is great, but our test suites can't test OpenH264 directly, so it's hard.
[Risks and why]: Pretty low; basically makes us share plugin instances across plugin users less.
[String/UUID change made/needed]: None.

Comment 41

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8599103 [details] [diff] [review]
[Aurora39] Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback

Approval Request Comment
[Feature/regressing bug #]: Something in EME regressed this
[User impact if declined]: Users won't be able to make a second WebRTC call in the same Firefox instance using OpenH264 GMP.
[Describe test coverage new/current, TreeHerder]: We have plenty of EME tests, so this won't break EME. I don't think the coverage of the OpenH264 use cases is great, but our test suites can't test OpenH264 directly, so it's hard.
[Risks and why]: Pretty low; basically makes us share plugin instances across plugin users less.
[String/UUID change made/needed]: None.

Comment 42

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8599686 [details] [diff] [review]
[Beta38] Patch 2: Add a clone of gmp-fake that doesn't do decryption (fx38 version)

Approval Request Comment
[Feature/regressing bug #]: Something in EME regressed this
[User impact if declined]: This makes the tests pass when patch 1 is applied.
[Describe test coverage new/current, TreeHerder]: We have enough tests to fail without this fix. ;) Patch 1 makes EME plugins less sharable, and that means the tests targeted at our "fake" OpenH264, which had a dummy EME decryptor in it, wasn't available to the tests. This moved the OpenH264 fake out into a separate test plugin. So this patch only affects tests.
[Risks and why]: Pretty low; this only affects tests.
[String/UUID change made/needed]: None.

Comment 43

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8599686 [details] [diff] [review]
[Beta38] Patch 2: Add a clone of gmp-fake that doesn't do decryption (fx38 version)

38 is an ESR, we want a clean testsuite. Taking it.
Should be in 38 RC1.

Comment 44

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7792f4778da8
https://hg.mozilla.org/mozilla-central/rev/491bcd3dba9f
https://hg.mozilla.org/mozilla-central/rev/a7da86419656

Comment 45

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8b7119109f2f
https://hg.mozilla.org/releases/mozilla-aurora/rev/b5dc3037d895

Comment 46

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/28521384c589
https://hg.mozilla.org/releases/mozilla-release/rev/d262c6789549

Comment 47

[Ethan Hugg [:ehugg]]

Tested with local build of Opt build M-C on Win8.  Was able to start/stop pc_test.html every time.  Also tested with talky.io and media.navigator.video.preferred_codec=126, and using CiscoSpark.com.  All worked fine with e10s disabled.

Comment 48

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/28521384c589
https://hg.mozilla.org/releases/mozilla-beta/rev/d262c6789549

Comment 49

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr38/rev/28521384c589
https://hg.mozilla.org/releases/mozilla-esr38/rev/d262c6789549

Comment 50

[jen strickland :jenstrickland]

Tracking for 39 and 40 while we verify the fix.

Comment 51

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Comment 42 says that this only affects tests, so I'm not sure what kind of manual verification is needed here. Jen or Liz, could you provide some details?

Comment 52

[Randell Jesup [:jesup] (needinfo me)]

(In reply to Florin Mezei, QA (:FlorinMezei) from comment #51)
> Comment 42 says that this only affects tests, so I'm not sure what kind of
> manual verification is needed here. Jen or Liz, could you provide some
> details?

Patch 2 (referred to by comment 42) is in fact something that only affects mochitests.

Comment 53

[jen strickland :jenstrickland]

Sorry, my bad, I missed that it only affected tests. Removed tracking for 39 and 40.

Comment 54

[Randell Jesup [:jesup] (needinfo me)]

(In reply to Randell Jesup [:jesup] from comment #52)
> Patch 2 (referred to by comment 42) is in fact something that only affects
> mochitests.

Patch 1 however is quite testable; see description

Comment 55

[Catalin Varga [QA][:VarCat]]

Verified as fixed using the following environment:

FF 38 ESR
Build Id: 20150505103531
OS: Win 8.1 x86

Comment 56

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Removing qe-verify flag as verification on Firefox 38 Beta should suffice.