Dynamic updates to display: contents reach unreachable code within RenderTreeUpdater::updateTextRenderer

[Emilio Cobos Álvarez]

Created attachment 311016 [details]
Stack

When importing the css-display-3 tests, a few tests that check dynamic changes to display: contents are crashing within that function, reaching unreachable code in RenderElement::insertChildInternal.

[Emilio Cobos Álvarez]

So I debugged this today, and it's likely the same bug as bug 172514.

In particular, we're finding as the next sibling a text renderer that hasn't been torn down when "display" changes, so it's still reparented to the old parent...

[Emilio Cobos Álvarez]

I finally found it. I was going crazy, and it required a lot of logging to see what's going on... This is likely to fix bug 172514 too, so I'll try to re-enable the test.

[Emilio Cobos Álvarez]

Created attachment 311234 [details]
Patch

[Emilio Cobos Álvarez]

Created attachment 311235 [details]
Patch

[Antti Koivisto]

Comment on attachment 311235 [details]
Patch

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

> Source/WebCore/ChangeLog:14
> +        In practice, we may want to remove that bit and use ElementRareData's
> +        RenderStyle instead (keeping it around as appropriate), to ensure they
> +        don't go out of sync, but that's out of scope of this patch for now.

Yeah, this is the way to go in long run. Not keeping the full style currently requires some awkward code in style resolution.

[Emilio Cobos Álvarez]

Comment on attachment 311235 [details]
Patch

Whoops, forgot to add the commit-queue flag :)

[WebKit Commit Bot]

Comment on attachment 311235 [details]
Patch

Rejecting attachment 311235 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 311235, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.webkit.org/results/3816649

[Emilio Cobos Álvarez]

Created attachment 311291 [details]
Patch

[Emilio Cobos Álvarez]

Comment on attachment 311291 [details]
Patch

Gah, I'm really bad at this ChangeLog thing, apparently :P.

Added the missing "Reviewed by" line.

[WebKit Commit Bot]

Comment on attachment 311291 [details]
Patch

Clearing flags on attachment: 311291

Committed r217477: <http://trac.webkit.org/changeset/217477>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

*** Bug 172613 has been marked as a duplicate of this bug. ***

[Radar WebKit Bug Importer]

<rdar://problem/32479799>