734019 - The big picture hangs , and finally Browser crashes with null signature

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, defect)

Description

[Dirkjan Ochtman (:djc)]

The URL makes Aurora reliably hang (12.0a2 from 2012-03-04 on Windows 7).

Comment 2

[Loic]

URL: http://www.boston.com/bigpicture/2012/02/gerd_ludwigs_long_shadow_of_ch.html
Firefox 11+ displays 'Not responding' then 'Unresponsive script' warnings:

----------
A script on this page may be busy, or it may have stopped responding. You can stop the script now, or you can continue to see if the script will complete.

Script: http://w.sharethis.com/button/sharethis.js#tabs=web%2Cpost&charset=utf-8&services=facebook%2Cdigg%2Cstumbleupon%2Ctwitter%2Creddit%2Cdelicious%2Cmixx%2Cmyspace%2Cnewsvine%2Cblogger%2Ctypepad%2Cwordpress%2Ctechnorati%2Clinkedin%2Cslashdot%2Cgoogle_bmarks%2Cyahoo_bmarks%2Cwindows_live%2Cfriendfeed%2Cpropeller%2Cblogmarks%2Cfurl%2Cblinklist%2Cfriendster&style=default&publisher=9afb63cc-565b-4af5-8f8b-95034feb717c:1
----------

Comment 3

[Thomas Ahlblom]

There are a number of open bugs with slowness/hangs/crashes on http://www.boston.com/bigpicture/, like bug 734746, bug 735437, bug 735440, bug 735441, bug 735446 and bug 735811. Additionally bug 734129 is a crash that has been fixed in FX13.

Comment 4

[Alice0775 White]

I can reproduce on Firefox11.0-14.0a1.

"Warning: Unresponsive script" dialog Pops up and UI is locked.
And finally Browser crashes with null signature.

Step To Reproduce:
1. Start Firefox with clean profile
2. Open http://www.boston.com/bigpicture/2012/03/
3. Click  link "Japan tsunami pictures: before and after"

Actual Results:
"Warning: Unresponsive script" dialog Pops up and UI is locked.
And finally Browser crashes with null signature.

bp-ddaa53c7-7d54-4690-922b-9c3a02120315
bp-edebf2d2-8a94-42b4-9f27-b04152120315


Regression window

No hung up, no "Warning: Unresponsive script" dialog.
And works properly:
http://hg.mozilla.org/mozilla-central/rev/a5e63e00db27
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111218 Firefox/11.0a1 ID:20111218031140

"Warning: Unresponsive script" dialog Pops up and UI is locked
And finally Browser crashes with null signature:
http://hg.mozilla.org/mozilla-central/rev/543af61eee05
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111218 Firefox/11.0a1 ID:20111218021643

Script: http://w.sharethis.com/button/sharethis.js#tabs=web%2Cpost&charset=utf-8&services=facebook%2Cdigg%2Cstumbleupon%2Ctwitter%2Creddit%2Cdelicious%2Cmixx%2Cmyspace%2Cnewsvine%2Cblogger%2Ctypepad%2Cwordpress%2Ctechnorati%2Clinkedin%2Cslashdot%2Cgoogle_bmarks%2Cyahoo_bmarks%2Cwindows_live%2Cfriendfeed%2Cpropeller%2Cblogmarks%2Cfurl%2Cblinklist%2Cfriendster&style=default&publisher=9afb63cc-565b-4af5-8f8b-95034feb717c:1

  function SHARETHIS_unlink(c) {
    var a;
    switch (SHARETHIS_typeof(c)) {
    case "object":
      a = {};
      for (var e in c) {
>>      a[e] = SHARETHIS_unlink(c[e])
      }
      break;
    case "hash":



Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a5e63e00db27&tochange=543af61eee05

Last good :c3525cd1ce44
First bad :f9f6f9ed788a

Triggered by
f9f6f9ed788a	Ms2ger — Bug 707576 - Remove nsIDOMNSElement; r=smaug

Comment 5

[Boris Zbarsky [:bzbarsky]]

This is a pretty commonly visited site....

Comment 6

[Alex Keybl [:akeybl]]

Tracking for 11 through 14. We may consider this as a ride-along if we chemspill for FF11.

ms2ger - please prepare a backout patch of bug 707576 for Aurora 13 and Beta 12 so that we can test the backout ahead of a possible chemspill.

Comment 7

[Loic]

I tried today and I'm not able to reproduce the chromehangs (with not responding/unresponsive script warnings), it appears the website has modified its code. Anyway maybe the patch of bug 707576 should be re-examined and tested.

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Can't reproduce here. I get two slow script warnings, but no real hang and no crash.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Do you get the slow script warnings with nighties from before the range in comment 4?

Comment 10

[Alice0775 White]

Attachment: Saved page

Comment 11

[Alice0775 White]

I cannot reproduce on the URL comment2 in Firefoc11-14.0a1 anymore.(Though I can reproduce with attachment 606705 [details])

Comment 12

[Boris Zbarsky [:bzbarsky]]

So I _think_ what happens here is that the big inline script has SHARETHIS_merge caled on two objects, one of which has an "element" property (an HTMLSpanElement) and one of which does not.  In particular, f ends up as undefined, while g is an HTMLSpanElement, so SHARETHIS_unlink is called on the HTMLSpanElement.

And if you look at SHARETHIS_unlink, calling it on any element of a data structure with loops (e.g. a DOM) will put it in an infinite loop.

So presumably we used to not end up landing in there at all....

Comment 13

[Boris Zbarsky [:bzbarsky]]

I think we should strongly consider backing out bug 707576 (assuming that addresses the issue) on aurora and beta if we don't have another fix plan soonish...

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Ms2ger, could you prepare a backout patch?

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Ms2ger, ping.

Comment 16

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/try/rev/873519bccfed appears to work (for aurora)

Comment 17

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Beta backout

Comment 18

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Aurora backout

Comment 19

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 613656 [details] [diff] [review]
Beta backout

[Approval Request Comment]
Regression caused by (bug #): bug 707576
User impact if declined: The big picture hangs
Testing completed (on m-c, etc.): Passes try
Risk to taking this patch (and alternatives if risky): Reverts to the previous state
String changes made by this patch: None

Comment 20

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 613658 [details] [diff] [review]
Aurora backout

[Approval Request Comment]
Regression caused by (bug #): bug 707576
User impact if declined: The big picture hangs
Testing completed (on m-c, etc.): Passes try
Risk to taking this patch (and alternatives if risky): Reverts to the previous state
String changes made by this patch: None

Comment 21

[Alex Keybl [:akeybl]]

Comment on attachment 613656 [details] [diff] [review]
Beta backout

[Triage Comment]
Approving this backout for Beta 13 since it caused a web regression. Please land asap.

Comment 22

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/releases/mozilla-aurora/rev/a656a751c055

http://hg.mozilla.org/releases/mozilla-beta/rev/ca80d75dbcb1

Comment 23

[Nomis101]

It seems this backout has broken building of the Eudora importer in Thunderbird Beta Mac builds:
http://tinderbox.mozilla.org/showlog.cgi?log=Thunderbird-Beta/1334097653.1334101565.32224.gz&fulltext=1

Comment 24

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Yes, it would, given that the original landing did as well. The long-term fix is bug 684466.

Comment 25

[Justin Wood (:Callek)]

FYI this backout broke comm-based builds [see-also c#23/24], I just found out with a SeaMonkey beta build run [after not double checking that comm-beta itself was green].

It is rare that a change breaks us, but if possible can we *try* to give an explicit heads up in the future?

This fix should be relatively simple its just not a fix I can do tonight [and rekick release automation in time for the hg outage tomorrow]

If no-one beats me to it, I'll try and whip together a patch to fix comm tomorrow sometime after the hg outage.

Comment 26

[Nomis101]

This is relatively simple to fix, you only need to back out this from comm-beta:
http://hg.mozilla.org/releases/comm-beta/rev/2086df5c1eab
I've done this yesterday on my hard drive and now it builds again. :-)

Comment 27

[Boris Zbarsky [:bzbarsky]]

Justin, sorry about that.  I didn't realize this patch affected comm-central when I pushed; otherwise I would have pinged you guys...

Comment 28

[David :Bienvenu]

I can fix this...

Comment 29

[David :Bienvenu]

backed out on comm-aurora and comm-beta:

http://hg.mozilla.org/releases/comm-aurora/rev/5f95d056c6a5
http://hg.mozilla.org/releases/comm-beta/rev/fde388f1f7ec

Comment 30

[Benjamin Smedberg]

This backout involved a rather significant IDL change after beta, which means that binary addons which compiled against beta1 (which is what we recommend) are now crashing or displaying other odd behavior (see bug 745453). Adding the addon-compat keyword, but is there a way we could have accomplished the core of this backout without the IDL changes? Is this something we can undo?

Comment 31

[Boris Zbarsky [:bzbarsky]]

The point of the patch being backed out was to merge one interface into another.  The backout, if done, kinda has too unmerge the interfaces, and that does involve iid changes...

The only other option is to find which part of the interface merge, exactly, broke the site and try to fix that part assuming _that's_ possible without IDL changes.

Of course we can undo the backout; that will re-break the site in question (which is pretty popular).

Comment 32

[Boris Zbarsky [:bzbarsky]]

And I'm really sorry the backout didn't happen much sooner.  :(

Comment 33

[Alex Keybl [:akeybl]]

(In reply to Boris Zbarsky (:bz) from comment #32)
> And I'm really sorry the backout didn't happen much sooner.  :(

Let's backout the backout on mozilla-beta if it resolves bug 745453. Losing Mac users is a much bigger risk than the web regression with big picture. Searching through input, there's maybe one mention of big picture w/r/t FF11.

Comment 34

[Boris Zbarsky [:bzbarsky]]

OK, backed out the backout on beta:

http://hg.mozilla.org/releases/mozilla-beta/rev/6709370fb419
http://hg.mozilla.org/releases/mozilla-beta/rev/3182caa7f19a

and on comm-beta:

http://hg.mozilla.org/releases/comm-beta/rev/746e4b54892f
http://hg.mozilla.org/releases/comm-beta/rev/4c6783ac2a3d

Comment 35

[Alex Keybl [:akeybl]]

I'd like to get some QA around this issue prior to relnoting for FF12, since I have a sneaking suspicion that boston.com has employed a workaround.

Comment 36

[Alex Keybl [:akeybl]]

(In reply to Alex Keybl [:akeybl] from comment #35)
> I'd like to get some QA around this issue prior to relnoting for FF12, since
> I have a sneaking suspicion that boston.com has employed a workaround.

QA has not been able to reproduce the issue across multiple OSs and versions. I don't think we need to track this for release any longer.

Comment 38

[Boris Zbarsky [:bzbarsky]]

Not exactly reproducible anymore....

Comment 39

[Lukas Blakk [:lsblakk] use ?needinfo]

[Triage Comment]
Updating status for 14, looks like this was never landed to central (when central was 14) so it shouldn't be affected here.