[webkit-reviews] review granted: [Bug 89000] [CSS Regions] Fix the lifecycle for the flow objects and their renderers : [Attachment 151916] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Andrei Bucur <abucur at adobe.com>'s
request for review:
Bug 89000: [CSS Regions] Fix the lifecycle for the flow objects and their
renderers
https://bugs.webkit.org/show_bug.cgi?id=89000

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=151916&action=review


LGTM.

> Source/WebCore/dom/WebKitNamedFlowCollection.cpp:68
> +    RefPtr<WebKitNamedFlow> newFlow = WebKitNamedFlow::create(this,
flowName);
> +    m_namedFlows.add(newFlow.get());
> +
> +    return newFlow.release();

Some of the WTF add functions return what they've added.  It's possible you
could make these 3 into one line if that's true.

> Source/WebCore/rendering/RenderNamedFlowThread.h:83
> +    bool canBeDestroyed() const { return m_regionList.isEmpty() &&
m_contentNodes.isEmpty(); }

Is this virtual?  Or specific to RenderNamedFlowThread?  What about when the
whole RenderView tree goes down?  Will things correctly get cleaned up then?