Bug 65581 – Crash in DocumentWriter::endIfNotLoadingMainResource

• Component: New Bugs
• Status: RESOLVED FIXED

[Adam Barth]

Crash in DocumentWriter::endIfNotLoadingMainResource

[Adam Barth]

Created attachment 102714 [details]
Patch

[Darin Adler]

Comment on attachment 102714 [details]
Patch

Can you run that test case on a computer without Flash installed?

[Adam Barth]

> Can you run that test case on a computer without Flash installed?

Let me try with the test plugin.

[Adam Barth]

Created attachment 102721 [details]
Patch

[Nate Chapin]

Comment on attachment 102721 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=102721&action=review

> Source/WebCore/ChangeLog:10
> +        This function is poorly designed because isLoadingMainResource is a
> +        poor proxy for determing whether to flush/finish the parser.  Really,
> +        we should how loads complete to match the model in HTML5, but that's

Typo: I think you're missing a word, "we should change how loads..."?

Also, FIXME in endIfNotLoadingMainResource to this effect?

> LayoutTests/fast/loader/reload-zero-byte-plugin.html:16
> +    }, 100);
> +}, 100);

Can we do this without setTimeout, or with shorter timeouts?  200ms isn't too bad, but I feel honor-bound to ask.

[Adam Barth]

Comment on attachment 102721 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=102721&action=review

>> LayoutTests/fast/loader/reload-zero-byte-plugin.html:16
>> +}, 100);
> 
> Can we do this without setTimeout, or with shorter timeouts?  200ms isn't too bad, but I feel honor-bound to ask.

The underlying problem is that opening content in a new window doesn't generate any sort of events.  Normally we could load some HTML that sent use a postMessage, but in this case we need to load this empty plugin.  It's possible we could teach the test plugin to send us a message on load...

[Nate Chapin]

(In reply to comment #6)
> (From update of attachment 102721 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=102721&action=review
> 
> >> LayoutTests/fast/loader/reload-zero-byte-plugin.html:16
> >> +}, 100);
> > 
> > Can we do this without setTimeout, or with shorter timeouts?  200ms isn't too bad, but I feel honor-bound to ask.
> 
> The underlying problem is that opening content in a new window doesn't generate any sort of events.  Normally we could load some HTML that sent use a postMessage, but in this case we need to load this empty plugin.  It's possible we could teach the test plugin to send us a message on load...

Eh, I won't ask you to do that.  the test plugin has way too much bolted onto it already imo.

[Alexey Proskuryakov]

When I run this test with r92135 on Mac, I don't get any crash, but get empty test results. That's surprising.

run-webkit-tests fast/loader/reload-zero-byte-plugin.html --repeat 100

[Alexey Proskuryakov]

> but get empty test results

Strike that - I applied the patch incorrectly. But still no crash.

[Adam Barth]

Perhaps the crash is limited to Chromium?  The Gtk folks have also asked for this null-check, but I can't find the bug atm.

[Adam Barth]

Created attachment 102793 [details]
Patch for landing

[Adam Barth]

> Strike that - I applied the patch incorrectly. But still no crash.

(I assume you tested in DRT and not just in Safari.)

[WebKit Review Bot]

Comment on attachment 102793 [details]
Patch for landing

Clearing flags on attachment: 102793

Committed r92298: <http://trac.webkit.org/changeset/92298>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Zhenyao Mo]

Skipped the added fast/loader/reload-zero-byte-plugin.html in r92306 because of fast/loader/repeat-same-document-navigation.html crashing

[Ryosuke Niwa]

fast/loader/reload-zero-byte-plugin.html started crashing after this patch was landed:
http://build.webkit.org/builders/Qt%20Linux%20Release/builds/36101

[Adam Barth]

I added it to the skipped list for Qt.

[Adam Barth]

I'm not sure why this bug is open.  Is there more to do here?  Should we file another issue about the Qt crash?