Bug 155109

Summary: Crash in WebCore::RenderElement::containingBlockForObjectInFlow
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: Layout and RenderingAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, bugs-noreply, commit-queue, esprehn+autocc, glenn, kondapallykalyan, mcatanzaro, simon.fraser, tpopela, webkit-bug-importer, WebkitBugTracker, zalan
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: PC   
OS: Linux   
See Also: https://bugzilla.redhat.com/show_bug.cgi?id=1314658
https://bugzilla.redhat.com/show_bug.cgi?id=1317201
https://bugzilla.redhat.com/show_bug.cgi?id=1318824
https://bugs.webkit.org/show_bug.cgi?id=155885
Attachments:
Description Flags
Backtrace
none
Patch
none
Patch
none
Backtrace with WebKitGTK+ 2.12.0 none

Description Michael Catanzaro 2016-03-07 05:39:02 PST
Created attachment 273175 [details]
Backtrace

I can't reproduce this, but a user testing WebKitGTK+ 2.11.91 reports that WebKit crashes whenever he visits www.seznam.cz in WebCore::RenderElement::containingBlockForObjectInFlow. Backtrace attached.
Comment 1 zalan 2016-03-07 15:07:46 PST
Created attachment 273221 [details]
Patch
Comment 2 zalan 2016-03-07 15:08:39 PST
I wasn't able to rerpo this, but it's certainly unsafe to call containingBlock() unconditionally in that loop.
Comment 3 Simon Fraser (smfr) 2016-03-07 15:12:20 PST
Comment on attachment 273221 [details]
Patch

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

> Source/WebCore/rendering/RenderBlock.cpp:259
> +            while (containingBlock && (containingBlock->style().position() == StaticPosition || (containingBlock->isInline() && !containingBlock->isReplaced()))
> +                && !is<RenderView>(*containingBlock)) {

I would put the RenderView check at the beginning:

while (containingBlock && !is<RenderView>(*containingBlock)...

> Source/WebCore/rendering/RenderBoxModelObject.cpp:243
> +    while (cb && cb->isAnonymous() && !is<RenderView>(*cb))

!is<RenderView>(*cb) second.

> Source/WebCore/rendering/RenderFlowThread.cpp:446
> +        while (objContainingBlock && !objContainingBlock->isRenderNamedFlowThread() && !is<RenderView>(*objContainingBlock)) {

while (objContainingBlock && !is<RenderView>(*objContainingBlock)

> Source/WebCore/rendering/RenderFlowThread.cpp:1228
> +    while (currentBlock && !currentBlock->isRenderFlowThread() && !is<RenderView>(*currentBlock)) {

Same
Comment 4 zalan 2016-03-07 15:18:32 PST
Created attachment 273224 [details]
Patch
Comment 5 WebKit Commit Bot 2016-03-07 16:45:15 PST
Comment on attachment 273224 [details]
Patch

Clearing flags on attachment: 273224

Committed r197716: <http://trac.webkit.org/changeset/197716>
Comment 6 WebKit Commit Bot 2016-03-07 16:45:19 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Michael Catanzaro 2016-03-25 07:09:31 PDT
Created attachment 274905 [details]
Backtrace with WebKitGTK+ 2.12.0

Unfortunately this patch didn't fix it; I have so far eight reports of this crash with WebKitGTK+ 2.11.92 (which includes the patch in this bug) and 12 with 2.12.0. Here's a backtrace taken with 2.12.0.

FWIW, it seems likely that the crash was introduced in 2.11.90 as I couldn't find any prior reports of this, so it's probably due to some recent change.
Comment 8 Michael Catanzaro 2016-03-25 07:18:53 PDT
Oh and a reported reproducer:

"I've tried to add an online account, selecting Facebook as provider.
If I switch to caps lock while typing the password, the dialog crashes.

Of course, this happens to other web services, like Google and Microsoft."

I couldn't reproduce this issue in my jhbuild environment. The web view in this case is displaying https://www.facebook.com/login
Comment 9 zalan 2016-03-25 08:39:20 PDT
(In reply to comment #7)
> Created attachment 274905 [details]
> Backtrace with WebKitGTK+ 2.12.0
> 
> Unfortunately this patch didn't fix it; I have so far eight reports of this
> crash with WebKitGTK+ 2.11.92 (which includes the patch in this bug) and 12
> with 2.12.0. Here's a backtrace taken with 2.12.0.
> 
> FWIW, it seems likely that the crash was introduced in 2.11.90 as I couldn't
> find any prior reports of this, so it's probably due to some recent change.

The attached backtrace shows a different issue. I'll file a new bugzilla for it.
Comment 10 Michael Catanzaro 2016-03-25 09:00:31 PDT
OK, thanks.
Comment 11 Radar WebKit Bug Importer 2016-03-25 11:23:04 PDT
<rdar://problem/25362489>