465013 - TM: General Error trying to play video on CNN

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jim Jeffery not reading bug-mail 1/2/11]

CNN videos no longer play with the latest builds following the merge from trace-monkey I suspect: 

bad in build/changeset
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1226683627/
http://hg.mozilla.org/mozilla-central/rev/b7a7180130d5

good in build/changeset:
http://hg.mozilla.org/mozilla-central/rev/6521a5409bd9
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1226676427/

No errors on the Console, no crash, just a blank/gray video window that states 'General Error'  Video controls are not loaded.  

Requesting blocking due to impending beta2 and that CNN is a popular site.

Comment 1

[Adrian]

flash video not working at all

Comment 2

[Jim Jeffery not reading bug-mail 1/2/11]

This is definitely involved with the TM merge :(

Comment 3

[Brendan Eich [:brendan]]

Any hope of testcase reduction? Or else bisecting among changesets in the tracemonkey repo to find the offending patch?

/be

Comment 4

[Larry Snow]

BTW, this is also true on YouTube. I have never seen this before.  Reported by me on XP and Elen on Vista here:

http://forums.mozillazine.org/viewtopic.php?f=23&t=948915

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081114 Firefox/3.0 ID:20081114164955

Comment 5

[Larry Snow]

YouTube works with this build just like CNN videos: 

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081114 Firefox/3.0 ID:20081114081911

Comment 6

[Larry Snow]

USAToday.com is also affected, so it appears that all videos are not working.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081114 Firefox/3.0 ID:20081114164955

Comment 7

[Andreas Gal :gal]

Works with jit off so this is definitively us. Any volunteers to bisect?

Comment 8

[TimoT]

This regressed between 2008-11-12-02 and 2008-11-13-02 tracemonkey nightlies.

Comment 9

[Larry Snow]

And the regression on MC is as stated above by Littlemut:

Good:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1226676427/
Bad:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1226683627/

Comment 10

[TimoT]

Seems to be bug 456511.

I backed out these 3 changesets and flash videos work again
https://bugzilla.mozilla.org/show_bug.cgi?id=456511#c50

Comment 11

[Brendan Eich [:brendan]]

Timo: thanks for bisecting. I will comment out individual call_imacro(...) calls to see what is failing -- please feel free to do the same after restoring all of the patches you reverted locally. Thanks again.

/be

Comment 12

[IU]

Interestingly, most flash fails, but videos at www.msnbc.msn.com work.

On abcnews.go.com, if you click a video, the video windows pops up and flash fails; but if you look at the bottom of that window, you'll notice JavaScript code--probably whatever script got busted.  I was going to past it, but after a select all copy to a text editor, I saw it would be a bit much to post here.

Comment 13

[Brendan Eich [:brendan]]

Attachment: reduced testcase

With -j this test prints

bgcolor="dummy" quality="dummy" allowScriptAccess="dummyboolean

Without, it prints

bgcolor="dummy" quality="dummy" allowScriptAccess="dummy" 

Dumb bug, hard-ish to reduce, easy fix.

/be

Comment 14

[Brendan Eich [:brendan]]

Attachment: fix

The atoms local in js_Interpret is (and must be) updated by the jstracer.h RECORD_ARGS macro. The atoms member of TraceRecorder was not updated upon tracing the JSOP_STOP at the end of the imacro. Easy to miss this if mentally checking off the "Set atoms = script->atomMap.vector" todo-item! The assignments look the same but differ in the LHS, and both are mandatory, but in different places :-P.

/be

Comment 15

[Brendan Eich [:brendan]]

Fixed in tracemonkey ahead of review, this is really an r=self thinko:

http://hg.mozilla.org/tracemonkey/rev/9e411a7500b5

/be

Comment 16

[Brendan Eich [:brendan]]

Jim, Larry, Timo, and anyone else helping test: thanks a ton for your help. Note that many video sites use the same (originally from Macromedia) flash scripting glue, so please find and mark dups of this bug if you can. And test against the next tracemonkey automatic build to confirm the fix on all affected sites. Thanks again,

/be

Comment 17

[Brendan Eich [:brendan]]

A bit hurried at home, and it shows. Followup nit fix:

http://hg.mozilla.org/tracemonkey/rev/74d95f4b5928

/be

Comment 18

[Robert Sayre]

Comment on attachment 348366 [details] [diff] [review]
fix

a=sayrer after this settles on TM for a bit

Comment 19

[Jim Jeffery not reading bug-mail 1/2/11]

Confirming the patch does fix the video problems.  CNN and ABCNews now play just fine using the latest hourly TM build:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081115 Minefield/3.1b2pre Firefox/3.0 ID:20081115125511

Vista HP SP1

Comment 20

[mdew]

we should land this for Beta 2 asap

Comment 21

[Andreas Gal :gal]

It will be merged into central very soon and will be in b2

Comment 22

[Larry Snow]

Videos are fine again in XP.  At least on CNN and YouTube, which are the only ones I checked.  Thanks to all.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081116 Firefox/3.0 ID:20081116005730

Comment 25

[Axel Hecht [:Pike]]

VERIFIED on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081116 Minefield/3.1b2pre, built from http://hg.mozilla.org/mozilla-central/rev/8b3caddca8f5.

Comment 26

[Bob Clary [:bc] (inactive)]

test landed http://hg.mozilla.org/mozilla-central/rev/8cccbdfac656 and cvs