[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
eric at webkit.org
eric at webkit.org
Wed Dec 22 13:17:10 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit 8cc44245eeee3fa4fe4cb78d086c2217c508911f
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Sep 10 09:12:11 2010 +0000
2010-09-10 Eric Seidel <eric at webkit.org>
Reviewed by Dimitri Glazkov.
Remove support for non-lazy attach and fix frames to load from insertedIntoDocument instead of attach
https://bugs.webkit.org/show_bug.cgi?id=45365
* fast/dom/beforeload/remove-frame-in-beforeload-listener.html:
- Add a comment to make clear that this test intentionally
does not cancel the load in the beforeload event, but rather
relies on WebCore's own Node::willRemove() implementation to
cancel the load when the frame is removed.
(The load is not actually directly canceled per say, but the
loading logic checks to see if the frame is still in the frame
tree right after creating the frame and aborts if its not.
HTMLFrameOwnerElement::willRemove clears the frame owner pointer
causing the frame to be removed from the tree.)
2010-09-10 Eric Seidel <eric at webkit.org>
Reviewed by Dimitri Glazkov.
Remove support for non-lazy attach and fix frames to load from insertedIntoDocument instead of attach
https://bugs.webkit.org/show_bug.cgi?id=45365
This change is the last piece of the puzzle which was preventing us from
removing all eager-attach logic and moving WebCore to using an entirely
recalcStyle-driven, on-demand renderer creation system, instead of every
element being synchronously attached during parsing, regardless of whether
it was ever going to be displayed or not.
This does not change the places we call lazyAttach vs. attach. This only
changes the behavior of frame/plugin elements when lazyAttach is called.
Previously lazyAttach would eager-attach those elements (and their ancestors)
because they were marked as canLazyAttach() { return false; }.
This is a very tricky change, please review carefully.
Mostly I'm moving logic which used to be in attach() into
insertedIntoDocument. Once it was there, there was no longer any reason
why frame elements couldn't lazyAttach, thus removing the need
for the non-lazy attach code path entirely.
We've not yet converted all callsites over to using lazyAttach() instead
of attach() however.
In order to move frame loading logic into insertedIntoDocument
instead of attach, I needed to make sure that any javascript calls
during the load would see an attached element. Thus I needed to mark
the element as needing style resolve so that it would attach itself
if needed.
I was not able to just call lazyAttach() from insertedIntoDocument directly
due to two conflicting assumptions in the rendering tree:
1. ContainerNode::attach() assumes its "safe" to call attach on all children
without checking first if the child is already attached. This seems sane
since its strange to think of a subtree as being attached w/o ancestors
already being attached. Although it is a hard rule that subtrees may not
have renderers w/o ancestors also having renderers, I do not believe it's
a hard rule that subtrees may not be attached. Remember, attached() does
not imply renderer()! It's possible ContainerNode::attach()'s assumption is wrong here.
2. Node::attach() asserts !attached(). This makes sense and I would not
want to remove this assert, however it means that if insertedIntoDocument
were to call lazyAttach() (thus marking the element as attached()) then
all callers would need to check if the element was already attached after
insertedIntoDocument (called by appendChild, parserAppendChild, etc.)
before calling attach or lazyAttach(). The following example:
element.innerHTML = "<span><iframe></span>" is one case where this
ASSERT would be hit if insertedIntoDocument called lazyAttach, since
ContainerNode::attach() (called on the span by in appendChild(DocumentFragment) code)
does not check if iframe is already attached.
Note: One subtle change here is that synchronous javascript which results
from javascript: or beforeload is now run as part of insertedIntoDocument
(thus any insert/append call *even* parserAddChild) instead of being
run during attach (technically in the post-attach callbacks).
This is covered by numerous layout tests.
* dom/ContainerNode.cpp:
(WebCore::willRemoveChild):
(WebCore::willRemoveChildren):
- Since insertedIntoDocument starts a load and yet does not mark the
element as attached, we need to always call willRemove().
See note above as to why we don't just mark attached() in insertedIntoDocument.
* dom/Node.cpp:
(WebCore::Node::markAncestorsWithChildNeedsStyleRecalc):
- Share some code between lazyAttach and setNeedsStyleRecalc.
(WebCore::Node::setNeedsStyleRecalc):
- Use the new markAncestorsWithChildNeedsStyleRecalc
(WebCore::Node::lazyAttach):
- Remove the non-lazy code path, and use markAncestorsWithChildNeedsStyleRecalc.
- Add an option to lazyAttach without marking attached(), used by HTMLFrameElementBase::insertedIntoDocument.
* dom/Node.h:
* html/HTMLFrameElementBase.cpp:
- m_shouldOpenURLAfterAttach is no longer needed, yay!
- m_checkAttachedTimer no longer has anything to do with attached(), so renamed it.
I also documented that the newly named m_checkInDocumentTimer is all about the
"magic iframe" performance quirk. (Which is actually speced in HTML5).
I was initially baffled by this code, so I documented it.
(WebCore::HTMLFrameElementBase::HTMLFrameElementBase)
(WebCore::HTMLFrameElementBase::insertedIntoDocument):
- This is the meat of this change, see discussion above.
(WebCore::HTMLFrameElementBase::attach):
- Code deleted or moved to insertedIntoDocument.
(WebCore::HTMLFrameElementBase::width):
- Fixed a bug in height()/width() which was probably causing crashes
and was causing incorrect behavior after this change.
renderBox() is not necessarily valid unless layout is up to date.
Updating layout, can cause renderBox() to go invalid, thus this
could have been a null-pointer crash.
(WebCore::HTMLFrameElementBase::height): see width()
(WebCore::HTMLFrameElementBase::setRemainsAliveOnRemovalFromTree): Timer rename.
(WebCore::HTMLFrameElementBase::checkInDocumentTimerFired): Timer rename.
* html/HTMLFrameElementBase.h:
* html/HTMLFrameOwnerElement.cpp:
(WebCore::HTMLFrameOwnerElement::willRemove):
- Disconnecting the owner element removes the frame from the frame tree.
frameDetached() calls Page::frameCount which expects that the frame is
already gone at this point and asserts when it's not. It's unclear how
this worked before, except that the frame removal was likely done in the
post-attach callback, so the frameCount was wrong (too high) during
frameDetached(), but was fixed up in the post-detach callback.
* html/parser/HTMLConstructionSite.cpp:
(WebCore::HTMLConstructionSite::attachAtSite):
- Simplified this code, and added a check for the case when the node was already removed.
Since the load logic is now run during insertedIntoDocument instead of attach(),
synchronous javascript is now running during insertedIntoDocument and we need to
make sure that the child is still in the tree.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@67182 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 9a776b6..a904dc9 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,21 @@
+2010-09-10 Eric Seidel <eric at webkit.org>
+
+ Reviewed by Dimitri Glazkov.
+
+ Remove support for non-lazy attach and fix frames to load from insertedIntoDocument instead of attach
+ https://bugs.webkit.org/show_bug.cgi?id=45365
+
+ * fast/dom/beforeload/remove-frame-in-beforeload-listener.html:
+ - Add a comment to make clear that this test intentionally
+ does not cancel the load in the beforeload event, but rather
+ relies on WebCore's own Node::willRemove() implementation to
+ cancel the load when the frame is removed.
+ (The load is not actually directly canceled per say, but the
+ loading logic checks to see if the frame is still in the frame
+ tree right after creating the frame and aborts if its not.
+ HTMLFrameOwnerElement::willRemove clears the frame owner pointer
+ causing the frame to be removed from the tree.)
+
2010-09-10 Kent Tamura <tkent at chromium.org>
Unreviewed, test expectation update.
diff --git a/LayoutTests/fast/dom/beforeload/remove-frame-in-beforeload-listener.html b/LayoutTests/fast/dom/beforeload/remove-frame-in-beforeload-listener.html
index fe45195..8d0f514 100644
--- a/LayoutTests/fast/dom/beforeload/remove-frame-in-beforeload-listener.html
+++ b/LayoutTests/fast/dom/beforeload/remove-frame-in-beforeload-listener.html
@@ -8,6 +8,10 @@
document.addEventListener("beforeload", function(event) {
if (event.target && event.target.parentElement)
event.target.parentElement.removeChild(event.target);
+ // Note, we intentionally do not cancel the load here,
+ // WebCore should automatically cancel it.
+ // Otherwise DRT will print:
+ // Blocked access to external URL http://webkit.org/
}, true);
</script>
</head>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 45d8cc3..a40f4d7 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,114 @@
+2010-09-10 Eric Seidel <eric at webkit.org>
+
+ Reviewed by Dimitri Glazkov.
+
+ Remove support for non-lazy attach and fix frames to load from insertedIntoDocument instead of attach
+ https://bugs.webkit.org/show_bug.cgi?id=45365
+
+ This change is the last piece of the puzzle which was preventing us from
+ removing all eager-attach logic and moving WebCore to using an entirely
+ recalcStyle-driven, on-demand renderer creation system, instead of every
+ element being synchronously attached during parsing, regardless of whether
+ it was ever going to be displayed or not.
+
+ This does not change the places we call lazyAttach vs. attach. This only
+ changes the behavior of frame/plugin elements when lazyAttach is called.
+ Previously lazyAttach would eager-attach those elements (and their ancestors)
+ because they were marked as canLazyAttach() { return false; }.
+
+ This is a very tricky change, please review carefully.
+
+ Mostly I'm moving logic which used to be in attach() into
+ insertedIntoDocument. Once it was there, there was no longer any reason
+ why frame elements couldn't lazyAttach, thus removing the need
+ for the non-lazy attach code path entirely.
+ We've not yet converted all callsites over to using lazyAttach() instead
+ of attach() however.
+
+ In order to move frame loading logic into insertedIntoDocument
+ instead of attach, I needed to make sure that any javascript calls
+ during the load would see an attached element. Thus I needed to mark
+ the element as needing style resolve so that it would attach itself
+ if needed.
+
+ I was not able to just call lazyAttach() from insertedIntoDocument directly
+ due to two conflicting assumptions in the rendering tree:
+ 1. ContainerNode::attach() assumes its "safe" to call attach on all children
+ without checking first if the child is already attached. This seems sane
+ since its strange to think of a subtree as being attached w/o ancestors
+ already being attached. Although it is a hard rule that subtrees may not
+ have renderers w/o ancestors also having renderers, I do not believe it's
+ a hard rule that subtrees may not be attached. Remember, attached() does
+ not imply renderer()! It's possible ContainerNode::attach()'s assumption is wrong here.
+ 2. Node::attach() asserts !attached(). This makes sense and I would not
+ want to remove this assert, however it means that if insertedIntoDocument
+ were to call lazyAttach() (thus marking the element as attached()) then
+ all callers would need to check if the element was already attached after
+ insertedIntoDocument (called by appendChild, parserAppendChild, etc.)
+ before calling attach or lazyAttach(). The following example:
+ element.innerHTML = "<span><iframe></span>" is one case where this
+ ASSERT would be hit if insertedIntoDocument called lazyAttach, since
+ ContainerNode::attach() (called on the span by in appendChild(DocumentFragment) code)
+ does not check if iframe is already attached.
+
+ Note: One subtle change here is that synchronous javascript which results
+ from javascript: or beforeload is now run as part of insertedIntoDocument
+ (thus any insert/append call *even* parserAddChild) instead of being
+ run during attach (technically in the post-attach callbacks).
+
+ This is covered by numerous layout tests.
+
+ * dom/ContainerNode.cpp:
+ (WebCore::willRemoveChild):
+ (WebCore::willRemoveChildren):
+ - Since insertedIntoDocument starts a load and yet does not mark the
+ element as attached, we need to always call willRemove().
+ See note above as to why we don't just mark attached() in insertedIntoDocument.
+ * dom/Node.cpp:
+ (WebCore::Node::markAncestorsWithChildNeedsStyleRecalc):
+ - Share some code between lazyAttach and setNeedsStyleRecalc.
+ (WebCore::Node::setNeedsStyleRecalc):
+ - Use the new markAncestorsWithChildNeedsStyleRecalc
+ (WebCore::Node::lazyAttach):
+ - Remove the non-lazy code path, and use markAncestorsWithChildNeedsStyleRecalc.
+ - Add an option to lazyAttach without marking attached(), used by HTMLFrameElementBase::insertedIntoDocument.
+ * dom/Node.h:
+ * html/HTMLFrameElementBase.cpp:
+ - m_shouldOpenURLAfterAttach is no longer needed, yay!
+ - m_checkAttachedTimer no longer has anything to do with attached(), so renamed it.
+ I also documented that the newly named m_checkInDocumentTimer is all about the
+ "magic iframe" performance quirk. (Which is actually speced in HTML5).
+ I was initially baffled by this code, so I documented it.
+ (WebCore::HTMLFrameElementBase::HTMLFrameElementBase)
+ (WebCore::HTMLFrameElementBase::insertedIntoDocument):
+ - This is the meat of this change, see discussion above.
+ (WebCore::HTMLFrameElementBase::attach):
+ - Code deleted or moved to insertedIntoDocument.
+ (WebCore::HTMLFrameElementBase::width):
+ - Fixed a bug in height()/width() which was probably causing crashes
+ and was causing incorrect behavior after this change.
+ renderBox() is not necessarily valid unless layout is up to date.
+ Updating layout, can cause renderBox() to go invalid, thus this
+ could have been a null-pointer crash.
+ (WebCore::HTMLFrameElementBase::height): see width()
+ (WebCore::HTMLFrameElementBase::setRemainsAliveOnRemovalFromTree): Timer rename.
+ (WebCore::HTMLFrameElementBase::checkInDocumentTimerFired): Timer rename.
+ * html/HTMLFrameElementBase.h:
+ * html/HTMLFrameOwnerElement.cpp:
+ (WebCore::HTMLFrameOwnerElement::willRemove):
+ - Disconnecting the owner element removes the frame from the frame tree.
+ frameDetached() calls Page::frameCount which expects that the frame is
+ already gone at this point and asserts when it's not. It's unclear how
+ this worked before, except that the frame removal was likely done in the
+ post-attach callback, so the frameCount was wrong (too high) during
+ frameDetached(), but was fixed up in the post-detach callback.
+ * html/parser/HTMLConstructionSite.cpp:
+ (WebCore::HTMLConstructionSite::attachAtSite):
+ - Simplified this code, and added a check for the case when the node was already removed.
+ Since the load logic is now run during insertedIntoDocument instead of attach(),
+ synchronous javascript is now running during insertedIntoDocument and we need to
+ make sure that the child is still in the tree.
+
2010-09-10 Dirk Pranke <dpranke at chromium.org>
Unreviewed, rolling out r67178.
diff --git a/WebCore/dom/ContainerNode.cpp b/WebCore/dom/ContainerNode.cpp
index 46de749..1e337b0 100644
--- a/WebCore/dom/ContainerNode.cpp
+++ b/WebCore/dom/ContainerNode.cpp
@@ -359,9 +359,7 @@ static void willRemoveChild(Node* child)
// fire removed from document mutation events.
dispatchChildRemovalEvents(child);
-
- if (child->attached())
- child->willRemove();
+ child->willRemove();
}
static void willRemoveChildren(ContainerNode* container)
@@ -373,9 +371,7 @@ static void willRemoveChildren(ContainerNode* container)
for (RefPtr<Node> child = container->firstChild(); child; child = child->nextSibling()) {
// fire removed from document mutation events.
dispatchChildRemovalEvents(child.get());
-
- if (child->attached())
- child->willRemove();
+ child->willRemove();
}
}
diff --git a/WebCore/dom/Node.cpp b/WebCore/dom/Node.cpp
index f6554de..8ff822a 100644
--- a/WebCore/dom/Node.cpp
+++ b/WebCore/dom/Node.cpp
@@ -718,6 +718,15 @@ inline void Node::setStyleChange(StyleChangeType changeType)
m_nodeFlags = (m_nodeFlags & ~StyleChangeMask) | changeType;
}
+inline void Node::markAncestorsWithChildNeedsStyleRecalc()
+{
+ for (Node* p = parentNode(); p && !p->childNeedsStyleRecalc(); p = p->parentNode())
+ p->setChildNeedsStyleRecalc();
+
+ if (document()->childNeedsStyleRecalc())
+ document()->scheduleStyleRecalc();
+}
+
void Node::setNeedsStyleRecalc(StyleChangeType changeType)
{
ASSERT(changeType != NoStyleChange);
@@ -728,49 +737,20 @@ void Node::setNeedsStyleRecalc(StyleChangeType changeType)
if (changeType > existingChangeType)
setStyleChange(changeType);
- if (existingChangeType == NoStyleChange) {
- for (Node* p = parentNode(); p && !p->childNeedsStyleRecalc(); p = p->parentNode())
- p->setChildNeedsStyleRecalc();
-
- if (document()->childNeedsStyleRecalc())
- document()->scheduleStyleRecalc();
- }
+ if (existingChangeType == NoStyleChange)
+ markAncestorsWithChildNeedsStyleRecalc();
}
-static Node* outermostLazyAttachedAncestor(Node* start)
+void Node::lazyAttach(ShouldSetAttached shouldSetAttached)
{
- Node* p = start;
- for (Node* next = p->parentNode(); !next->renderer(); p = next, next = next->parentNode()) {}
- return p;
-}
-
-void Node::lazyAttach()
-{
- bool mustDoFullAttach = false;
-
for (Node* n = this; n; n = n->traverseNextNode(this)) {
- if (!n->canLazyAttach()) {
- mustDoFullAttach = true;
- break;
- }
-
if (n->firstChild())
n->setChildNeedsStyleRecalc();
n->setStyleChange(FullStyleChange);
- n->setAttached();
- }
-
- if (mustDoFullAttach) {
- Node* lazyAttachedAncestor = outermostLazyAttachedAncestor(this);
- if (lazyAttachedAncestor->attached())
- lazyAttachedAncestor->detach();
- lazyAttachedAncestor->attach();
- } else {
- for (Node* p = parentNode(); p && !p->childNeedsStyleRecalc(); p = p->parentNode())
- p->setChildNeedsStyleRecalc();
- if (document()->childNeedsStyleRecalc())
- document()->scheduleStyleRecalc();
+ if (shouldSetAttached == SetAttached)
+ n->setAttached();
}
+ markAncestorsWithChildNeedsStyleRecalc();
}
void Node::setFocus(bool b)
diff --git a/WebCore/dom/Node.h b/WebCore/dom/Node.h
index 258c8fd..587effd 100644
--- a/WebCore/dom/Node.h
+++ b/WebCore/dom/Node.h
@@ -310,8 +310,11 @@ public:
void setIsLink() { setFlag(IsLinkFlag); }
void clearIsLink() { clearFlag(IsLinkFlag); }
- void lazyAttach();
- virtual bool canLazyAttach() { return true; }
+ enum ShouldSetAttached {
+ SetAttached,
+ DoNotSetAttached
+ };
+ void lazyAttach(ShouldSetAttached = SetAttached);
virtual void setFocus(bool b = true);
virtual void setActive(bool f = true, bool /*pause*/ = false) { setFlag(f, IsActiveFlag); }
@@ -681,6 +684,9 @@ private:
void setStyleChange(StyleChangeType);
+ // Used to share code between lazyAttach and setNeedsStyleRecalc.
+ void markAncestorsWithChildNeedsStyleRecalc();
+
virtual void refEventTarget() { ref(); }
virtual void derefEventTarget() { deref(); }
diff --git a/WebCore/html/HTMLFrameElementBase.cpp b/WebCore/html/HTMLFrameElementBase.cpp
index b967926..8cca465 100644
--- a/WebCore/html/HTMLFrameElementBase.cpp
+++ b/WebCore/html/HTMLFrameElementBase.cpp
@@ -52,9 +52,8 @@ HTMLFrameElementBase::HTMLFrameElementBase(const QualifiedName& tagName, Documen
, m_scrolling(ScrollbarAuto)
, m_marginWidth(-1)
, m_marginHeight(-1)
- , m_checkAttachedTimer(this, &HTMLFrameElementBase::checkAttachedTimerFired)
+ , m_checkInDocumentTimer(this, &HTMLFrameElementBase::checkInDocumentTimerFired)
, m_viewSource(false)
- , m_shouldOpenURLAfterAttach(false)
, m_remainsAliveOnRemovalFromTree(false)
{
}
@@ -183,33 +182,31 @@ void HTMLFrameElementBase::updateOnReparenting()
void HTMLFrameElementBase::insertedIntoDocument()
{
HTMLFrameOwnerElement::insertedIntoDocument();
-
- // We delay frame loading until after the render tree is fully constructed.
- // Othewise, a synchronous load that executed JavaScript would see incorrect
- // (0) values for the frame's renderer-dependent properties, like width.
- m_shouldOpenURLAfterAttach = true;
- if (m_remainsAliveOnRemovalFromTree)
+ if (m_remainsAliveOnRemovalFromTree) {
updateOnReparenting();
-}
-
-void HTMLFrameElementBase::removedFromDocument()
-{
- m_shouldOpenURLAfterAttach = false;
+ setRemainsAliveOnRemovalFromTree(false);
+ return;
+ }
+ // DocumentFragments don't kick of any loads.
+ if (!document()->frame())
+ return;
- HTMLFrameOwnerElement::removedFromDocument();
+ // Loads may cause synchronous javascript execution (e.g. beforeload or
+ // src=javascript), which could try to access the renderer before the normal
+ // parser machinery would call lazyAttach() and set us as needing style
+ // resolve. Any code which expects this to be attached will resolve style
+ // before using renderer(), so this will make sure we attach in time.
+ // FIXME: Normally lazyAttach marks the renderer as attached(), but we don't
+ // want to do that here, as as callers expect to call attach() right after
+ // this and attach() will ASSERT(!attached())
+ ASSERT(!renderer()); // This recalc is unecessary if we already have a renderer.
+ lazyAttach(DoNotSetAttached);
+ setNameAndOpenURL();
}
void HTMLFrameElementBase::attach()
{
- if (m_shouldOpenURLAfterAttach) {
- m_shouldOpenURLAfterAttach = false;
- if (!m_remainsAliveOnRemovalFromTree)
- queuePostAttachCallback(&HTMLFrameElementBase::setNameAndOpenURLCallback, this);
- }
-
- setRemainsAliveOnRemovalFromTree(false);
-
HTMLFrameOwnerElement::attach();
if (RenderPart* part = renderPart()) {
@@ -258,19 +255,17 @@ bool HTMLFrameElementBase::isURLAttribute(Attribute *attr) const
int HTMLFrameElementBase::width() const
{
+ document()->updateLayoutIgnorePendingStylesheets();
if (!renderBox())
return 0;
-
- document()->updateLayoutIgnorePendingStylesheets();
return renderBox()->width();
}
int HTMLFrameElementBase::height() const
{
+ document()->updateLayoutIgnorePendingStylesheets();
if (!renderBox())
return 0;
-
- document()->updateLayoutIgnorePendingStylesheets();
return renderBox()->height();
}
@@ -281,12 +276,12 @@ void HTMLFrameElementBase::setRemainsAliveOnRemovalFromTree(bool value)
// There is a possibility that JS will do document.adoptNode() on this element but will not insert it into the tree.
// Start the async timer that is normally stopped by attach(). If it's not stopped and fires, it'll unload the frame.
if (value)
- m_checkAttachedTimer.startOneShot(0);
+ m_checkInDocumentTimer.startOneShot(0);
else
- m_checkAttachedTimer.stop();
+ m_checkInDocumentTimer.stop();
}
-void HTMLFrameElementBase::checkAttachedTimerFired(Timer<HTMLFrameElementBase>*)
+void HTMLFrameElementBase::checkInDocumentTimerFired(Timer<HTMLFrameElementBase>*)
{
ASSERT(!attached());
ASSERT(m_remainsAliveOnRemovalFromTree);
diff --git a/WebCore/html/HTMLFrameElementBase.h b/WebCore/html/HTMLFrameElementBase.h
index 1b6e2a3..ae41e75 100644
--- a/WebCore/html/HTMLFrameElementBase.h
+++ b/WebCore/html/HTMLFrameElementBase.h
@@ -50,15 +50,10 @@ protected:
bool isURLAllowed() const;
virtual void parseMappedAttribute(Attribute*);
-
virtual void insertedIntoDocument();
- virtual void removedFromDocument();
-
virtual void attach();
private:
- virtual bool canLazyAttach() { return false; }
-
virtual bool supportsFocus() const;
virtual void setFocus(bool);
@@ -67,7 +62,7 @@ private:
virtual void setName();
virtual void willRemove();
- void checkAttachedTimerFired(Timer<HTMLFrameElementBase>*);
+ void checkInDocumentTimerFired(Timer<HTMLFrameElementBase>*);
void updateOnReparenting();
bool viewSourceMode() const { return m_viewSource; }
@@ -85,12 +80,19 @@ private:
int m_marginWidth;
int m_marginHeight;
- Timer<HTMLFrameElementBase> m_checkAttachedTimer;
+ // This is a performance optimization some call "magic iframe" which avoids
+ // tearing down the frame hierarchy when a web page calls adoptNode on a
+ // frame owning element but does not immediately insert it into the new
+ // document before JavaScript yields to WebCore. If the element is not yet
+ // in a document by the time this timer fires, the frame hierarchy teardown
+ // will continue. This can also be seen as implementation of:
+ // "Removing an iframe from a Document does not cause its browsing context
+ // to be discarded. Indeed, an iframe's browsing context can survive its
+ // original parent Document if its iframe is moved to another Document."
+ // From HTML5: http://www.whatwg.org/specs/web-apps/current-work/multipage/the-iframe-element.html#the-iframe-element
+ Timer<HTMLFrameElementBase> m_checkInDocumentTimer;
bool m_viewSource;
-
- bool m_shouldOpenURLAfterAttach;
-
bool m_remainsAliveOnRemovalFromTree;
};
diff --git a/WebCore/html/HTMLFrameOwnerElement.cpp b/WebCore/html/HTMLFrameOwnerElement.cpp
index 3f946f0..b409bcc 100644
--- a/WebCore/html/HTMLFrameOwnerElement.cpp
+++ b/WebCore/html/HTMLFrameOwnerElement.cpp
@@ -51,9 +51,11 @@ RenderPart* HTMLFrameOwnerElement::renderPart() const
void HTMLFrameOwnerElement::willRemove()
{
+ // FIXME: It is unclear why this can't be moved to removedFromDocument()
+ // this is the only implementation of willRemove in WebCore!
if (Frame* frame = contentFrame()) {
- frame->disconnectOwnerElement();
frame->loader()->frameDetached();
+ frame->disconnectOwnerElement();
}
HTMLElement::willRemove();
diff --git a/WebCore/html/HTMLPlugInImageElement.h b/WebCore/html/HTMLPlugInImageElement.h
index 60ad0e6..9616fe8 100644
--- a/WebCore/html/HTMLPlugInImageElement.h
+++ b/WebCore/html/HTMLPlugInImageElement.h
@@ -59,7 +59,6 @@ protected:
bool wouldLoadAsNetscapePlugin(const String& url, const String& serviceType);
private:
- virtual bool canLazyAttach() { return false; }
virtual RenderObject* createRenderer(RenderArena*, RenderStyle*);
virtual void recalcStyle(StyleChange);
diff --git a/WebCore/html/parser/HTMLConstructionSite.cpp b/WebCore/html/parser/HTMLConstructionSite.cpp
index 0172b3d..6215bba 100644
--- a/WebCore/html/parser/HTMLConstructionSite.cpp
+++ b/WebCore/html/parser/HTMLConstructionSite.cpp
@@ -114,19 +114,19 @@ PassRefPtr<ChildType> HTMLConstructionSite::attach(ContainerNode* parent, PassRe
void HTMLConstructionSite::attachAtSite(const AttachmentSite& site, PassRefPtr<Node> prpChild)
{
+ // FIXME: It's unfortunate that we need to hold a reference to child
+ // here to call attach(). We should investigate whether we can rely on
+ // |site.parent| to hold a ref at this point.
RefPtr<Node> child = prpChild;
- if (site.nextChild) {
+ if (site.nextChild)
site.parent->parserInsertBefore(child, site.nextChild);
- if (site.parent->attached() && !child->attached())
- child->attach();
- return;
- }
- site.parent->parserAddChild(child);
- // It's slightly unfortunate that we need to hold a reference to child
- // here to call attach(). We should investigate whether we can rely on
- // |site.parent| to hold a ref at this point.
- if (site.parent->attached() && !child->attached())
+ else
+ site.parent->parserAddChild(child);
+
+ // JavaScript run from beforeload (or DOM Mutation or event handlers)
+ // might have removed the child, in which case we should not attach it.
+ if (child->parentNode() && site.parent->attached() && !child->attached())
child->attach();
}
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list