Bug 23992 - REGRESSION: crash on windows loading http://www.stickam.com/liveStreams.do
Summary: REGRESSION: crash on windows loading http://www.stickam.com/liveStreams.do
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac Windows XP
: P1 Normal
Assignee: Nobody
URL: http://www.stickam.com/liveStreams.do
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-17 13:55 PST by Ojan Vafai
Modified: 2009-02-18 14:12 PST (History)
2 users (show)

See Also:


Attachments
Fix crash on page load of http://www.stickam.com/liveStreams.do. Just readds a null-check that was removed in r40824. (437 bytes, patch)
2009-02-17 14:43 PST, Ojan Vafai
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ojan Vafai 2009-02-17 13:55:29 PST
I only have a debug build of Chromium. So you can see the stack here: http://crbug.com/7682

But a *Windows* webkit nightly seems to crash in the same place. Mac nightlies load fine.
Comment 1 Ojan Vafai 2009-02-17 14:01:14 PST
Looks like it started with darin's change here: http://trac.webkit.org/changeset/40824

Specifically removing the null check for doc in Frame::contentRenderer() (Frame.cpp:1130)
Comment 2 Eric Seidel (no email) 2009-02-17 14:08:12 PST
marking p1 since this is a regression.
Comment 3 Ojan Vafai 2009-02-17 14:30:01 PST
I'm happy to just add the null-check back in. Don't know if that's the right fix though. Will send a patch with the null-check if I don't hear otherwise.
Comment 4 Ojan Vafai 2009-02-17 14:43:03 PST
Created attachment 27738 [details]
Fix crash on page load of http://www.stickam.com/liveStreams.do. Just readds a null-check that was removed in r40824.

 WebCore/page/Frame.cpp |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Comment 5 Alexey Proskuryakov 2009-02-18 04:11:39 PST
Comment on attachment 27738 [details]
Fix crash on page load of http://www.stickam.com/liveStreams.do. Just readds a null-check that was removed in r40824.

r=me

The code change looks fine, but I'm somewhat confused whether this needs a layout test. Did this problem cause crashes in existing tests on Windows? If it did not, can a new test be created for this fix?
Comment 6 Ojan Vafai 2009-02-18 14:02:29 PST
IRC discussion:
ojan: ap: regarding https://bugs.webkit.org/show_bug.cgi?id=23992...i haven't really been able to come up with a reduced case for the crash. it does crash everytime loading the page in question, but i haven' been able to figure out where exactly. saving a static version of the page and loading it doesn't cause a crash. i can dig further into the stack trace to try and understand when we're hitting this, but i'm not sure how much fruit that will yield.
ap: ojan: ok

I did dig further. Looks like it's crashing in the actual load event. I think (not sure) that an iframe is being appended to the page onload of the top-level window. The contentRenderer is grabbed before the iframe has a document.
Comment 7 Darin Fisher (:fishd, Google) 2009-02-18 14:12:17 PST
Landed as http://trac.webkit.org/changeset/41066