Closed Bug 687610 Opened 13 years ago Closed 13 years ago

[OOPP] [10.6] [10.7] QuickTime hangs FF while trying to play a sound from the plugin process

Categories

(Core Graveyard :: Plug-ins, defect)

Product:
Core Graveyard
Component:
Plug-ins
Version:
7 Branch
Platform:
x86_64
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox8-, firefox9 fixed)

Status:
VERIFIED FIXED
Milestone:
mozilla9
Tracking Flags:
Tracking Status
firefox8 - ---
firefox9 --- fixed

People

(Reporter: stormy, Assigned: smichaud)

Details

(Whiteboard: [inbound][qa!] STR in comment #31)

Attachments

(5 files, 3 obsolete files)

QuickTime dialog that makes Firefox freeze
42.56 KB, image/png
Details
Sample of Firefox with QT dialog up
40.78 KB, text/plain
Details
Sample of plugincontainer with QT dialog up
29.87 KB, text/plain
Details
Fix rev2 (follow bsmedberg's suggestions)
10.42 KB, patch
jaas
: review+
benjamin
: review+
akeybl
: approval-mozilla-aurora+
akeybl
: approval-mozilla-beta-
Details | Diff | Splinter Review
What Chrome does in the same case
32.65 KB, image/png
Details 
Firefox 7 beta hangs every time the QuickTime dialog comes up. 

I can hit cancel on the dialog and Firefox resumes again. However, for some reason, the QuickTime dialog is often behind my Firefox window. The only way for me to find it is to either manually move the Firefox window (tabbing through active processes doesn't find it) or to Force Quit on Firefox.

I've attached a copy of the QuickTime dialog.

I'm in Mountain View and happy to show any one the next time it happens.
To be clear, I don't think the dialog box is the problem. It's the fact that it shows up behind the active window and firefox won't do anything until it's closed.
This happens on Zimbra FYI but I've never seen the dialog cause Firefox to hang...but I probably instinctively hit cancel.
Component: General → Plug-ins
Product: Firefox → Core
QA Contact: general → plugins
It doesn't really hang Firefox because if you close the dialog, Firefox still works. The problem is that the dialog doesn't pop up. It appears behind the active Firefox window, so you don't even know it's there. So it looks like Firefox hangs.
Anyone have a way to reproduce this?

Stormy,	it would help a	lot if you gave	examples of sites that trigger
this dialog (whether or	not you	can get	the dialog to consistently pop
up behind Firefox).

(In reply to comment #2)

> This happens on Zimbra FYI but I've never seen the dialog cause
> Firefox to hang

Is the Zimbra issue QuickTime-related?  Can you reproduce it?
Summary: Firefox hangs because of hidden QuickTime dialog → Firefox appears to hang because of hidden QuickTime modal dialog
I had one this morning that was either Gmail or Zimbra. I'll watch for the next one.
Zimbra calendar alerts. I'll get exact steps to reproduce when I get in the office
> Zimbra calendar alerts.

Interesting.  Now that you mention it, I've seen "hangs" with these, but only on Linux, and not because they show up behind the browser window.
Just tested it and Zimbra calendar alert did not pop up the dialog for me.
> Just tested it and Zimbra calendar alert did not pop up the dialog for me.

What does this mean? :-)

Which dialog?  The Zimbra calendar alert itself?  The QuickTime one?

Have you ever seen the QuickTime dialog in Zimbra?
(In reply to Steven Michaud from comment #9)
> > Just tested it and Zimbra calendar alert did not pop up the dialog for me.
> 
> What does this mean? :-)

Sorry. I set a meeting in Zimbra and waited until it notified me of the meeting with a calendar alert box (that stays in the tab.) Firefox did not hang and the QuickTime dialog did not show up.
> 
> Which dialog?  The Zimbra calendar alert itself?  The QuickTime one?
> 
> Have you ever seen the QuickTime dialog in Zimbra?

No. The QuickTime dialog always shows up behind my current Firefox window. As a separate window.
(In reply to Steven Michaud from comment #4)
> Anyone have a way to reproduce this?
> 
> Stormy,	it would help a	lot if you gave	examples of sites that trigger
> this dialog (whether or	not you	can get	the dialog to consistently pop
> up behind Firefox).
> 

The QuickTime dialog popped up. The active tab in Firefox was Gmail (which was taking a long time). I was actually using a different app when it happened.
(In reply to Steven Michaud from comment #4)
> Anyone have a way to reproduce this?
> 
> Stormy,	it would help a	lot if you gave	examples of sites that trigger
> this dialog (whether or	not you	can get	the dialog to consistently pop
> up behind Firefox).
> 

The QuickTime dialog popped up. The active tab in Firefox was Gmail (which was taking a long time). I was actually using a different app when it happened.

There was an active appointment reminder alert in Zimbra too but it had been there for a while. (And the browser was still working with the Zimbra alert.)
I can reproduce this generally at will.

1. Go to zimbra
2. Click calendar
3. Get tons of alerts because I never use the web interface
4. Click in the HTML content dialog that pops up
5. Window pops up and Fx beachballs

Attaching a sample.
> 4. Click in the HTML content dialog that pops up

What's this?  Is it a QT dialog?  Does it have anything to do with Zimbra?  If not where does it come from?

I haven't yet tried to reproduce.  I'll need to create a meeting and let it's time pass by.

And by the way, what versions of FF and OS X are you using?
>> 4. Click in the HTML content dialog that pops up
>
> What's this?

After setting up a test meeting for a few minutes in the future (with myself as the only attendee, and no location or resources), I log into Zimbra and click on Calendar.  After a few seconds I see an "Appointment Reminders" dialog, with various options (including Open, Dismiss, Snooze and Dismiss All).  Is this the "HTML content dialog" you see?

I can click anywhere in it without causing any trouble.

I don't see any QT dialog.
Wild guess -- have you and Stormy somehow set up QuickTime to play the "ding" sound that you hear shortly after the Appointment Reminders dialog appears?
By the way, I've been testing with FF 6.0.2 on OS X 10.7.1.
Do you and Stormy see this bug with a clean profile?
I can reproduce it on Nightly and Firefox 6 and seem to remember it going back at least to 4. I haven't checked 3.6 yet. It is set up to play a sound when new mail or alerts pop up (though I believe that is the default) and my mail skin is the "gmail" one.

For some reasons my steps aren't 100%...I'll if I can get better ones.
> the "ding" sound that you hear shortly after the Appointment
> Reminders dialog appears

Playing around some more, I only hear the "ding" when the Appointment
Reminders dialog appears for a meeting that I'm late to (whose
starting time is in the past).

Still no problems, though -- now testing with today's mozilla-central
nightly (on OS X 10.7.1 as before).

Do test with a clean profile, to see if it makes any difference.

This will probably undo your Zimbra settings -- since they're probably stored in cookies.

But I strongly suspect that this bug doesn't occur with the default Zimbra settings (which is what I'm using).
I can confirm that this happens with the default settings, at least the ones we use at Mozilla. (Or perhaps they were changed recently.)  I had to disable the sound notifications to fix this.  Regardless, we shouldn't be hanging the main UI on this, and the modal dialog shouldn't be in the background, right?
> I can confirm that this happens with the default settings, at least
> the ones we use at Mozilla.

Thanks for the info.  But (out of curiosity) how do you	know what the
default settings are?

Did you	also test with a clean profile?

> I had to disable the sound notifications to fix this.

I don't understand.  Are you saying that disabling the sound
notifications stops the bug from happening?

> Regardless, we shouldn't be hanging the main UI on this, and the
> modal dialog shouldn't be in the background, right?

Of course not.  But we need to know how to reproduce this bug before
we can fix it (or work around it if it's someone else's bug).
Whoever's seen this bug:

Let us know what version of OS X you saw it on.

Let us know which version of Firefox you used.

Let us know the results of testing with a clean profile.
(Following up comment #23)

>> I had to disable the sound notifications to fix this.
>
> I don't understand.  Are you saying that disabling the sound
> notifications stops the bug from happening?

Also tell me how, exactly, to disable sound notifications.
I've never changed the default Mozilla Zimbra settings. I'm pretty sure it's this option that causes the problem:
		Play a sound (requires QuickTime or Windows Media plugin)
I'm now testing on OS X 10.6.8, where I've managed to see the QuickTime dialog from comment #0 once.  But I'm still struggling.

I dismissed that dialog by selecting Cancel (after waiting a minute or two for complicated reasons).  I haven't seen it again.

I assume the people who see this bug see that dialog more than once.  Is that true?

Once again, I'd like to hear which versions of OS X people see this bug with.  Anyone see it on OS X 10.7?

Finally, I suspect this bug is an OOPP bug, and will go away in 32-bit mode (where the QuickTime plugin runs in-process).  People who can consistently reproduce this bug, please test in 32-bit mode:

1) Right-click (or ctrl-click) on your FF distro.
2) Choose Get Info.
3) Check "Open in 32-bit mode".
4) Run your FF distro.
I'm on 10.6.8.

I see (or don't see, as it's hidden) this dialog multiple times a day. I'm guessing it's related to how often the Zimbra meeting alert dialog comes up and I usually have 8-12 events a day on my calendar.

I will try the 32 bit mode.
Thanks, Stormy, for the info -- particularly for telling me that
you're on OS X 10.6.8.

> I see (or don't see, as it's hidden) this dialog multiple times a
> day.

Have you actually *seen* it more than once?

I'm beginning to wonder if it actually triggers the hang, or if the
hang has some other cause.

I find that on OS X 10.6.8 (unlike on OS X 10.7.1) I never hear the
alarm sound (what I called the "ding").  And I find that the QuickTime
plugin always gets loaded (in its own plugin-container process) when I
should have heard the ding.

But I haven't seen the dialog from comment #0 more than once.  And I
still haven't experienced a hang ... or anything like it.

> I will try the 32 bit mode.

Thanks!
I get the QuickTime dialog multiple times a day. (The one I included a screen shot of.) To see it and hit cancel, I have to move my browser window aside. 

Once I hit cancel on the QuickTime dialog, my browser works again.

(When I said "or don't see it", I just meant that I have to go looking for it.)
OK, I've now made some progress.  I'm also now quite sure this is an
OOPP bug, and very possibly a QuickTime bug (in a later comment I'll
say more about why).  It doesn't seem to happen on OS X 10.7 (Lion).
It does happen on OS X 10.6.  Later I'll test on OS X 10.5.

The STR for this bug is *incredibly* finicky.  Which isn't to say that
it isn't often encountered -- just that almost nobody who encounters
it will be able to figure out exactly how to reproduce it.

1) Create an "appointment" in Zimbra calender to test with.  The start
   time must be at least 5 minutes in the future (or the "Appointment
   Reminders" dialog won't appear).  The length must also exceed a
   minimum -- I don't know how long exactly, but I've been testing
   with 30 minute "appointments".

   a) Log on to Zimbra, select the Calendar tab, and choose New
      Appointment.

   b) For "Subject" type some arbitrary value (I've been using "Test").

   c) Under "Show as" select "Free".

   d) Under "Mark as" select "Private".

   e) Under "Attendee" put your mozilla.com account name.

   f) Under Start and End, make the start time at least five minutes
      in the future, and accept the default length of 30 minutes.

   g) "Save" the appointment, and quickly log out.

) Under ~/Library/Preferences, delete the "QuickTime Preferences"
   file.  Then restart your computer.

   If you don't do this, then the QuickTime "additional software"
   dialog will appear at most once every few hours.

3) Wait until at least a minute after your test appointment has
   started.  Then run FF in 64-bit mode, log on to Zimbra, and wait
   for the "Appointment Reminders" dialog to appear, then 5-10 more
   seconds for QuickTime to load and fail to play the Zimbra alarm
   sound (the "ding").

   Firefox doesn't freeze immediately.  For example you can still
   navigate the FF menus.  But any number of actions can cause FF to
   freeze.  And (interestingly) I've found that the freeze only lasts
   about 30 seconds.

   Here are two ways to freeze:

   a) Click on the Desktop.  The QuickTime "additional software"
      dialog appears.

      It disappears if you wait for about 30 seconds.  Or you can
      click Cancel in this dialog to dismiss it.  If so you don't get
      a freeze.

      But if you don't dismiss the dialog or wait for it to close,
      Firefox will freeze as soon as you try to restore its app focus
      (for example by clicking on the browser window).  The freeze
      will end when the QuickTime dialog times out.

   b) Choose "About Firefox".  The QuickTime "additional software"
      dialog appears.

      Once again it disappears if you wait for 30 seconds, or you can
      click Cancel to dismiss it.  The About Firefox dialog appears as
      soon as the QuickTime dialog disappears, and you don't get a
      freeze.

      But if you click on the browser window Firefox freezes, and
      stays frozen until the QuickTime dialog times out.
Whiteboard: STR in comment #31
If you run FF in 32-bit mode and try my STR from comment #31 (on OS X
10.6.8), you don't see any problems at all -- the alarm sound (the
"ding") plays, and QuickTime doesn't think it needs to install any
additional software.

In other words,	QuickTime can play the "ding" without any additional
software -- so it's a bug (of some kind) that it fails to play it,
then goes on to	display	that dialog.

I suspect that QuickTime on OS X 10.6 (though not on OS X 10.7) has
trouble playing media (at least audio media) while running in a
background process.  This may be QuickTime's bug.  But it may also be
possible that we can do something to work around it.

Benjamin, what do you think?
Summary: Firefox appears to hang because of hidden QuickTime modal dialog → [OOPP] [10.6] QuickTime hangs FF while trying to play a sound from the plugin process
> Later I'll test on OS X 10.5.

There's not much point, since we don't use 64-bit mode (or OOPP) on OS X 10.5.
Another thing:  I wonder if, by displaying the "additional software" dialog, QuickTime brings the plugin process to the foreground, and if this (at least indirectly) causes the hang.
I don't have the problem if I run in 32 bit mode. 

Switched to 32 bit mode, ran Firefox through several meetings with alerts, no problem. Switched back to 64 bit mode, first meeting produced a Quicktime dialog.
> I suspect that QuickTime on OS X 10.6 (though not on OS X 10.7) has
> trouble playing media (at least audio media) while running in a
> background process.

I wonder if it's only *some* kinds of media it has this trouble with
-- media that it normally plays using some kind of "external" software
(which QuickTime prompts you to install if it thinks it's missing).
So maybe it's this "external" software (though not so "external" that
QuickTime doesn't know how to install it) that doesn't work properly
when run from a background process.
We have custom code to detect when the plugins tries to show a window, we interpose the call and forward it to the browser process for it to focus the plugin dialog. Looks like we're failing to handle this sequence.

Relevant code is dom/plugins/ipc/PluginInterposeOSX.mm.
Quicktime directs you to here if you click continue:

http://www.apple.com/quicktime/resources/components.html?os=OSX&ctype=71746578&csubtype=ff9c8000

Attaching a sample from the plugincontainer process
(In reply to comment #37)

> We have custom code to detect when the plugins tries to show a
> window, we interpose the call and forward it to the browser process
> for it to focus the plugin dialog. Looks like we're failing to
> handle this sequence.
>
> Relevant code is dom/plugins/ipc/PluginInterposeOSX.mm.

I'd forgotten about that.

Do you want to dig into it, Benoit?  Or shall I do it?
I'm looking into a lot of issues for Mobile OGL Layers currently. I could find some time for it but it wont be this week.
(In reply to comment #38)

> Quicktime directs you to here if you click continue:
>
> http://www.apple.com/quicktime/resources/components.html?os=OSX&ctype=71746578&csubtype=ff9c8000

I don't have any of the items installed that are listed at this URL,
and QuickTime has no trouble on my system in 32-bit mode.  So I think
that even in 64-bit mode, QuickTime must not be trying and failing to
use any of these programs.

So QuickTime's use of the "additional software" dialog when running
out-of-process is (presumably) on more example of a false or
misleading error message :-)
(In reply to comment #41)

> I'm looking into a lot of issues for Mobile OGL Layers currently. I
> could find some time for it but it wont be this week.

OK.  This is probably important enough for me to spend the next few
days looking for a fix/workaround.
Since my STR from comment #31 is awkward and slow, I'll be looking for a reduced testcase.  For that it'll probably help to look at the Zimbra source code.

Anyone know what versions of which Zimbra products we're running?
FYI, the "Update Adobe Flash" dialog appears correctly on top of the active browser window.
I haven't had much time to work on this in the last couple of days, and I'm going to be out Friday -- so I'll keep working on this next week.

I've discovered one interesting thing, though:

Zimbra's alert sound (what I've been calling the "ding") won't play in 64-bit mode even when loaded directly (by visiting the following URL).  You also don't see the QuickTime GUI for playing a sound.  It works fine in 32-bit mode, though.

https://mail.mozilla.com/zimbra/public/sounds/im/alert.wav

Anyone know of other examples of QuickTime media that play in 32-bit mode but not in 64-bit mode?
> Anyone know of other examples of QuickTime media that play in 32-bit
> mode but not in 64-bit mode?

Anyone know of other examples of media that QuickTime can play in
32-bit mode but not in 64-bit mode?
Josh, does any of this ring any bells?  Particularly what I have to say in comment #46?
(In reply to Steven Michaud from comment #48)
> Josh, does any of this ring any bells?  Particularly what I have to say in
> comment #46?

No, sorry.
(Following up comment #46)

The plot thickens, again:

> https://mail.mozilla.com/zimbra/public/sounds/im/alert.wav

In 64-bit mode, this sound only fails to play if alert.wav is already
in the network cache.  If you first clear the cache (Preferences :
Advanced : Network : Clear Now), then visit the URL, the sound plays
just fine.
(In reply to Steven Michaud from comment #47)
> > Anyone know of other examples of QuickTime media that play in 32-bit
> > mode but not in 64-bit mode?
> 
> Anyone know of other examples of media that QuickTime can play in
> 32-bit mode but not in 64-bit mode?

There's a whole host of them, especially when you add 3rd party 32-bit stuff like perian. When Apple switched over to Quicktime X they dropped support for a bunch of 32-bit only stuff and formats, which is why when you hit that media it (should) give a dialog and offer to download the old quicktime through SU. I was on that project but can't remember the specifics anymore :-)
(In reply to comment #51)

Thanks, Chris.  And thanks in advance if you can remember (and post)
links to some examples.

But evidence (particularly comment #50) is building that this is
largely (or entirely) an FF bug, so these examples are probably
irrelevant.
This bug is actually two different bugs	(both specific to OS X
10.6.X):

1) Firefox/QuickTime can't play at least some *.wav files from the
   network cache in 64-bit mode.  This may be identical to bug 670036.

2) When QuickTime can't play a file, it sometimes displays an
   "additional software" dialog.  In 64-bit mode this dialog can hang
   the browser process.

Both need to be fixed.  But for now I'm paying more attention to #1,
because it seems to be more fundamental.
This bug grows stranger by the hour.

I've now found that you can "fix" it (fix the problems playing *.wav
files in 64-bit mode) by preventing the use of NPP_StreamAsFile (by
changing *stype to NP_NORMAL on return from NPP_NewStream whenever the
QuickTime plugin sets it to NP_ASFILE or NP_ASFILEONLY).

Here's a patch that does this.  A tryserver build should follow by
tomorrow morning.

I *don't* think this patch is a viable fix -- unless we're truly
desperate and can't find anything better.  Rather we should try to
find out *why* NPP_StreamAsFile sometimes doesn't work properly -- at
least with the QuickTime plugin.
bsmedberg, cjones:

Any idea why my patch from comment #54 "works"?

Any idea what's wrong with how we call (what we do for) NPP_StreamAsFile?
See bug 644684 and 644716 for possibly related bugs. There is an async race condition that may be affecting quicktime in this case.
Attached patch Possible fix (obsolete) — — Splinter Review 
Here's a patch that (in my tests) fixes this bug.  Or more precisely
it fixes problem #1 of comment #53.  It also fixes bug 670036 and
(probably) bug 644684.

It does this by making our behavior conform to the following statement
from MDN's doc on NPP_StreamAsFile()
(https://developer.mozilla.org/en/NPP_StreamAsFile):

"When the stream is complete, the browser calls NPP_StreamAsFile to
provide the instance with a full path name for a local file for the
stream."

The QuickTime plugin can't play *.wav files when we call its
NPP_StreamAsFile() method "too early" -- before we're finished calling
NPP_WriteReady() and NPP_Write() on the same stream.  For some reason
we only do this when we replay a *.wav file from the browser's network
cache (not when we "play" it from its original location).

At first I thought that	calling NPP_StreamAsFile() "too early" means
its file isn't ready (the one whose name we pass in the fname
parameter).  But additional logging shows this isn't true -- the file
is always the correct length, and can be opened (and closed) without
trouble.

Instead it seems that the QuickTime plugin gets confused if we call
NPP_StreamAsFile() before we finish calling NPP_WriteReady() and
NPP_Write.

The QuickTime plugin never opens or reads any files on calls to
NPP_StreamAsFile() (only during calls to NPP_Write()).  And (as we've
seen from my earlier patch in comment #54), it works just fine if we
*never* call NPP_StreamAsFile() (only NPP_WriteReady() and
NPP_Write()).  But it must get some information from our calls to
NPP_StreamAsFile(), which causes it to misbehave if those calls happen
"too early".

Other plugins than QuickTime may not care that we sometimes call
NPP_StreamAsFile() at the wrong time.  But the QuickTime plugin has a
significant number of users, and our current behavior does contradict
our own documentation.  So I think we should fix this problem.

I'm doing a tryserver run on all platforms.  The results should be
ready by tomorrow morning.

We still have serious problems with the QuickTime plugin in current
trunk code (at least on OS X), which my patch doesn't fix.  But they
happen without my patch, and it doesn't make them any worse.
Assignee: nobody → smichaud
Attachment #564437 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #565345 - Flags: review?(benjamin)
This bug (both parts from comment #53) is reproducible on OS X 10.7, but for some reason not as reliably as on OS X 10.6.
Summary: [OOPP] [10.6] QuickTime hangs FF while trying to play a sound from the plugin process → [OOPP] [10.6] [10.7] QuickTime hangs FF while trying to play a sound from the plugin process
There don't seem to have been any non-spurious test failures in my tryserver run.

Here are the builds for all platforms:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-fa3385a54298/

And here's the OS X build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-fa3385a54298/try-macosx/fennec-10.0a1.en-US.mac.dmg

Please try it/them out, and let us know your results!
Comment on attachment 565345 [details] [diff] [review]
Possible fix

This approach is unfortunately insufficient: the problem is that the parent may delete the file as soon as NPP_StreamAsFile (or NPP_DestroyStream) returns, so we cannot deliver that notification asynchronously unless we also keep the file alive longer than normal. We can fix this by holding the nsPluginStreamListenerPeer alive even after NPP_DestroyStream is called on the parent side. I think we can probably do this by exposing the following API from nsNPAPIPlugin.h:

void RetainStream(NPStream* pstream, nsISupports** retained) and calling this from BrowserStreamParent::StreamAsFile, and saving the retained object until BrowserStreamParent::RecvStreamDestroyed.

I'd really really like an automated test for this behavior.
Attachment #565345 - Flags: review?(benjamin) → review-
Comment on attachment 565345 [details] [diff] [review]
Possible fix

Thanks for your suggestion.  I'll revise my patch.
(Following up comment #59)

> We still have serious problems with the QuickTime plugin in current
> trunk code (at least on OS X), which my patch doesn't fix.  But they
> happen without my patch, and it doesn't make them any worse.

See bug 692759.
How's this?

Once again I'm doing a full tryserver run, the results of which should be available tomorrow morning.

I'll start working on the tests next week.
Attachment #565345 - Attachment is obsolete: true
Attachment #565679 - Flags: review?(benjamin)
Once again, there don't seem to have been any non-spurious test failures for my patch from comment #65:

Here are the tryserver builds for all platforms:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-e414b35bede0/

And here's the OS X tryserver build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-e414b35bede0/try-macosx64/firefox-10.0a1.en-US.mac.dmg
Comment on attachment 565679 [details] [diff] [review]
Fix rev1 (follow bsmedberg's suggestions)

I really don't think that the helper method StreamAsFilePending is necessary. Can you just put that logic directly into RecvNPP_StreamAsFile? Also the const_cast is unnecessary and wrong, so please remove it.

On the parent side, is there any reason why mStreamPeer shouldn't be a nsCOMPtr<nsISupports>? Then you don't have to initialize it explicitly or deal with the manual refcounting, and ReleaseOurStream can be removed and you can just use mStreamPeer = NULL instead.

Note that since we ignore the result of RetainOurStream, you should either just remove the helper method entirely or make it return void.

r=me with those changes, thanks for taking this! Please get josh to at least agree to the approach once you've uploaded the final patch.
Attachment #565679 - Flags: review?(benjamin) → review+

        Attachment #565679 -
        Attachment is obsolete: true

        Attachment #565993 -
        Flags: review?(joshmoz)

        Attachment #565993 -
        Flags: review?(benjamin)

    
  

        Attachment #565993 -
        Flags: review?(benjamin) → review+

    
    

    
  
I'm having trouble designing an automated test for this bug.

It should be reasonably straightforward to test that no call to
NPP_Write() or NPP_WriteReady()	occurs on a stream after
NPP_StreamAsFile() has been called on it (though this will probably
require changes to the test plugin).

But I'm having trouble figuring out how to test that we	don't have the
race condition described in bug 644684 comment #6. 

Any suggestions, bsmedberg?

    
    

    
  
Do you know what sequence of messages led to the race in the first place? Let's assume that it's something like this:

->NPP_Write (async)
->NPP_Write (async)
->NPP_StreamAsFile (RPC)

  ->Receive all three message "at once": the first two post the BrowserStreamChild::Deliver event, the third is processed immediately

So basically to reproduce this, the plugin needs to:

* request the data
* sleep so that all the data is delivered "at once"
* check the ordering invariants
* make sure that the plugin file actually exists and has the correct contents

See http://mxr.mozilla.org/mozilla-central/source/dom/plugins/test/testplugin/nptest.cpp#3024 for an existing test where we sleep to trigger a potential race condition. (Used by mochitest http://mxr.mozilla.org/mozilla-central/source/dom/plugins/test/mochitest/test_GCrace.html?force=1)

    
    

    
  
> Do you know what sequence of messages led to the race in the first
> place?

No, I don't.

> ...
>
> So basically to reproduce this, the plugin needs to:
>
> * request the data
> * sleep so that all the data is delivered "at once"
> * check the ordering invariants
> * make sure that the plugin file actually exists and has the correct
>   contents

Sounds reasonable.  I'll give it a try.	 Thanks.

The trick will be making that last test consistently fail with
existing code :-)

    
    

    
  


  
FYI, here's what happens in Chrome when I get a meeting alert in Zimbra.

    
    

    
  
I'm having serious trouble writing tests for the bug (even the simple
test for making sure NPP_Write() isn't called after
NPP_StreamAsFile()).  The problem is that NPP_StreamAsFile() never
gets called!  I've double-checked that smode is being set properly in
NPP_NewStream() to NP_ASFILE, and it is (I load the test plugin with
streammode="asfile").

The other stream methods do get called (NPP_NewStream, NPP_WriteReady,
NPP_Write, NPP_DestroyStream and NPP_URLNotify).  Maybe the browser is
calling NPP_URLNotify instead of NPP_StreamAsFile.  Anyone know?

    
    

    
  
(Following up comment #73)

I should have mentioned that I'm basing my tests on the test plugin's streamTest method (in nptest.cpp).  I've altered it slightly to accept a streamAsFileCallback parameter.

    
    

    
  
It doesn't look like Josh is going to do his review anytime soon.  And
it appears we won't be able to write tests for this bug until another
bug is fixed -- the one that stops NPP_StreamAsFile from being called
when invoking the test plugin's streamTest method.

Also, this bug is a major regression, which we should fix as soon as
possible (hopefully in FF 8, the next release).

So I'm inclined to land	my patch now.

I'll leave Josh's review open.  And I'll spend some more time trying
to find the bug that my attempt to write tests uncovered.

What do you think, bsmedberg?

          tracking-firefox8:
          --- → ?

    
    

    
  
Why is there a 9 day review delay for a critical regression fix? Nobody else can do that review?

    
    

    
  
At this point (as I already pointed out) I've decided to land the patch without Josh's review (though I'll leave the review open).  I *would* like bsmedberg's OK.  But he's already r+ed the patch.  So if by tomorrow morning we haven't heard from him, I'll go ahead and land it.

    
    

    
  
http://hg.mozilla.org/integration/mozilla-inbound/rev/630de2647c0b
Whiteboard: STR in comment #31 → [inbound] STR in comment #31

    
    

    
  
Comment on attachment 565993 [details] [diff] [review]
Fix rev2 (follow bsmedberg's suggestions)

I was on vacation, thus the review delay.

        Attachment #565993 -
        Flags: review?(joshmoz) → review+

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/630de2647c0b
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10

    
    

    
  
comment #79: everyone has the right to be on vacation. I am more worried about the fact that we just blocked on you. Next time if you aren't around we need a fallback, and a fallback for the fallback. Do we?

    
    

    
  
bsmedberg was the fallback. This should have just landed without Josh's review immediately on October 10, considering he was on vacation.

    
    

    
  
Comment on attachment 565993 [details] [diff] [review]
Fix rev2 (follow bsmedberg's suggestions)

This patch is low-risk, but fixes serious breakage in the QuickTime plugin.

        Attachment #565993 -
        Flags: approval-mozilla-beta?

        Attachment #565993 -
        Flags: approval-mozilla-aurora?

    
    

    
  
Comment on attachment 565993 [details] [diff] [review]
Fix rev2 (follow bsmedberg's suggestions)

[Triage Comment]
* Approving for Aurora since this is a regression from our last released version
* Denying for Beta given the limited visibility any fallout may have (<2 weeks from release), and the fact that this code appears to touch more than OS X

        Attachment #565993 -
        Flags: approval-mozilla-beta?

        Attachment #565993 -
        Flags: approval-mozilla-beta-

        Attachment #565993 -
        Flags: approval-mozilla-aurora?

        Attachment #565993 -
        Flags: approval-mozilla-aurora+

    
    

    
  
Comment on attachment 565993 [details] [diff] [review]
Fix rev2 (follow bsmedberg's suggestions)

Landed on aurora:
http://hg.mozilla.org/releases/mozilla-aurora/rev/a1497cd3ec8f

> * Approving for Aurora since this is a regression from our last
>   released version
> * Denying for Beta given the limited visibility any fallout may have
>   (<2 weeks from release), and the fact that this code appears to
>   touch more than OS X

Actually this bug did effect our last release (FF 7), and possibly
also earlier releases.  So it's less of a problem not fixing it in FF
8.

But it's still a very bad bug (basically all QuickTime media is
currently broken when running OOP, at least on the Mac).  So it makes
sense to accelerate landing a fix (i.e. to land this patch at least on
the Aurora branch).  We should still have plenty of time (before the
Aurora branch becomes a release) to find out if it causes problems on
other platforms, and/or with other plugins.

    
    

    
  
Forgot to do this earlier.

          status-firefox9:
          --- → fixed
Target Milestone: mozilla10 → mozilla9

    
  
Whiteboard: [inbound] STR in comment #31 → [inbound][qa+] STR in comment #31

    
    

    
  
Hi guys.
There are other steps besides the ones from comment31? I need to verify this bug and those comments are a little bit slow.
Thanks

    
    

    
  
> There are other steps besides the ones from comment 31?

Not that I ever discovered.

    
    

    
  
Hi

In order to verify this I have to download zimbra calendar locally? If this is the case can you please provide a link and the settings of the application.
I'm asking this because I haven't use zimbra before and I need some guidance in this matter.

Thanks

    
    

    
  
> In order to verify this I have to download zimbra calendar locally?

No.  You need an account on the Mozilla Zimbra mail server (https://mail.mozilla.com/).  If you don't have one I'm sure one can be arranged ... but I don't how to start the process (though at some point you'll probably have to open a Bugzilla bug on the subject).

In any case the Mozilla Zimbra server is down.  I don't know when it'll be coming back up (see bug 707587).

    
    

    
  
Vlad, this will have to be tested by a Mozilla QA employee. Thanks anyway.

    
    

    
  
Ok Anthony.
Thanks
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #91)
> Vlad, this will have to be tested by a Mozilla QA employee. Thanks anyway.

    
    

    
  
I will give the verification process a try now by following the steps as pointed out in comment 31.

    
    

    
  
The steps from comment 31 are perfect. Thanks Steven. I can now reproduce it regularly in 8.0.1. But I have a question regarding the verification. Starting with Firefox 9 Beta the sound always gets played. Is that the indication that the bug is completely fixed? I'm kinda worried that we can only verify that we have fixed the caching issue but not the underlying Quicktime related bug. Would be great if you could give us some feedback on it. Thanks.

    
    

    
  
The underlying bug is bug 670036.  Just verify that bug's fixed.

The gist of bug 670036 is that QuickTime media won't play if it's already in the network cache.

    
    

    
  
Bug 670036 has already  been verified on all branches. So I will go ahead and simply check that the sound always gets played and we don't get the Quicktime dialog anymore.

Marking as verified fixed for the following builds:

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0) Gecko/20100101 Firefox/9.0

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0a2) Gecko/20111214 Firefox/10.0a2

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0a1) Gecko/20111215 Firefox/11.0a1
Status: RESOLVED → VERIFIED
Hardware: x86 → x86_64
Whiteboard: [inbound][qa+] STR in comment #31 → [inbound][qa!] STR in comment #31
Whiteboard: [inbound][qa!] STR in comment #31 → [inbound][qa-] STR in comment #31
Whiteboard: [inbound][qa-] STR in comment #31 → [inbound][qa!] STR in comment #31

    
  
Product: Core → Core Graveyard

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: