386493 - [FIX] Crash when reloading embedded video [@ npdsplay.dll]

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P1)

Description

[Ria Klaassen (not reading all bugmail)]

Attachment: testcase

On load or reload of the testcase I get a crash:
https://crash-reports.mozilla.com/reports/report/index/6746358f-27bb-11dc-acd3-001a4bd43ed6?date=2007-07-01-10

Firefox 3.0a6pre Crash Report
UUID	6746358f-27bb-11dc-acd3-001a4bd43ed6
Time	2007-07-01 03:11:15.940000-07:00
Build ID	2007063020
OS	Windows NT
OS Version	5.1.2600 Service Pack 2
CPU	x86
CPU Info	AuthenticAMD family 6 model 8 stepping 0
Crash Reason	EXCEPTION_ACCESS_VIOLATION
Crash Address	0x7d94000
Stack of Crashing Thread
frame 	signature
0 	npdsplay.dll@0x1ee53
1 	npdsplay.dll@0x27de8

Couldn't find a regression range; in Feb 2006 the crash didn't occur often enough.
I assume this is a different bug than bug 364028.

Comment 1

[Martijn Wargers (dead)]

I can't reproduce it with the testcase anymore, but I get this crash when reloading this page: http://members.aol.com/jrzycrim01/mozilla/wmp/vidtest-HS.html
with current trunk build and using windows Media Player 10.

Comment 2

[Ria Klaassen (not reading all bugmail)]

This bug was fixed on 18 July, I think by Bug 347736.

Comment 3

[Ria Klaassen (not reading all bugmail)]

Although I see no movie either, so what is "fixed"? No crash or no movie?

Comment 4

[Martijn Wargers (dead)]

You don't see a movie anymore with your testcase? Do you see the windows Media Player appearing (but no movie)?
I can see the movie just fine with the testcase, using current trunk build.

Comment 5

[Ria Klaassen (not reading all bugmail)]

I see WMP but no movie. Maybe because I'm using WMP 9.

Comment 6

[Martijn Wargers (dead)]

(In reply to comment #5)
> I see WMP but no movie. Maybe because I'm using WMP 9.

Hmm, so WMP 9 needs to be watched for, too.
Not sure what to do about this, it might be best to file a new bug for that (CC me?).

Comment 7

[Johnny Stenback (:jst)]

I used to be able to reproduce this, but now I don't see the crash any more. I've got WMP 11, and I see the video just fine. Anyone else?

Comment 8

[Martijn Wargers (dead)]

Johnny, I can still reproduce it with http://members.aol.com/jrzycrim01/mozilla/wmp/vidtest-HS.html
using current trunk build and WMP 10. Could you try it on that url?

Comment 9

[Johnny Stenback (:jst)]

Attachment: Fix.

This has probably been happening more or less since the fix for bug 1156 went in (~2 years ago, post-branching for Firefox 2 etc). The problem here is really just a bug in WMP. What's happening here is that we start the load of the data in the object tag, and once we get an OnStartRequest() we initialize the plugin. IOW, the request for the object data (i.e. the video file) is already coming at us at this point. Now, when the WMP plugin is initialized, it requests two javascript: URLs to be loaded (on from NPP_New() and one from the second call to NPP_SetWindow()), both for "document.location". Iff those javascript: loads finish before the first OnDataAvailable() comes in for the object data, then all is well, but if we end up feeding the plugin any of the video data before the two javascript: URL loads are done, it crashes.

So my fix for this is rather uggly, though much less so than it looked like it would be when I first realized what was going on here. The fix basically flags the listeners for the javascript: URL loads that are requested during plugin intialization and when a stream listener receives data, it suspends the requests for any data that isn't for one of these initialization URLs if we're still loading one of the initialization URLs.

If anyone has better ideas, please do tell :)

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 285545 [details] [diff] [review]
Fix.

Looks good.  sr=bzbarsky

Comment 11

[Ria Klaassen (not reading all bugmail)]

Could bug 401304 be a duplicate of this bug?

Comment 12

[Johnny Stenback (:jst)]

Indeed, the fix for this appears to fix the crash reported there as well.

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 285545 [details] [diff] [review]
Fix.

+        // Also suspend the stream if the stream we're loading is not

perhaps mention this bug# in the comment.

Comment 15

[Johnny Stenback (:jst)]

Fix checked in with reference to the bug number added to the comment per biesi.

Comment 16

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007111304 Minefield/3.0b2pre
and WMP10

Also verified fixed on windows vista with current trunk build and WMP11.