Firefox hangs and finally crashes when using Accessible Event Watcher(AccEvent) and flash

RESOLVED FIXED in mozilla22

Status

()

Core
Plug-ins
P3
critical
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: Tarun Ramaswamy, Assigned: tbsaunde)

Tracking

(Blocks: 1 bug, {access, hang, helpwanted})

14 Branch
mozilla22
x86
Windows 7
access, hang, helpwanted
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

5 years ago
Created attachment 651029 [details]
07a4a4ca-7dea-48b3-8efe-759c6032f7c1.dmp

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20100101 Firefox/14.0.1
Build ID: 20120713134347

Steps to reproduce:

Steps to reproduce:

1. Download and start Accessibility Event watcher (accevent).
2. Select Option -> Get Events in Context.
3. Open Options-> Settings...and select the following
	Objects list - everything
	Event Information list - everything
	Events list - everything
	object properties list - everything
	Use Invoke checkbox - unchecked
	Next 4 checkboxes - checked
	DebugBreak on next event  checkbox - unchecked "Don't filter" radio button selected
  Select OK to accept and close the dialog. The accevent would now start listening to all events

4. Launch Firefox and open any video in youtube.com
5. Launch another Firefox window and open www.fox.com or/and www.foxnew.com, http://timesofindia.indiatimes.com/
6. Repeat step 4 and 5 to see the hang and finally the crash.


Actual results:

The plugin-container.exe crashes.

The PluginInstanceChild::CreateWinlessPopupSurrogate() creates a window. It intially exchanges some messages using oleacc and finally hangs  at COMMessageFilter::MessagePending in the firefox. Due to the timeout of 45 sec in firefox, the plugin container finally crashes.


Expected results:

It should not crash.

Comment 1

5 years ago
Can you provide the crash ID from about:crashes?
(Reporter)

Comment 2

5 years ago
https://crash-stats.mozilla.com/report/index/66843eda-cec1-46d2-a16c-c6c8b2120811
and https://crash-stats.mozilla.com/report/index/bp-3f8b9e56-b23e-45ff-8f34-a82c12120811

Updated

5 years ago
Severity: normal → critical
Status: UNCONFIRMED → NEW
Crash Signature: [@ hang | mozilla::ipc::RPCChannel::EnteredCall() ]
Component: Untriaged → Plug-ins
Ever confirmed: true
Keywords: hang
Product: Firefox → Core

Comment 3

5 years ago
Benjamin, David, I was just notified of this bug by one of the devs at GW Micro. Seems like Window-Eyes is hitting this one frequently. Anything we can do to prevent the deadlock and subsequent crash?

Comment 4

5 years ago
I am very interested in solution of this problem as it is very critical for Window-Eyes users who try to browse pages containing Flash. If there is anything I could do to help you in finding quick solution please let me know...

Updated

5 years ago
Keywords: access

Comment 5

5 years ago
Has there been any movement on this issue? As mentioned, it is a critical problem for Window-Eyes uses. The hang takes down Firefox completely.

Thanks!

Comment 6

5 years ago
Benjamin would you mind to look at the crash or cc somebody who can help?
No, I really don't have time to look at this one. I'd love for somebody to diagnose it; in particular, you'll probably need to attach to all four processes at the time of the hang (Firefox, plugin-container, and the two FlashPlayerPlugin processes) and get stacks/minidumps from all of them. I'm working on a project to make the hang reports more useful and collect this info, but that's not done yet.

Email me privately if you're interested and I can provide some more information about Flash debugging.

Updated

5 years ago
Keywords: helpwanted
Hanging Window-Eyes users makes us sad.

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> ... in particular, you'll probably need to attach to all four
> processes at the time of the hang (Firefox, plugin-container, and the two
> FlashPlayerPlugin processes) and get stacks/minidumps from all of them. I'm
> working on a project to make the hang reports more useful and collect this
> info, but that's not done yet.

Any progress here?

Aaron, does this problem happen when only one FF window is open?

I'm also curious if setting about:config plugins.click_to_play is something that might help reduce frequency (although I'm not sure what the core problem is).

I just realized I haven't checked click_to_play accessibility itself yet. Marco can you try it?

Comment 9

5 years ago
For me it is enough to try to watch just any movie on youtube.com with Window-Eyes running.
OK Tarun, Darek, we need someone willing to recreate this on FF nightly or FF aurora:
http://nightly.mozilla.org/
http://www.mozilla.org/en-US/firefox/channel/#aurora

Then if you can check about:crashes (just type that into the location bar) and paste the topmost crash link into this bug (or email me) that would be a big step toward helping us diagnose this. Thanks!
(Reporter)

Comment 11

5 years ago
Hi David,

This is the link to the crash with the nightly build.
https://crash-analysis.mozilla.com/hang-reports/2012/11-13/hr-20121113-b80495fb-a5c6-46ee-bf6d-ade1ea8774cd.html

Crash ID: bp-hr-20121113-b80495fb-a5c6-46ee-bf6d-ade1ea8774cd

Thanks,
Tarun
Analysis:

A Flash instance is present.
* The browser has previously started to asynchronously paint it by sending AsyncSetWindow several times.
** The browser has now received stream data and is calling NPP_NewStream which is an sync (RPC) call.
* The plugin-container is currently processing the *first* AsyncSetWindow.
** It has called into Flash via NPP_SetWindow.
** Flash ends up calling NPN_GetValue for NPNVnetscapeWindow. This is an RPC call, so incoming asynchronous messsages reenter it.
** We re-enter RecvAsyncSetWindow for another async painting message. This method has protection against being called while there is a pending call (mPendingPluginCall) but this protection is not working, so we call into DoAsyncSetWindow again.
** We haven't yet finished the *first* DoAsyncSetWindow, so we end up creating the popup surrogate this time around in a highly nested stack...
** CreateWindow then ends up in event.dll (no symbols...) which enters a COM loop, presumably for some IAccessible, although the stack only mentions IDispatch.
** This ends up in COMMessageFilter::MessagePending which hackily flushes the RPC queue and this method appears to be ilooping (plugin-container is using 50% CPU, which is 100% of one core). 
* The Flash sandbox is waiting on RPC and doesn't appear to be involved.

This may just be a bug in RPCChannel::FlushPendingRPCQueue() and/or OnMaybeDequeueOne. Or it could be that the nested DoAsyncSetWindow cause the problem. It sounds like we should fix the nesting and investigate the other issue.

Comment 13

5 years ago
Hello! Thank you very much for looking at this problem!
For me, with Window-Eyes 7.9.0.8 running, it looks slightly different and finally I do not get the crash. 

When I go to YouTube site, search something and open the page with specific movie it starts buffering, but the circle in the middle of the picture stops animating and I loose access to FF. I can Alt-Tab to other applications running, but the speech produced by Window-Eyes is truncated after few words every time I move. When I come back to FF its window even does not repaint itself. The system can stay in this state for a long time, but FF does not crash.

So is there any way to create some report from this state?
Priority: -- → P2
Benjamin who would be a good candidate to take this bug? Is there work happening on related bugs?
I don't think there is anyone available to work on this bug right now: jmathies would be my first choice but I think his Metro work is more important.

Comment 16

5 years ago
checking in to see if there has been any movement on this.  thanks...
Priority: P2 → P3

Comment 17

5 years ago
With Firefox 18.0, and either Window-Eyes 8.0 or JAWS 14.0, when viewing a video on youtube.com, Firefox will freeze (especially when a Flash video and Flash ad show up on the same page at the same time). The same is also true using AccExplorer. This behavior makes using Window-Eyes unusable with Firefox and Flash. We know you guys have a lot going on, but we'd greatly appreciate it if somebody could really work on getting this bug fixed. I'm attaching a dump of the hung Firefox process per Marco's request.

Thanks.

Comment 18

5 years ago
Looks like the dump is too large to attach to this report, but it can be obtained from ftp://ftp.gwmicro.com/special/firefox_hang.zip.

Thanks.
As pointed out, this bug can easily be duplicated using Window-Eyes, JAWS, AccEvent, and AccExplorer.  Any program which traverses the MSAA tree section of Flash will likely cause Firefox to freeze up.  It is a time bomb waiting to go and with little effort it will blow causing the blind user total frustration.

There used to be a time that Firefox was well received because of its interest in accessibility. This is an unacceptable accessibility issue.  This bug clearly makes using Firefox with Flash impossible for anyone running any sort of serious AT product. Yet instead of seeing this as unacceptable, the priority of this bug has been reduced.

This bug is critical and needs to be prioritized as such.
Flash has been costing us a *lot* of pain.

Aaron, Doug I want to make sure you are aware of bug 785047 in case there is something there that might help or make you have insights into this bug that can help us uncover what is going on.

I know Benjamin wants to get to this but we have quite a list of flash interop issues. Have you filed mirror bugs with Adobe or are we confident this is a FF-side issue?

Comment 21

5 years ago
David,

We're confident in that Tarun Ramaswamy, from Adobe, posted this bug initially. While I realize that doesn't absolve them, it at least demonstrates they debugged it to the point of determining it was no longer an Adobe issue. Again, that doesn't mean it's not, but it's all we have to go on at the moment until we can get someone else to take a look. It seems that Benjamin was getting started back in Comment 12, but was unable to continue.

Aaron
(Reporter)

Comment 22

5 years ago
Hi David,

I did the initial investigation for this issue for our internal bug 3291796 and logged this bug. I would be happy to help if you guys need anything.

Thanks,
Tarun
Aaron, Tarun, thanks. Tarun, could you email me directly any investigation/info from your internal bug?
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #12)
> Analysis:

...

> ** We re-enter RecvAsyncSetWindow for another async painting message. This
> method has protection against being called while there is a pending call
> (mPendingPluginCall) but this protection is not working, so we call into
> DoAsyncSetWindow again.

Benjamin is there a related bug filed about the protection not working?
David,

Sorry for the slow response.  I see others have chimed in.  I feel for you with all the pain Flash has been causing.  But rest assured, it isn’t stopping with just you guys (smile).

I am not confident if this is just a Mozilla bug or if it is Adobe or a combination of both.  We went round and round with Adobe on this and they posted the original bug feeling this was out of their hands.  I’m happy to see Tarun is willing to work with you guys on this.  Thanks Tarun!

I’m not sure if this is an in or out of context issue.  Window-Eyes is certainly in context…I’m assuming JAWS is as well but of course don’t know for sure.  I can also duplicate this easily with the MSAA AccExplorer utility.  But again, I’m not sure if it is in or out.  My guess would be it is out of context.  But whether the FF hang comes from Window-Eyes, JAWS or AccExplorer if I kill the Flash player process using task manager, FF and the calling application work normally but of course FF puts up a message in the flash window saying “The Adobe Flash plugin has crashed.”  So this is a long way of saying I'm not sure if it is related to bug 785047.

But it is clear, just the act of traversing the MSAA tree (well, the Flash section) causes the deadlock.
Thanks Doug. These bugs are the trickiest. Tarun thanks of emailing me directly.
I can finally reliably reproduce this with Window-Eyes and Firefox nightly builds on a particular Youtube video that is guaranteed to give me a Flash player and not an HTML5 video one. I can no longer, however, reproduce seemingly related bug 599814. I see an immediate freeze now, like described in this bug, but no unknown accessibles, like described in that other bug initialy.
Marco, this is great news.  I've been using http://www.youtube.com/watch?v=8UVNT4wvIGY to duplicate this with Window-Eyes, JAWS and AccExplorer.  It seems to always have a second Flash ad so the combination of the main video and the ad bring it out reliably with all 3 applications.

David and Tarun, have the two of you made any progress on this?

Thanks everyone.
I can recreate exactly comment 0. I'm not sure of the fix yet.

Aside; Marco did you run into any issues with WE and CTP (click to play) mode? I ask because if we could do CTP on flash adds it would be a nice mitigation.
Window-Eyes is one of the ATs I tried which has problems clicking the current click-to-play snippet, which I filed bug 836746 for. So, no, this is currently not a way to mittigate this problem. The Flash simply can't be activated right now because of the markup we inject in place of the Flash object.
Blocks: 843657
I just tested Trevor's recent idea for a workaround (delaying attachment of plugins to the a11y tree) and it seemed to work. Assigning this bug to him to avoid the work around when it isn't needed and to improve the tree graft delay - possibly making it an about:config setting (using some doc ready event probably too fragile).
Assignee: nobody → trev.saunders
(Assignee)

Comment 32

4 years ago
Created attachment 719219 [details] [diff] [review]
hack around the issue

since the issue seems to have to do with setting up the window lets just try and avoid people poking it at it until its set up.

Alex I'd like you to look at this, but I'm not sure it needs to happen before this lands.
Attachment #719219 - Flags: review?(surkov.alexander)
Attachment #719219 - Flags: review?(dbolter)
Trevor did you fire this at try? I'd like Marco to test a build.

Comment 34

4 years ago
Comment on attachment 719219 [details] [diff] [review]
hack around the issue

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

::: accessible/src/base/nsAccessibilityService.cpp
@@ +208,5 @@
>  }
>  
> +#ifdef XP_WIN
> +static StaticAutoPtr<nsTArray<nsCOMPtr<nsIContent> > > sPendingPlugins;
> +static StaticAutoPtr<nsTArray<nsCOMPtr<nsITimer> > > sTimers;

what's benefits of allocation in dynamic memory vs static nsCOMPtr<nsIContent> >?

@@ +227,5 @@
> +    DocAccessible* doc = ps->GetDocAccessible();
> +    if (!doc)
> +      return NS_OK;
> +
> +    // make sure that if we created an accessible for the plugin that wasn't

nit: uppercase it, make -> Make

@@ +230,5 @@
> +
> +    // make sure that if we created an accessible for the plugin that wasn't
> +    // a plugin accessible we remove it before creating the right accessible
> +    doc->ContentRemoved(mContent->GetParent(), mContent);
> +    doc->ContentInserted(mContent->GetParent(), mContent, mContent);

Reinsertion doesn't seem working because aStartContent == aEndContent. Did somebody tested that actually?

btw, you need to add tree logging for these or use RecreateAccessible thing

@@ +232,5 @@
> +    // a plugin accessible we remove it before creating the right accessible
> +    doc->ContentRemoved(mContent->GetParent(), mContent);
> +    doc->ContentInserted(mContent->GetParent(), mContent, mContent);
> +    sTimers.RemoveElement(aTimer);
> +    sPendingPlugins.RemoveElement(mContent);

I would like to see a comment here that we should remove content after accessible recreation to avoid reentrancy.

@@ +235,5 @@
> +    sTimers.RemoveElement(aTimer);
> +    sPendingPlugins.RemoveElement(mContent);
> +    return NS_OK;
> +  }
> +          

nit: whitespaces on empty line

@@ +1062,5 @@
>    DocManager::Shutdown();
>  
> +#ifdef XP_WIN
> +  sPendingPlugins = nullptr;
> +  sTimers = nullptr;

don't you to take care to cancel timers?
Attachment #719219 - Flags: review?(surkov.alexander)
(Assignee)

Comment 35

4 years ago
(In reply to David Bolter [:davidb] from comment #33)
> Trevor did you fire this at try? I'd like Marco to test a build.

no, will later tonight though


(In reply to alexander :surkov from comment #34)
> Comment on attachment 719219 [details] [diff] [review]
> hack around the issue
> 
> Review of attachment 719219 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/base/nsAccessibilityService.cpp
> @@ +208,5 @@
> >  }
> >  
> > +#ifdef XP_WIN
> > +static StaticAutoPtr<nsTArray<nsCOMPtr<nsIContent> > > sPendingPlugins;
> > +static StaticAutoPtr<nsTArray<nsCOMPtr<nsITimer> > > sTimers;
> 
> what's benefits of allocation in dynamic memory vs static
> nsCOMPtr<nsIContent> >?

static constructors :(

> @@ +230,5 @@
> > +
> > +    // make sure that if we created an accessible for the plugin that wasn't
> > +    // a plugin accessible we remove it before creating the right accessible
> > +    doc->ContentRemoved(mContent->GetParent(), mContent);
> > +    doc->ContentInserted(mContent->GetParent(), mContent, mContent);
> 
> Reinsertion doesn't seem working because aStartContent == aEndContent. Did
> somebody tested that actually?

I'm not sure if David checked plugin actually showed up.  I thought I remembered us doing that other places when I wrote it, but I guess we always use foo, foo->GetNextSibling()

> btw, you need to add tree logging for these or use RecreateAccessible thing

I guess that would be nicer, though I'm not sure its "necessary"

> @@ +1062,5 @@
> >    DocManager::Shutdown();
> >  
> > +#ifdef XP_WIN
> > +  sPendingPlugins = nullptr;
> > +  sTimers = nullptr;
> 
> don't you to take care to cancel timers?

theoretically maybe, on the other hand you need to hold a ref to them yourself so they may just go away anyway, and even if they don't we don't reach this till xpcom-shutdown so it seems pretty unlikely they'll be able to run even if we ever get back to the main loop after this.  I guess I could do it to be safe or something but I really doubt it matters.
(Assignee)

Comment 36

4 years ago
Created attachment 719339 [details] [diff] [review]
patch 2
Attachment #719339 - Flags: review?(surkov.alexander)

Comment 37

4 years ago
Comment on attachment 719339 [details] [diff] [review]
patch 2

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

actually I'd want if somebody confirmed that this trick works (because the previous patch doesn't look working).

::: accessible/src/base/nsAccessibilityService.cpp
@@ +209,5 @@
>  }
>  
> +#ifdef XP_WIN
> +static StaticAutoPtr<nsTArray<nsCOMPtr<nsIContent> > > sPendingPlugins;
> +static StaticAutoPtr<nsTArray<nsCOMPtr<nsITimer> > > sTimers;

I think I would do them per document. Having hanging them for 10 sec when tab was open/closed doesn't seem nice
Attachment #719339 - Flags: feedback?(marco.zehe)

Updated

4 years ago
Attachment #719219 - Attachment is obsolete: true
Attachment #719219 - Flags: review?(dbolter)
(Assignee)

Comment 38

4 years ago
thanks for reminder https://tbpl.mozilla.org/?tree=Try&rev=04dbe5962df3
Build is here: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/trev.saunders@gmail.com-04dbe5962df3/try-win32/
I tried this try build, and found that it improves the situation somewhat. The complete freeze does no longer occur. However, there is still a lot of communication going on between JAWS and, presumably, Flash that causes the video to stutter, JAWS to react very slowly to keystrokes (like 3-5 seconds per stroke), and the CPU fan spinning and spinning instantly the video starts.

So while the situation is better as it's no longer hanging, it's also not pretty.

Comment 41

4 years ago
Marco, do you find the same with Window-Eyes?
(In reply to Aaron Smith from comment #41)
> Marco, do you find the same with Window-Eyes?

It's different. In Window-Eyes, the video starts, and I can navigate with the browse cursor up to the point where the Flash is supposed to be. The video then freezes for a moment, then resumes, and I don't actually see any Flash content added to the virtual buffer.

So if possible, can you download the try-server build linked to above and try yourself to see what result you get?
Marco, just to be clear, with Window-Eyes 8.1 we default to not providing any Flash content in the browse mode buffer.  Simply reading through the Flash tree caused Firefox to hang.  This was added in 8.1 so people could at least play Youtube videos.  You can either use a version prior to Window-Eyes 8.1 (which you may already be using) or I can tell you how to force 8.1 to load the Flash content.
Doug, I was using Window-Eyes 8.0, since that was the version I had reproduced the hang with before. However, knowing how I can force WE 8.1 to load the Flash content will still be helpful for my second install, which runs 8.1.
Marco, to enable Flash content in WE 8.1 simply do the following:

while Window-Eyes 8.1 is running and Firefox has focus simply bring up the Window-Eyes control panel (control-backslash).  Press alt-A for the apps pulldown menu.  Get to the Firefox Enhanced option and hit enter to open its pulldown.  In there select the Allow Flash in Browse Mode.  This will check the option allowing Flash content in browse mode.  This option is a toggle so you can just select it again to uncheck it.

Comment 46

4 years ago
Comment on attachment 719339 [details] [diff] [review]
patch 2

Marco, what is the status?
Attachment #719339 - Flags: review?(surkov.alexander)
I'd love to follow Doug's instructions, but Nightly, which is what I need to test it with, doesn't seem to be recognized by WE as Firefox. I don't have "Firefox enhanced" in the apps pull down. So my report stands that I gave over a week ago.
Marco, As long as the executable name is firefox.exe, it whould work.  But you can manually install the Window-Eyes Firefox Enhance app by going to: http://www.gwmicro.com/ac/firefox_enhanced and clicking the download now link and just run it, it will self install.
Like I said in my last report, with the current try-server build, Window-Eyes and Firefox no longer hang, but Window-Eyes 8.1 with the Allow Flash in browse mode setting still doesn't see the Flash content. When I right-arrow from the previous line, it dings once, then dings again as I leave the place where the Flash content should be.

But the hang is definitely gone.

Comment 50

4 years ago
Marco, did you make sure to wait 10 seconds (this number is hardcoded in the try build) after page load before trying the plugin?
(In reply to alexander :surkov from comment #50)
> Marco, did you make sure to wait 10 seconds (this number is hardcoded in the
> try build) after page load before trying the plugin?

Yes, I waited for over a minute actually to make sure the video was playing continuously and the fan didn't start spinning, before attempting to look for the Flash content.
I tested the build from comment 39 and used accprobe to make sure we do expose the video a11y tree. We do.
Comment on attachment 719339 [details] [diff] [review]
patch 2

Based on comment #52 and my own testing, f=me. The rest needs to be taken care of by AT vendors IMO.
Attachment #719339 - Flags: feedback?(marco.zehe) → feedback+

Comment 54

4 years ago
Marco, do all Windows screen readers/AT need this? For example, does it improve user experience for NVDA?
No, NVDA does not need this bug. It never was affected by it, because NVDA loads Flash in a totally different way. it does not try to embed the Flash content into its virtual buffer straight away, only exposes the HTML level object as a point to press Enter on and then interact with the Flash content in its own bubble. There's a key stroke to return to the web document from Flash (or Java, for that matter), so the whole problem never occurs with NVDA because it accesses Flash out of the context of the web content.

Comment 56

4 years ago
Ok, what AT needs that?
JAWS and WE definitely. I am not certain about System Access, Dolphin and Baum. Have never heard about Flash support at all in their products. But I'd guess for now, do it for JAWS and WE, and if others stumble upon it and complain, expand that blacklisting to them as well.
(Assignee)

Comment 58

4 years ago
Marco, Alex, where are we at here?

> ::: accessible/src/base/nsAccessibilityService.cpp
> @@ +209,5 @@
> >  }
> >  
> > +#ifdef XP_WIN
> > +static StaticAutoPtr<nsTArray<nsCOMPtr<nsIContent> > > sPendingPlugins;
> > +static StaticAutoPtr<nsTArray<nsCOMPtr<nsITimer> > > sTimers;
> 
> I think I would do them per document. Having hanging them for 10 sec when
> tab was open/closed doesn't seem nice

not sure what you want to change here

Comment 59

4 years ago
Trev, it'd be great to add two things
1) whitelist this behavior, i.e. enable it for JAWS and WE only
2) add a preference to change the delay in plugin creation
(Assignee)

Comment 60

4 years ago
Created attachment 729358 [details] [diff] [review]
bug 781971 - hack around plugin hangs
Attachment #729358 - Flags: review?(surkov.alexander)

Comment 61

4 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #58)

> > > +#ifdef XP_WIN
> > > +static StaticAutoPtr<nsTArray<nsCOMPtr<nsIContent> > > sPendingPlugins;
> > > +static StaticAutoPtr<nsTArray<nsCOMPtr<nsITimer> > > sTimers;
> > 
> > I think I would do them per document. Having hanging them for 10 sec when
> > tab was open/closed doesn't seem nice
> 
> not sure what you want to change here

I meant a timer doesn't get canceled when document gets shutdown
(Assignee)

Comment 62

4 years ago
(In reply to alexander :surkov from comment #61)
> (In reply to Trevor Saunders (:tbsaunde) from comment #58)
> 
> > > > +#ifdef XP_WIN
> > > > +static StaticAutoPtr<nsTArray<nsCOMPtr<nsIContent> > > sPendingPlugins;
> > > > +static StaticAutoPtr<nsTArray<nsCOMPtr<nsITimer> > > sTimers;
> > > 
> > > I think I would do them per document. Having hanging them for 10 sec when
> > > tab was open/closed doesn't seem nice
> > 
> > not sure what you want to change here
> 
> I meant a timer doesn't get canceled when document gets shutdown

doing that seems non trivial would you object to just removing the content when the timer fires so the length of time we stuff alive is bounded?  (I meantto do that in the patch but it seems I forgot)
(Assignee)

Comment 63

4 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #62)
> (In reply to alexander :surkov from comment #61)
> > (In reply to Trevor Saunders (:tbsaunde) from comment #58)
> > 
> > > > > +#ifdef XP_WIN
> > > > > +static StaticAutoPtr<nsTArray<nsCOMPtr<nsIContent> > > sPendingPlugins;
> > > > > +static StaticAutoPtr<nsTArray<nsCOMPtr<nsITimer> > > sTimers;
> > > > 
> > > > I think I would do them per document. Having hanging them for 10 sec when
> > > > tab was open/closed doesn't seem nice
> > > 
> > > not sure what you want to change here
> > 
> > I meant a timer doesn't get canceled when document gets shutdown
> 
> doing that seems non trivial would you object to just removing the content
> when the timer fires so the length of time we stuff alive is bounded?  (I
> meantto do that in the patch but it seems I forgot)

only in the cases we can't recreate the accessible of course
(Assignee)

Comment 64

4 years ago
Created attachment 729855 [details] [diff] [review]
bug 781971 - hack around plugin hangs

remove the content from the pending plugins list when the timer fires if the doc it is in is nolonger accessible
Attachment #719339 - Attachment is obsolete: true
Attachment #729358 - Attachment is obsolete: true
Attachment #729358 - Flags: review?(surkov.alexander)
Attachment #729855 - Flags: review?(surkov.alexander)

Comment 65

4 years ago
(In reply to Marco Zehe (:MarcoZ) from comment #57)
> JAWS and WE definitely. I am not certain about System Access, Dolphin and
> Baum. Have never heard about Flash support at all in their products. But I'd
> guess for now, do it for JAWS and WE, and if others stumble upon it and
> complain, expand that blacklisting to them as well.

Marco, btw, can you see any problems with JAWS running and dom.ipc.plugins.enabled.a11y pref turning on (without the patch)?
Yup it hangs just the same like Window-Eyes does with my test video.

Comment 67

4 years ago
Comment on attachment 729855 [details] [diff] [review]
bug 781971 - hack around plugin hangs

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

::: accessible/src/base/nsAccessibilityService.cpp
@@ +213,5 @@
>  }
>  
> +#ifdef XP_WIN
> +static StaticAutoPtr<nsTArray<nsCOMPtr<nsIContent> > > sPendingPlugins;
> +static StaticAutoPtr<nsTArray<nsCOMPtr<nsITimer> > > sTimers;

nit: sPluginTimers? to point it's plugin related

@@ +215,5 @@
> +#ifdef XP_WIN
> +static StaticAutoPtr<nsTArray<nsCOMPtr<nsIContent> > > sPendingPlugins;
> +static StaticAutoPtr<nsTArray<nsCOMPtr<nsITimer> > > sTimers;
> +
> +class TimerCallBack MOZ_FINAL : public nsITimerCallback

nit: maybe PluginTimeCallback?

@@ +236,5 @@
> +        return NS_OK;
> +      }
> +    }
> +
> +    //We couldn't get a doc accessible so presumably the document went away.

nit: space before We

@@ +238,5 @@
> +    }
> +
> +    //We couldn't get a doc accessible so presumably the document went away.
> +    // In this case don't leak our ref to the content.
> +    sPendingPlugins->RemoveElement(mContent);

don't you need to remove a timer instance as well?

@@ +276,5 @@
> +      sPendingPlugins->AppendElement(aContent);
> +      return nullptr;
> +    }
> +
> +    // we need to remove aContent from the pending plugins here to avoid

nit: uppercase we

@@ +277,5 @@
> +      return nullptr;
> +    }
> +
> +    // we need to remove aContent from the pending plugins here to avoid
> +    // reentrancy.  when the timer fires it calls

nit: extra space before when, uppsercase when

::: modules/libpref/src/init/all.js
@@ +330,5 @@
> +#ifdef XP_WIN
> +// Some accessibility tools poke at windows in the plugin process during setup
> +// which can cause hangs.  To hack around this set accessibility.delay_plugins
> +// to true, you can also try increasing accessibility.delay_plugin_time if your
> +// machine is slow and you still experience hangs.

it'd be good to refer to this bug here.
Attachment #729855 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 68

4 years ago
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/aadfaa02374a
https://hg.mozilla.org/mozilla-central/rev/aadfaa02374a
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22

Comment 70

4 years ago
Where can I download the build with the fix? The link above doesn't work anymore.

Comment 71

4 years ago
just try nightly build (http://nightly.mozilla.org/)

Comment 72

4 years ago
I just tried nightly build, and it still hangs when I try to play any Youtube video. If I kill flashplugin from the Windows task manager, it unfreezes. I use Jaws 14.

Comment 73

4 years ago
Marco, can you see the same?
Flags: needinfo?(marco.zehe)
Unfortunately not. :(
Flags: needinfo?(marco.zehe)

Comment 75

4 years ago
(In reply to Michael Smith from comment #72)
> I just tried nightly build, and it still hangs when I try to play any
> Youtube video. If I kill flashplugin from the Windows task manager, it
> unfreezes. I use Jaws 14.

Did you try to play with preferences? Does a delay number make a difference?

Comment 76

4 years ago
How can I change the preference? about:config? Which one?

Comment 77

4 years ago
yes, accessibility.delay_plugins and accessibility.delay_plugin_time

Comment 78

4 years ago
To what value?

Comment 79

4 years ago
just play with them, I don't have a ready recipe. 'true' and '10000' values were supposed to work in most cases but it seems not on your machine.

Comment 80

3 years ago
I'm still having this issue. I've basically disabled flash in FF & activate it when I want to play a video. Sometimes it works--often it crashes. I can end the plugin container process in Task Manager when it does crash, but Firefox will usually then crash shortly (& I mean very shortly) thereafter. It's really highly annoying.

Comment 81

3 years ago
Did you try to play with preferences as described in comment 75 - comment 79? It doesn't help? Can you give crash reports?
You need to log in before you can comment on or make changes to this bug.