Bug 61702 - REGRESSION(r87566): It made all tests assert on Qt in debug mode (Requested by Ossy_weekend on #webkit).
Summary: REGRESSION(r87566): It made all tests assert on Qt in debug mode (Requested b...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: WebKit Review Bot
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-29 01:28 PDT by WebKit Review Bot
Modified: 2011-05-29 14:33 PDT (History)
4 users (show)

See Also:


Attachments
ROLLOUT of r87566 (9.77 KB, patch)
2011-05-29 01:28 PDT, WebKit Review Bot
no flags Details | Formatted Diff | Diff
gdb backtrace for Qt assertion (3.06 KB, text/plain)
2011-05-29 14:14 PDT, Csaba Osztrogonác
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description WebKit Review Bot 2011-05-29 01:28:04 PDT
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
Comment 1 WebKit Review Bot 2011-05-29 01:28:24 PDT
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.
Comment 2 WebKit Commit Bot 2011-05-29 01:30:47 PDT
Comment on attachment 95283 [details]
ROLLOUT of r87566

Clearing flags on attachment: 95283

Committed r87635: <http://trac.webkit.org/changeset/87635>
Comment 3 WebKit Commit Bot 2011-05-29 01:30:52 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Brady Eidson 2011-05-29 08:41:38 PDT
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.
Comment 5 Brady Eidson 2011-05-29 08:48:43 PDT
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.
Comment 6 Brady Eidson 2011-05-29 10:11:16 PDT
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.
Comment 7 Csaba Osztrogonác 2011-05-29 14:14:59 PDT
Created attachment 95299 [details]
gdb backtrace for Qt assertion
Comment 8 Darin Adler 2011-05-29 14:17:59 PDT
(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.
Comment 9 Brady Eidson 2011-05-29 14:30:24 PDT
(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.
Comment 10 Csaba Osztrogonác 2011-05-29 14:33:01 PDT
(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.