[webkit-reviews] review denied: [Bug 90865] [CSSRegions]Crash when moving anonymous block children inside a named flow : [Attachment 156361] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 23 00:36:21 PDT 2012


Abhishek Arya <inferno at chromium.org> has denied Andrei Onea <onea at adobe.com>'s
request for review:
Bug 90865: [CSSRegions]Crash when moving anonymous block children inside a
named flow
https://bugs.webkit.org/show_bug.cgi?id=90865

Attachment 156361: Patch
https://bugs.webkit.org/attachment.cgi?id=156361&action=review

------- Additional Comments from Abhishek Arya <inferno at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=156361&action=review


> Source/WebCore/rendering/RenderBlock.cpp:1140
> +    anonBlock->moveAllChildrenTo(parent, nextSibling, child->hasLayer(),
childFlowThread);

Why are we adding so much complexity to moveAllChild functions, just for
regions. We can probably get way with an easier fix here. How about
CurrentRenderFlowThreadMaintainer local to keep the enclosingRenderFlowThread
for the duration of this function. This is quite an unusual case in this
function that we remove the anonymous block first and then move its children,
but probably it is done to keep anonymous block alive (otherwise last anonymous
block will be auto-deleted).

> LayoutTests/fast/regions/move-anonymous-block-inside-named-flow-crash.html:1
> +<html>

Please add <!DOCTYPE html>

> LayoutTests/fast/regions/move-anonymous-block-inside-named-flow-crash.html:6
> +.flow {-webkit-flow-from: flow; width: 100px; height: 100px;}

nit: spaces around { }

> LayoutTests/fast/regions/move-anonymous-block-inside-named-flow-crash.html:15

> +    if (window.layoutTestController)

Please use window.testRunner

> LayoutTests/fast/regions/move-anonymous-block-inside-named-flow-crash.html:18

> +    test.innerHTML = "Bug 90865:[CSSRegions]Crash when moving anonymous
block children inside a named flow. Test passes if it does not CRASH or
ASSERT.";

Split the message across lines.


More information about the webkit-reviews mailing list