Bug 52770 – RenderTableSection's setNeedsCellRecalc needs to null check table()

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Robinson]

RenderTableSection's setNeedsCellRecalc needs to null check table()

[James Robinson]

Created attachment 79526 [details]
Patch

[James Robinson]

This patch fixes the crash reported in http://code.google.com/p/chromium/issues/detail?id=69212.  It's obvious from inspector that this should check for NULL but I'm not sure exactly how to make a layout tests with a RenderTableSection that has a null parent.  The repro instructions on the chromium bug (install https://chrome.google.com/webstore/detail/hngfmkbjhlcbdgmppkpkdejbgmblalmi, go to http://dicademusica.blogspot.com/2011/01/fotos-engracadas-com-mulheres.html, reload) worked for me.  I'm guessing the failure is related to one of the ::-webkit-scrollbar styles that https://chrome.google.com/webstore/detail/hngfmkbjhlcbdgmppkpkdejbgmblalmi sets.

[Eric Seidel (no email)]

Created attachment 79534 [details]
css from the extension (this is injected into every page)

Should be possible to copy/paste this CSS at the top of a local version of the page in question to reproduce the crash, no?

The extension also has a background.html page, but that doesn't seem related.

[Eric Seidel (no email)]

The naive approach to a test case didn't work, sadly.

I might try reducing the extension's CSS instead.

[James Robinson]

The page doesn't seem to crash immediately, so it seems that there's some interaction with some delayed resource that leads to the crash.  It may take some fiddling to get it just right (sadly).

[Eric Seidel (no email)]

Using a local copy of the extension (which I"m about to reduce) I'm able to crash with the live site.  I'm not able to crash with a curl'd down copy of the site however.  (Yes, my local extension is loaded for file urls.)

[Eric Seidel (no email)]

<iframe src="http://static.megacubo.net/live/banners/top468.html?c=ffffff,000000" width="468" height="60" frameborder="0"></iframe>

Is the code causing the crash.

[Eric Seidel (no email)]

Created attachment 79539 [details]
stand-alone reduction (crashes when you reload the page, no extensions required!)

[Eric Seidel (no email)]

Created attachment 79540 [details]
layout test

[James Robinson]

Created attachment 79542 [details]
Patch

[James Robinson]

Mad props to Eric for the reduction.  How's this?

[Eric Seidel (no email)]

Comment on attachment 79542 [details]
Patch

No shout-out in the ChangeLog?  How will the interwebs know how baller I was here w/o it? :)

[Eric Seidel (no email)]

Comment on attachment 79542 [details]
Patch

Are there perf concerns with moving this function to be non-inline?  I kinda doubt it, but curious. I guess we'll see when the PLT bots run.

[James Robinson]

(In reply to comment #13)
> (From update of attachment 79542 [details])
> Are there perf concerns with moving this function to be non-inline?  I kinda doubt it, but curious. I guess we'll see when the PLT bots run.

I don't think this function is super hot, but it's getting a little big to be inline at this point.

[James Robinson]

Committed r76276: <http://trac.webkit.org/changeset/76276>