[webkit-reviews] review granted: [Bug 32848] Avoid reloading iframe on re-parenting between documents. : [Attachment 47240] Patch v3

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has granted Dmitry Titov
<dimich at chromium.org>'s request for review:
Bug 32848: Avoid reloading iframe on re-parenting between documents.
https://bugs.webkit.org/show_bug.cgi?id=32848

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

------- Additional Comments from David Levin <levin at chromium.org>
OK the js test looks fine. One nit below.

> diff --git a/WebCore/html/HTMLFrameElementBase.cpp
b/WebCore/html/HTMLFrameElementBase.cpp
> @@ -50,8 +50,11 @@ HTMLFrameElementBase::HTMLFrameElementBase(const
QualifiedName& tagName, Documen
>      , m_scrolling(ScrollbarAuto)
>      , m_marginWidth(-1)
>      , m_marginHeight(-1)
> +    , m_checkAttachedTimer(this,
&HTMLFrameElementBase::checkAttachedTimerFired)
>      , m_viewSource(false)
>      , m_shouldOpenURLAfterAttach(false)
> +    , m_remainsAliveOnRemovalFromTree(false)
> +

Nit: Extra blank line here.


> +void
HTMLFrameElementBase::checkAttachedTimerFired(Timer<HTMLFrameElementBase>*)
> +{
> +    ASSERT(!attached());

Ah, so this enforces that setRemainsAliveOnRemovalFromTree is only called
during a remove operation (and that attach cancels the timer) which seems fine.