| Summary: | RenderTableSection's setNeedsCellRecalc needs to null check table() | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | James Robinson <jamesr> | ||||||||||||
| Component: | New Bugs | Assignee: | James Robinson <jamesr> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | dglazkov, eric, hyatt, jamesr, mitz, simon.fraser | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Other | ||||||||||||||
| OS: | OS X 10.5 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
James Robinson
2011-01-19 17:11:30 PST
Created attachment 79526 [details]
Patch
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. 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.
The naive approach to a test case didn't work, sadly. I might try reducing the extension's CSS instead. 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). 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.) <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. Created attachment 79539 [details]
stand-alone reduction (crashes when you reload the page, no extensions required!)
Created attachment 79540 [details]
layout test
Created attachment 79542 [details]
Patch
Mad props to Eric for the reduction. How's this? 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? :)
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.
(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. Committed r76276: <http://trac.webkit.org/changeset/76276> |