677658 - crash mozalloc_abort [@ mozalloc_abort(char const* const) | mozalloc_handle_oom() | nsHtml5TreeOperation::AppendText(wchar_t const*, int, nsIContent*, nsHtml5TreeOpExecutor*)]

Status: RESOLVED FIXED

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

Description

[Alice0775 White]

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/f414db34c70b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a1) Gecko/20110808 Firefox/8.0a1 ID:20110808030804

Reported @1for-matik
See http://forums.mozillazine.org/viewtopic.php?p=11111195#p11111195

High CPU and Huge memory usage while loading the page.
The browser become unresponsive.
And finally, the browser crashes with crash report.

bp-58beeda6-939e-48c8-855f-ccabe2110809 .


Reproducible: Always

Steps to Reproduce:

1. Start browser with clean profile
2. Open URL
3. 

Actual Results: 
  High CPU and Huge memory usage while loading the page.
  The browser become unresponsive.
  And finally, the browser crashes with crash report.

Expected Results: 
  No high CPU usage
  No Huge memory usage
  No Crash

Regression window(m-i hourly)
Works:
http://hg.mozilla.org/integration/mozilla-inbound/rev/2d6026252d03
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a1) Gecko/20110731 Firefox/8.0a1 ID:20110731202744
Fails:
http://hg.mozilla.org/integration/mozilla-inbound/rev/7e84bd591246
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a1) Gecko/20110731 Firefox/8.0a1 ID:20110801010116
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2d6026252d03&tochange=7e84bd591246

Suspected bug;
Bug 563322 - Does nsGenericHTMLElement::SetInnerHTML need to call scriptloader->SetEnabled(...)

Comment 1

[Alice0775 White]

The following cset triggers the issue.
ff515cbd864e	Henri Sivonen — Bug 563322 part 1 - Avoid calling nsScriptLoader::SetEnabled in the innerHTML setter when using the HTML parser. r=Olli.Pettay.

Comment 2

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

I can certainly reproduce.
Henri is on vacation, so I can take a look.

Comment 3

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

Well, I can't reproduce the crash, but some kind of hang/endless loop.

Comment 4

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

Attachment: backout+change to InsertAdjacentHTML

I uploaded this to tryserver.

But I still don't know why not-disabling scriptloader causes the problem

Comment 5

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

Ok, bug 563322 certainly regressed async script handling.

Comment 6

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

Attachment: patch

Added nsAutoScriptBlockerSuppressNodeRemoved to fix unnecessary warning.
Includes also a test.

I think we should take this kind of approach for FF8, and then perhaps fix
this in other ways for FF9 (once hsivonen is back), so that disabling script
loader wouldn't be needed.

Uploaded the patch to tryserver.

Comment 7

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

And FYI, Webkit and Opera don't run the script when added using innerHTML.

Comment 8

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

Passed on try

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 552084 [details] [diff] [review]
patch

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

Sucks to not know why this fixes the crash/hang, but this does seem like a safer approach for now.

Comment 10

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

http://hg.mozilla.org/mozilla-central/rev/3bf55a9e34e8

Comment 11

[Henri Sivonen (:hsivonen)]

So this effectively undid bug 563322, right? What was the problem? Did the parser not mark scripts as already executed properly?

Comment 12

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

(In reply to Henri Sivonen (:hsivonen) from comment #11)
> So this effectively undid bug 563322, right?
Yes


> What was the problem? Did the
> parser not mark scripts as already executed properly?
IIRC nsScriptLoader::ProcessScriptElement just let the processing go through.
If scriptloader is disabled, that method returns early.