Last Comment Bug 1133351 - Flash hangs on right-click on arbitrary SWFs
: Flash hangs on right-click on arbitrary SWFs
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: 37 Branch
: All Windows 8.1
-- normal (vote)
: mozilla40
Assigned To: Aaron Klotz [:aklotz]
:
: Benjamin Smedberg [:bsmedberg]
Mentors:
: 1143395 1165189 (view as bug list)
Depends on: 1254969 1138078 1151318
Blocks: 1014673
  Show dependency treegraph
 
Reported: 2015-02-15 10:09 PST by Till Schneidereit [till]
Modified: 2016-03-09 05:18 PST (History)
14 users (show)
florin.mezei: qe‑verify+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
wontfix
+
verified
verified
unaffected
40+
verified


Attachments
about:support content (7.53 KB, text/plain)
2015-02-15 10:15 PST, Till Schneidereit [till]
no flags Details
Make Windows IPC play nicely with COM STA marshaling (10.37 KB, patch)
2015-03-25 15:07 PDT, Aaron Klotz [:aklotz]
no flags Details | Diff | Splinter Review
Make Windows IPC play nicely with COM STA marshaling (r2) (14.19 KB, patch)
2015-03-25 20:56 PDT, Aaron Klotz [:aklotz]
benjamin: review+
lhenry: approval‑mozilla‑beta+
lmandel: approval‑mozilla‑esr38+
Details | Diff | Splinter Review
Part 2: Use SetWinEventHook to watch for the COM window (6.61 KB, patch)
2015-03-30 12:29 PDT, Aaron Klotz [:aklotz]
no flags Details | Diff | Splinter Review
Part 2: Use SetWinEventHook to watch for the COM window (r2) (6.46 KB, patch)
2015-03-30 14:07 PDT, Aaron Klotz [:aklotz]
bent.mozilla: review+
Details | Diff | Splinter Review
Part 2: Use SetWinEventHook to watch for the COM window (r3) (6.40 KB, patch)
2015-03-31 10:22 PDT, Aaron Klotz [:aklotz]
aklotz: review+
lhenry: approval‑mozilla‑beta+
lmandel: approval‑mozilla‑esr38+
Details | Diff | Splinter Review

User Story
STR:

* Using Windows 8.1
* Open C:\Program Files\Common Files\microsoft shared\ink\tabtip.exe
* Open a page containing windowless Flash: e.g. http://benjamin.smedbergs.us/tests/ctptests/flash-solo-wless.html
* Right-click the Flash element

Actual:
Flash deadlocks for 45 seconds      
Description User image Till Schneidereit [till] 2015-02-15 10:09:24 PST
I can produce immediate plugin hangs when clicking on all and any embedded SWFs by simply right-clicking on them in current beta:

https://crash-stats.mozilla.com/report/index/e3850bd6-ed92-4f47-b5fd-013792150215
https://crash-stats.mozilla.com/report/index/11d1c79a-aa08-4ba6-a9b8-746442150215
https://crash-stats.mozilla.com/report/index/e39ef536-a518-4de1-ba4f-7839e2150215


With beta 8 that had the sandbox experiment active, the hang only occurred on about 25% of all clicks:

https://crash-stats.mozilla.com/report/index/da206d69-ea07-4c75-b458-8e8942150215


In current Developer Edition, the hang also occurred, but only after 50 or so clicks:

https://crash-stats.mozilla.com/report/index/41ddf124-5570-40a0-9230-723272150215

I took a full memory dump of a hang in beta 9 with Process Explorer:
https://www.dropbox.com/s/3236hlwum6esdlu/flash-hang-dump-2015-02-15.zip?dl=0

Note that CPU usage of the plugin-container.exe process was closer to or at times above 1. I wasn't able to contact bsmedberg for now, so I'm still posting the bug.
Comment 1 User image Till Schneidereit [till] 2015-02-15 10:15:30 PST
Created attachment 8564767 [details]
about:support content
Comment 2 User image Benjamin Smedberg [:bsmedberg] 2015-02-16 10:09:53 PST
Does this happen on both windowed and windowless SWFs, or just one kind?

This is definitely accessibility-related: CARET::UpdateMSAAEditFieldState ends up calling into COM which is running a modal loop and blocking up the works. Do you know of utilities or laptop drivers that might be using the accessibility API?
Comment 3 User image Benjamin Smedberg [:bsmedberg] 2015-02-16 11:37:06 PST
Can you run a debug build and see if there is any interesting spew about "received 'nonqueued'" message and if so what the message ID is?
Comment 4 User image Till Schneidereit [till] 2015-02-17 09:22:08 PST
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #3)
> Does this happen on both windowed and windowless SWFs, or just one kind?

It only happens on windowless SWFs. All wmodes except for "window" seem to be affected.

> Can you run a debug build and see if there is any interesting spew about
> "received 'nonqueued'" message and if so what the message ID is?

I didn't get a message about receiving nonqueued messages, but a failed assertion about sending one:

[15404] ###!!! ASSERTION: Failed to prevent a nonqueued message from running!: '!mozilla::ipc::MessageChannel::IsPumpingMessages()', file c:\builds\moz2_slave\m-beta-w32-d-00000000000000000\build\widget\windows\nsWindow.cpp, line 4252

I found out that terminating the TabTip.exe screen keyboard process makes the hang go away. Restarting it makes the hang reappear. So yes, it's accessibility related all right.

One thing that might or might not be interesting is that, with TabTip.exe disabled, the following message is spewed 4 times for each opening of the context menu:

[3108] WARNING: An unhandled ISMEX_SEND message was received during spin loop! (7F): file c:\builds\moz2_slave\m-beta-w32-d-00000000000000000\build\widget\windows\nsWindow.cpp, line 4242
Comment 5 User image Benjamin Smedberg [:bsmedberg] 2015-02-17 09:42:23 PST
Can you break at the assertion and tell me what message code is running, and maybe what window the message is being delivered to?

0x7F is WM_GETICON, which is interesting. We probably need to add that to nsWindow::IsAsyncResponseEvent. But I don't think that has much to do with the hang.
Comment 6 User image Till Schneidereit [till] 2015-02-21 13:56:21 PST
On a hunch I just tried running TabTip.exe in my Windows 8.1 instance in VMWare, and sure enough, I can reproduce the hang.

AFAICT, that means two things: debugging this doesn't require buying a specific machine, and it probably affects each and every Windows machine with a touch display, because TabTip.exe is running by default on those.

Benjamin, do you want me to continue analyzing this, or do you have a Windows installation you can do it on directly?
Comment 7 User image Benjamin Smedberg [:bsmedberg] 2015-02-26 10:49:55 PST
I'll take this.
Comment 8 User image Benjamin Smedberg [:bsmedberg] 2015-03-02 08:26:34 PST
According to the data collected in bug 1138078, this bug accounts for 2-3% of all Flash hangs.
Comment 9 User image Aaron Klotz [:aklotz] 2015-03-25 15:07:58 PDT
Created attachment 8583381 [details] [diff] [review]
Make Windows IPC play nicely with COM STA marshaling
Comment 10 User image Aaron Klotz [:aklotz] 2015-03-25 17:11:01 PDT
Comment on attachment 8583381 [details] [diff] [review]
Make Windows IPC play nicely with COM STA marshaling

This is the patch that bent and I came up with. We decided to avoid going with CoWaitForMultipleHandles and instead add COM-aware smarts to our existing code. To make this work we need to find the hidden COM window for our thread, then pump any messages that are destined for that window.

We're a bit unsure as to the lifetime of this hidden window so we have some assertions in there that will fail if the window is destroyed.

We also intentionally included an unbalanced CoInitialize call in an effort to prevent the hidden COM window from being destroyed.
Comment 11 User image Aaron Klotz [:aklotz] 2015-03-25 20:56:17 PDT
Created attachment 8583584 [details] [diff] [review]
Make Windows IPC play nicely with COM STA marshaling (r2)

Missed some file removals when I imported the patch. Here's the revised one.
Comment 12 User image Jim Mathies [:jimm] 2015-03-26 06:58:45 PDT
Comment on attachment 8583584 [details] [diff] [review]
Make Windows IPC play nicely with COM STA marshaling (r2)

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

::: ipc/glue/WindowsMessageLoop.cpp
@@ +650,5 @@
>    // on startup.
>    if (!gUIThreadId) {
>      gUIThreadId = GetCurrentThreadId();
> +
> +    CoInitialize(nullptr);

Why do we need to trick COM into keeping itself alive longer than it normally would?

Unbalanced calls like this on the main thread are kinda gross.. at least add a comment warning people that we have unmatched coinit calls, and that this is a purposeful omission.
Comment 13 User image Ben Turner (not reading bugmail, use the needinfo flag!) 2015-03-26 08:24:05 PDT
Comment on attachment 8583584 [details] [diff] [review]
Make Windows IPC play nicely with COM STA marshaling (r2)

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

::: ipc/glue/WindowsMessageLoop.cpp
@@ +650,5 @@
>    // on startup.
>    if (!gUIThreadId) {
>      gUIThreadId = GetCurrentThreadId();
> +
> +    CoInitialize(nullptr);

Our fear was that if the COM counter ever goes to 0 then Windows might try to kill the hidden window, and we don't want that hidden window to ever go away to avoid the cost of having to find it again every time we send an IPC message.
Comment 14 User image Ben Turner (not reading bugmail, use the needinfo flag!) 2015-03-26 08:25:11 PDT
(In reply to Ben Turner <unresponsive until 3/30> (use the needinfo flag!) from comment #13)
> Our fear was that if the COM counter ever goes to 0 then Windows might try
> to kill the hidden window

Of course none of this is documented so we're just being paranoid.
Comment 15 User image Aaron Klotz [:aklotz] 2015-03-26 09:38:43 PDT
I guess that one option is that we could make the COM HWND a member of MessageChannel and include balanced CoInitialize/CoUninitialize calls there... my concern in that case would be mainly that CoInitialize can be an expensive call to make.
Comment 16 User image Benjamin Smedberg [:bsmedberg] 2015-03-26 09:42:55 PDT
We already have OleInitialize (which implies CoInitialized) at plugin startup, so I don't think there's anything necessary to do here:

http://mxr.mozilla.org/mozilla-central/source/dom/plugins/ipc/PluginProcessChild.cpp#98
Comment 17 User image Ben Turner (not reading bugmail, use the needinfo flag!) 2015-03-26 09:57:47 PDT
This is needed in the parent process though.
Comment 18 User image Benjamin Smedberg [:bsmedberg] 2015-03-27 09:55:08 PDT
This needs dedicated testing, both of the current tests as well as the old testcases that led to the original message filter, and around general usage of accessibility tools.
Comment 20 User image Aaron Klotz [:aklotz] 2015-03-28 06:10:01 PDT
I'm going to have to back this out: our worst fears have come true by this assertion failure:
https://bugzilla.mozilla.org/show_bug.cgi?id=622272#c227

It looks like the HWND of the COM window changed :-(

https://hg.mozilla.org/integration/mozilla-inbound/rev/7bcbabfb63fe
Comment 21 User image Phil Ringnalda (:philor) 2015-03-28 11:52:02 PDT
https://hg.mozilla.org/mozilla-central/rev/0cc8abe4e2bb
Comment 22 User image Phil Ringnalda (:philor) 2015-03-28 12:44:25 PDT
Sorry, missed seeing the backout, and then for extra clownshoes hg's internal merge made lovely decisions while merging the no-backout m-c back to the backout m-i. Should be properly backed out everywhere now, I hope.
Comment 23 User image Phil Ringnalda (:philor) 2015-03-28 20:37:39 PDT
https://hg.mozilla.org/mozilla-central/rev/7bcbabfb63fe
Comment 24 User image Ben Turner (not reading bugmail, use the needinfo flag!) 2015-03-28 21:15:12 PDT
(In reply to Phil Ringnalda (:philor) from comment #23)
> https://hg.mozilla.org/mozilla-central/rev/7bcbabfb63fe

Hm, this needs another backout now?
Comment 25 User image Ben Turner (not reading bugmail, use the needinfo flag!) 2015-03-28 21:15:56 PDT
(In reply to Aaron Klotz [:aklotz] (please use needinfo) from comment #20)
> It looks like the HWND of the COM window changed :-(

I guess we could try subclassing the COM window and watch for WM_DESTROY?
Comment 26 User image Phil Ringnalda (:philor) 2015-03-28 21:31:45 PDT
Poor worn out bug is going to wind up with whiplash. No, 7bcbabfb63fe was the backout, but since e9769c80ee59 was too, mcMerge somehow decided that 7bcbabfb63fe must not have been a backout despite saying it was, and called it a fix.
Comment 27 User image Aaron Klotz [:aklotz] 2015-03-29 14:18:35 PDT
(In reply to Ben Turner <unresponsive until 3/30> (use the needinfo flag!) from comment #25)
> (In reply to Aaron Klotz [:aklotz] (please use needinfo) from comment #20)
> > It looks like the HWND of the COM window changed :-(
> 
> I guess we could try subclassing the COM window and watch for WM_DESTROY?

I'd like to try monitoring the main thread for window creation and destruction using SetWinEventHook. That way we wouldn't need to subclass and we wouldn't need to mess around with CoInitialize/CoUninitialize. I'll throw together a patch and see if it works with message-only windows.
Comment 28 User image Aaron Klotz [:aklotz] 2015-03-30 12:29:02 PDT
Created attachment 8585651 [details] [diff] [review]
Part 2: Use SetWinEventHook to watch for the COM window

This patch applies on top of the first one. It uses SetWinEventHook to watch for window creation and destruction events and updates the COM HWND accordingly. We still need FindCOMWindow for the initial start up in case it has been created prior to setting the hook.
Comment 29 User image Aaron Klotz [:aklotz] 2015-03-30 14:07:13 PDT
Created attachment 8585709 [details] [diff] [review]
Part 2: Use SetWinEventHook to watch for the COM window (r2)

Fixed something I wasn't happy with.
Comment 30 User image Ben Turner (not reading bugmail, use the needinfo flag!) 2015-03-31 00:48:57 PDT
Comment on attachment 8585709 [details] [diff] [review]
Part 2: Use SetWinEventHook to watch for the COM window (r2)

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

I can't believe this works!

Also, r+ :)

::: ipc/glue/WindowsMessageLoop.cpp
@@ +104,5 @@
>  HHOOK gDeferredGetMsgHook = nullptr;
>  HHOOK gDeferredCallWndProcHook = nullptr;
>  
>  DWORD gUIThreadId = 0;
>  HWND gCOMWindow = 0;

Nit: Rename to gOLEWindow maybe?

@@ +105,5 @@
>  HHOOK gDeferredCallWndProcHook = nullptr;
>  
>  DWORD gUIThreadId = 0;
>  HWND gCOMWindow = 0;
> +HWINEVENTHOOK gWinEventHook = nullptr;

Might want to comment that we intentionally never Unhook this, we just save the value to see if we need to install a hook or not.

@@ +106,5 @@
>  
>  DWORD gUIThreadId = 0;
>  HWND gCOMWindow = 0;
> +HWINEVENTHOOK gWinEventHook = nullptr;
> +const wchar_t OleWindowClassName[] = L"OleMainThreadWndClass";

Nit: s/OleWindowClassName/kOleWindowClassName/

@@ +116,5 @@
> +WinEventHook(HWINEVENTHOOK aWinEventHook, DWORD aEvent, HWND aHwnd,
> +             LONG aIdObject, LONG aIdChild, DWORD aEventThread,
> +             DWORD aMsEventTime)
> +{
> +  MOZ_ASSERT(aWinEventHook == gWinEventHook);

Also |MOZ_ASSERT(gUIThreadId == aEventThread)| too?

@@ +127,5 @@
> +      wchar_t classBuf[256] = {0};
> +      int result = ::GetClassNameW(aHwnd, classBuf,
> +                                   MOZ_ARRAY_LENGTH(classBuf));
> +      if (result != (MOZ_ARRAY_LENGTH(OleWindowClassName) - 1) ||
> +          wcsncmp(OleWindowClassName, classBuf, MOZ_ARRAY_LENGTH(classBuf))) {

The last param should probably be |result| I think, though it doesn't really matter I guess.

@@ +131,5 @@
> +          wcsncmp(OleWindowClassName, classBuf, MOZ_ARRAY_LENGTH(classBuf))) {
> +        // Not a class we're interested in
> +        return;
> +      }
> +      gCOMWindow = aHwnd;

Maybe |MOZ_ASSERT(FindCOMWindow() == aHwnd)| here?

@@ +136,5 @@
> +      break;
> +    }
> +    case EVENT_OBJECT_DESTROY: {
> +      if (aHwnd == gCOMWindow && aIdObject == OBJID_WINDOW &&
> +          aIdChild == CHILDID_SELF) {

Is the |aIdChild == CHILDID_SELF| condition something that could be asserted rather than checked in release builds? I hope these windows never get child windows...

@@ +712,5 @@
> +
> +  if (!gWinEventHook) {
> +    gWinEventHook = SetWinEventHook(EVENT_OBJECT_CREATE, EVENT_OBJECT_DESTROY,
> +                                    NULL, &WinEventHook, GetCurrentProcessId(),
> +                                    gUIThreadId, WINEVENT_OUTOFCONTEXT);

Hm, does it work if you pass 0 as the last param?
Comment 31 User image Jim Mathies [:jimm] 2015-03-31 02:32:49 PDT
(In reply to Aaron Klotz [:aklotz] (please use needinfo) from comment #15)
> I guess that one option is that we could make the COM HWND a member of
> MessageChannel and include balanced CoInitialize/CoUninitialize calls
> there... my concern in that case would be mainly that CoInitialize can be an
> expensive call to make.

If CoInit has been called before on the same thread I'm pretty sure it's basically a no-op. All it does is up a counter.

I wouldn't expect the com window to go away as long as we have an open ended CoInit. You could add one of these down in widget or similar (with a matching uninit).
Comment 32 User image Aaron Klotz [:aklotz] 2015-03-31 10:14:49 PDT
(In reply to Ben Turner [:bent] (use the needinfo flag!) from comment #30)
> Comment on attachment 8585709 [details] [diff] [review]
> Part 2: Use SetWinEventHook to watch for the COM window (r2)
> 
> Review of attachment 8585709 [details] [diff] [review]:
> -----------------------------------------------------------------
> 

All comments fixed as requested except where noted.

> 
> ::: ipc/glue/WindowsMessageLoop.cpp
> @@ +104,5 @@
> >  HHOOK gDeferredGetMsgHook = nullptr;
> >  HHOOK gDeferredCallWndProcHook = nullptr;
> >  
> >  DWORD gUIThreadId = 0;
> >  HWND gCOMWindow = 0;
> 
> Nit: Rename to gOLEWindow maybe?

I thought about this for a bit and decided to standardize on "COM" in the variable/function names. All of our modifications in this bug now use "COM" for consistency.

> @@ +712,5 @@
> > +
> > +  if (!gWinEventHook) {
> > +    gWinEventHook = SetWinEventHook(EVENT_OBJECT_CREATE, EVENT_OBJECT_DESTROY,
> > +                                    NULL, &WinEventHook, GetCurrentProcessId(),
> > +                                    gUIThreadId, WINEVENT_OUTOFCONTEXT);
> 
> Hm, does it work if you pass 0 as the last param?

I looked at the header file for those flags, and it turns out that WINEVENT_OUTOFCONTEXT is #defined as 0 anyway.
Comment 33 User image Aaron Klotz [:aklotz] 2015-03-31 10:20:16 PDT
(In reply to Jim Mathies [:jimm] from comment #31)
> (In reply to Aaron Klotz [:aklotz] (please use needinfo) from comment #15)
> > I guess that one option is that we could make the COM HWND a member of
> > MessageChannel and include balanced CoInitialize/CoUninitialize calls
> > there... my concern in that case would be mainly that CoInitialize can be an
> > expensive call to make.
> 
> If CoInit has been called before on the same thread I'm pretty sure it's
> basically a no-op. All it does is up a counter.

Our concern was that from the various invocations that we found in the tree, it was unclear that there would always be a previous CoInitialize.

> 
> I wouldn't expect the com window to go away as long as we have an open ended
> CoInit.

This turns out to be a false assumption: we added some assertions in the initial patch to catch this scenario and they fired during testing on inbound. Hence the backout and my follow-up patch.

> You could add one of these down in widget or similar (with a
> matching uninit).

My follow up patch eliminates the need to tangle with CoInitialize, so we should be OK now. Thanks for the suggestions.
Comment 34 User image Aaron Klotz [:aklotz] 2015-03-31 10:22:34 PDT
Created attachment 8586209 [details] [diff] [review]
Part 2: Use SetWinEventHook to watch for the COM window (r3)

Patch including review comments. Carrying forward r+.
Comment 37 User image Jim Mathies [:jimm] 2015-04-19 05:23:30 PDT
*** Bug 1143395 has been marked as a duplicate of this bug. ***
Comment 38 User image Aaron Klotz [:aklotz] 2015-06-02 10:37:33 PDT
Comment on attachment 8583584 [details] [diff] [review]
Make Windows IPC play nicely with COM STA marshaling (r2)

Approval Request Comment
[Feature/regressing bug #]: NPAPI plugins on windows
[User impact if declined]: Plugin hangs particularly with accessibility or screen keyboards
[Describe test coverage new/current, TreeHerder]: Plugin code coverage, already landed on aurora and central.
[Risks and why]: Low, this has been baking on 40 for quite a while and our plugin hang situation is very good there.
[String/UUID change made/needed]: None
Comment 39 User image Aaron Klotz [:aklotz] 2015-06-02 10:37:51 PDT
Comment on attachment 8586209 [details] [diff] [review]
Part 2: Use SetWinEventHook to watch for the COM window (r3)

Approval Request Comment
[Feature/regressing bug #]: NPAPI plugins on windows
[User impact if declined]: Plugin hangs particularly with accessibility or screen keyboards
[Describe test coverage new/current, TreeHerder]: Plugin code coverage, already landed on aurora and central.
[Risks and why]: Low, this has been baking on 40 for quite a while and our plugin hang situation is very good there.
[String/UUID change made/needed]: None
Comment 40 User image Liz Henry (:lizzard) (needinfo? me) 2015-06-03 09:49:03 PDT
Marking 38+ as affected since this looks like it's been a regression since beta 37.
Comment 41 User image Liz Henry (:lizzard) (needinfo? me) 2015-06-03 09:50:22 PDT
Comment on attachment 8586209 [details] [diff] [review]
Part 2: Use SetWinEventHook to watch for the COM window (r3)

Approved for uplift to beta. This would be great to fix for 39!
Comment 42 User image Liz Henry (:lizzard) (needinfo? me) 2015-06-03 09:54:23 PDT
Comment on attachment 8583584 [details] [diff] [review]
Make Windows IPC play nicely with COM STA marshaling (r2)

Approved for uplift to beta.
Comment 44 User image Aaron Klotz [:aklotz] 2015-06-09 10:21:44 PDT
*** Bug 1165189 has been marked as a duplicate of this bug. ***
Comment 45 User image Catalin Varga [QA][:VarCat] 2015-06-18 08:06:00 PDT
Verified as fixed using:

FF 39.0b6
Build Id: 20150615125213
OS: Win 8.1 x86
Comment 46 User image Alice0775 White 2015-06-23 01:29:15 PDT
[Tracking Requested - why for this release]: hang see 1165189, it is worth to fix in esr38
Comment 47 User image Virtual_ManPL [:Virtual] - (ni? me) 2015-07-02 03:04:45 PDT
Any plans for uplifting these patches to ESR channel?
Comment 48 User image Aaron Klotz [:aklotz] 2015-07-02 09:55:37 PDT
Possibly. I'd like to let everything sit on 39 release for a bit before we do so.
Comment 49 User image Kate Glazko 2015-07-27 14:39:13 PDT
Hi, it has been 3 weeks- any plans for ESR?
Comment 50 User image Aaron Klotz [:aklotz] 2015-07-28 13:59:43 PDT
Comment on attachment 8583584 [details] [diff] [review]
Make Windows IPC play nicely with COM STA marshaling (r2)

We can do that, but they'll need to land alongside bug 1151318. They come as a package deal. :-)

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
Reduction in plugin (especially Flash) hangs.
User impact if declined:
Higher rate of flash hangs.
Fix Landed on Version:
Released on 39.
Risk to taking this patch (and alternatives if risky): 
Pretty minimal at this point. These patches have ridden the trains and have been doing well on release.
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Comment 51 User image Aaron Klotz [:aklotz] 2015-07-28 14:00:49 PDT
Comment on attachment 8586209 [details] [diff] [review]
Part 2: Use SetWinEventHook to watch for the COM window (r3)

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
Reduction in plugin (especially Flash) hangs.
User impact if declined:
Higher rate of flash hangs.
Fix Landed on Version:
Released on 39.
Risk to taking this patch (and alternatives if risky): 
Pretty minimal at this point. These patches have ridden the trains and have been doing well on release.
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Comment 52 User image Lawrence Mandel [:lmandel] (use needinfo) 2015-07-29 11:45:08 PDT
Comment on attachment 8583584 [details] [diff] [review]
Make Windows IPC play nicely with COM STA marshaling (r2)

This is a pretty severe Flash regression. We'll take this in ESR 38.2.0 - the first release that will see automatic updates from ESR31. ESR38+

aklotz - Can you please write a draft release note for this change?
Comment 53 User image Aaron Klotz [:aklotz] 2015-07-29 11:50:13 PDT
"Fixed an issue where Firefox would hang after right-clicking Flash content on Windows 8."
Comment 54 User image Lawrence Mandel [:lmandel] (use needinfo) 2015-07-29 11:58:42 PDT
Thanks Aaron. I've edited as

Fixed: Firefox may become unresponsive after right-clicking Flash content on Windows 8
Comment 56 User image Florin Mezei, QA (:FlorinMezei) 2015-08-05 08:26:40 PDT
Reproduced with specified steps in Firefox 38.0.5, on Windows 7 x64. The issue no longer reproduced using Firefox ESR 38.2.0 on Windows 7 x64, Windows 8.1 x64, and Windows 10 x64. Right-clicking the SWF, while tabtip was open, worked just fine.

Note You need to log in before you can comment on or make changes to this bug.