Bug 61702 – REGRESSION(r87566): It made all tests assert on Qt in debug mode (Requested by Ossy_weekend on #webkit).

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[WebKit Review Bot]

http://trac.webkit.org/changeset/87566 broke the build:
It made all tests assert on Qt in debug mode (Requested by Ossy_weekend on #webkit).

This is an automatic bug report generated by the sheriff-bot. If this bug
report was created because of a flaky test, please file a bug for the flaky
test (if we don't already have one on file) and dup this bug against that bug
so that we can track how often these flaky tests case pain.

"Only you can prevent forest fires." -- Smokey the Bear

[WebKit Review Bot]

Created attachment 95283 [details]
ROLLOUT of r87566

Any committer can land this patch automatically by marking it commit-queue+.  The commit-queue will build and test the patch before landing to ensure that the rollout will be successful.  This process takes approximately 15 minutes.

If you would like to land the rollout faster, you can use the following command:

  webkit-patch land-attachment ATTACHMENT_ID --ignore-builders

where ATTACHMENT_ID is the ID of this attachment.

[WebKit Commit Bot]

Comment on attachment 95283 [details]
ROLLOUT of r87566

Clearing flags on attachment: 95283

Committed r87635: <http://trac.webkit.org/changeset/87635>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Brady Eidson]

This entirely automatically managed bug is void of important information.

Such as, what were the ASSERTs?  Where are the backtraces?

Were they in Qt WebKit, and not in WebCore?

If they were in Qt WebKit, then Qt WebKit needs to fix their stuff.

This change is was sound on other platforms, and Qt just reverted WebCore to a very crashy state for all platforms.

[Brady Eidson]

On build.webkit.org I could only find one public facing Qt build bot that is a debug bot:
http://build.webkit.org/builders/Qt%20Windows%2032-bit%20Debug

And it did not show these ASSERTs during layout tests.

Can someone point me to the public-facing bot that showed these ASSERTs?
Can someone point me to the backtraces or ASSERTs that actually occurred?

I'm going to roll this back in later today if no one gets back to me on this.

[Brady Eidson]

The comment in https://bugs.webkit.org/show_bug.cgi?id=61494 that described the issue said simply:

ASSERTION FAILED: documentLoader
../../../Source/WebCore/dom/Document.cpp(4521) : void WebCore::Document::initSecurityContext()

-The comment above that ASSERT is true, and the ASSERT should still be valid.  That it always fires in Qt WebKit is indicative of a bug in Qt WebKit!

-A full backtrace would be much more useful in enabling non-Qt folks help figure out what is wrong.

[Csaba Osztrogonác]

Created attachment 95299 [details]
gdb backtrace for Qt assertion

[Darin Adler]

(In reply to comment #6)
> ASSERTION FAILED: documentLoader
> ../../../Source/WebCore/dom/Document.cpp(4521) : void WebCore::Document::initSecurityContext()
> 
> -The comment above that ASSERT is true, and the ASSERT should still be valid.  That it always fires in Qt WebKit is indicative of a bug in Qt WebKit!

No, I don’t think the comment is true.

The comment assumes that any document in a Frame will have a document loader. But a document created with the DOM does not have one.

I think the assertion is wrong.

[Brady Eidson]

(In reply to comment #8)
> (In reply to comment #6)
> > ASSERTION FAILED: documentLoader
> > ../../../Source/WebCore/dom/Document.cpp(4521) : void WebCore::Document::initSecurityContext()
> > 
> > -The comment above that ASSERT is true, and the ASSERT should still be valid.  That it always fires in Qt WebKit is indicative of a bug in Qt WebKit!
> 
> No, I don’t think the comment is true.
> 
> The comment assumes that any document in a Frame will have a document loader. But a document created with the DOM does not have one.
> 
> I think the assertion is wrong.

With the full backtrace, it was easy to see that alternate possibility!  Thanks to Csaba for providing that.

It seems bizarre that Qt WebKit follows this code path but no other WebKits apparently do.

Maybe all of the "since we're in a frame, we should have a document loader" asserts are wrong.

[Csaba Osztrogonác]

(In reply to comment #5)
> Can someone point me to the public-facing bot that showed these ASSERTs?
We have debug buildbots here: http://build.webkit.sed.hu/waterfall
They weren't so stable to add them to build.webkit.org, but nowadays 
they are quite stable and green. It would be helpful to add  a 32 and 
a 64 bit debug Qt tester bot to build.webkit.org. I think we can do
it in this week.

> Can someone point me to the backtraces or ASSERTs that actually occurred?
Done.