The basic rule of win32 programming is, if you create native UI, you have to pump windowing messages so the thread can handle things like SendMessage calls. If you don't you risk random deadlocks. DeviceManagerD3D9::Init() creates windows, and runs on a background chromium thread that does not use the right message pump - http://mxr.mozilla.org/mozilla-central/source/gfx/layers/d3d9/DeviceManagerD3D9.cpp#179 This is causing random deadlocks when we enumerate our windows for screen sharing.

It'd probably be better if we just do this for d3d9, since afaict that's the only compositor that needs this. I'm not seeing a way to detect that here where we create the thread in question.

Note this bug will pretty much deadlock any application that tries to query this window for information. Screen readers are good candidates, also debugger tools, and of course our own webrtc code.

Comment on attachment 8488155 [details] [diff] [review] patch v.1 Review of attachment 8488155 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! ::: gfx/layers/ipc/CompositorParent.cpp @@ +149,5 @@ > * Compositor hangs seen in the wild, but is short enough to not miss getting > * native hang stacks. */ > options.permanent_hang_timeout = 2048; // milliseconds > +#if defined(_WIN32) > + options.message_loop_type = MessageLoop::TYPE_UI; nit: Tiny comment would be helpful probably :-).

(In reply to Bas Schouten (:bas.schouten) from comment #3) > > nit: Tiny comment would be helpful probably :-). will do. try push - https://tbpl.mozilla.org/?tree=Try&rev=f4857492ab30

Comment on attachment 8488155 [details] [diff] [review] patch v.1 Patch works for me (and I'm the one who ran into the problem; likely due to my old 7600GT graphics card)

We put together a work around for this for screensharing over in bug 1060738, but that fix isn't working right. It turns out that Windows attaches an IME window to this d3d window, and when we enumerate that, it also locks up the browser. jesup tested the proper fix (posted here) and found no issues. So we're curious if the gfx team feels comfortable uplifting this fix to beta along with the work in bug 1060738. From jesup, our options are: a) land real fix, uplift to beta. I like this. b) ask if the hwnd is d3d9 or a child of it (I assume that can be done). c) Disable screensharing on dx9 windows boxes (I really dislike this - but it only affects older hardware. Downside is we have no UI for showing why they can't use it)

(In reply to Jim Mathies [:jimm] from comment #6) > ...So we're curious if the gfx team feels comfortable > uplifting this fix to beta along with the work in bug 1060738. I'm comfortable if Bas is. > > From jesup, our options are: > a) land real fix, uplift to beta. I like this. > b) ask if the hwnd is d3d9 or a child of it (I assume that can be done). > c) Disable screensharing on dx9 windows boxes (I really dislike this - but > it only affects older hardware. Downside is we have no UI for showing why > they can't use it)

(In reply to Milan Sreckovic [:milan] from comment #10) > (In reply to Jim Mathies [:jimm] from comment #6) > > ...So we're curious if the gfx team feels comfortable > > uplifting this fix to beta along with the work in bug 1060738. > > I'm comfortable if Bas is. > > > > > From jesup, our options are: > > a) land real fix, uplift to beta. I like this. > > b) ask if the hwnd is d3d9 or a child of it (I assume that can be done). > > c) Disable screensharing on dx9 windows boxes (I really dislike this - but > > it only affects older hardware. Downside is we have no UI for showing why > > they can't use it) I am.

Comment on attachment 8488297 [details] [diff] [review] patch to land Approval Request Comment [Feature/regressing bug #]: Issue with the original implementation of OMTC on windows. [User impact if declined]: Random lock ups of 3rd party applications that try to interact with our browser, and internal lockups in webrtc screen sharing code. [Describe test coverage new/current, TBPL]: Landed on mc this morning. I'll land on aurora/beta beginning of next week. [Risks and why]: It's a fairly big under the hood change for the main compositor thread on Windows. The new code path is well tested however. [String/UUID change made/needed]: none.

I guess we won't uplift this that to ESR.

https://hg.mozilla.org/releases/mozilla-aurora/rev/c693ee0aa738 https://hg.mozilla.org/releases/mozilla-beta/rev/e3fe616ef9a2

https://hg.mozilla.org/releases/mozilla-beta/rev/a128f3f1ce1f The correct patch