Changeset 21447 in webkit

Timestamp:

May 13, 2007 8:28:01 AM (17 years ago)

Author:

beidson

Message:

Reviewed by Mitz

​http://bugs.webkit.org/show_bug.cgi?id=13701

Since HTMLInputElements register themselves with their Document for special handling, if their document
ever changes then bad things can happen.

Unfortunately, with adoptNode, the document can change. This has been a long standing crasher with all
input fields but was given recent high visibility by r21430

The solution is to add functionality for a Node to react to its owner document changing via a pair of
virtual callbacks. Then we can tap into those callbacks in HTMLInputElement.

No layout test possible until ​http://bugs.webkit.org/show_bug.cgi?id=13672 is resolved

• dom/Node.cpp: (WebCore::Node::setDocument): Call willMoveToNewOwnerDocument and didMoveToNewOwnerDocument

• dom/Node.h: (WebCore::Node::willMoveToNewOwnerDocument): Virtual callback to a node for before an owner document change (WebCore::Node::didMoveToNewOwnerDocument): Virtual callback to a node for after an owner document change

• html/HTMLInputElement.cpp: (WebCore::HTMLInputElement::~HTMLInputElement): Unconditionally unregister both for both form state and the cache restoration callback (WebCore::HTMLInputElement::setInputType): Change registration with the document only if we're in one (WebCore::HTMLInputElement::attach): Don't bother registering for the callback here - that will be handled by setInputType() or by a change in owner document (WebCore::HTMLInputElement::willMoveToNewOwnerDocument): Unregister with the old owner document (WebCore::HTMLInputElement::didMoveToNewOwnerDocument): Register with the new owner document
• html/HTMLInputElement.h:

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Node.cpp (modified) (1 diff)
• dom/Node.h (modified) (1 diff)
• html/HTMLInputElement.cpp (modified) (4 diffs)
• html/HTMLInputElement.h (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-05-13  Brady Eidson  <beidson@apple.com>
+
+        Reviewed by Mitz
+
+        http://bugs.webkit.org/show_bug.cgi?id=13701
+
+        Since HTMLInputElements register themselves with their Document for special handling, if their document
+        ever changes then bad things can happen.
+
+        Unfortunately, with adoptNode, the document can change.  This has been a long standing crasher with all 
+        input fields but was given recent high visibility by r21430
+
+        The solution is to add functionality for a Node to react to its owner document changing via a pair of 
+        virtual callbacks.  Then we can tap into those callbacks in HTMLInputElement.
+
+        No layout test possible until http://bugs.webkit.org/show_bug.cgi?id=13672 is resolved
+
+        * dom/Node.cpp:
+        (WebCore::Node::setDocument): Call willMoveToNewOwnerDocument and didMoveToNewOwnerDocument
+
+        * dom/Node.h:
+        (WebCore::Node::willMoveToNewOwnerDocument): Virtual callback to a node for before an owner document change
+        (WebCore::Node::didMoveToNewOwnerDocument): Virtual callback to a node for after an owner document change
+
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::~HTMLInputElement): Unconditionally unregister both for both form state
+          and the cache restoration callback
+        (WebCore::HTMLInputElement::setInputType): Change registration with the document only if we're in one
+        (WebCore::HTMLInputElement::attach): Don't bother registering for the callback here - that will be
+          handled by setInputType() or by a change in owner document
+        (WebCore::HTMLInputElement::willMoveToNewOwnerDocument): Unregister with the old owner document
+        (WebCore::HTMLInputElement::didMoveToNewOwnerDocument): Register with the new owner document
+        * html/HTMLInputElement.h:
+
 2007-05-13  Darin Adler  <darin@apple.com>
 

trunk/WebCore/dom/Node.cpp

@@ -169 +169 @@
         return;
 
+    willMoveToNewOwnerDocument();
+
     {
         KJS::JSLock lock;
         KJS::ScriptInterpreter::updateDOMNodeDocument(this, m_document.get(), doc);
-    }
+    }    
     m_document = doc;
+
+    didMoveToNewOwnerDocument();
 }
 

trunk/WebCore/dom/Node.h

@@ -456 +456 @@
 
 protected:
+    virtual void willMoveToNewOwnerDocument() { }
+    virtual void didMoveToNewOwnerDocument() { }
+    
     NodeListsNodeData* m_nodeLists;
 

trunk/WebCore/html/HTMLInputElement.cpp

@@ -137 +137 @@
 HTMLInputElement::~HTMLInputElement()
 {
-    document()->unregisterFormElementWithState(this);
-    document()->unregisterForDidRestoreFromCacheCallback(this);
+    ownerDocument()->unregisterFormElementWithState(this);
+    ownerDocument()->unregisterForDidRestoreFromCacheCallback(this);
     delete m_imageLoader;
 }
@@ -324 +324 @@
 
             if (wasPasswordField && !isPasswordField) {
-                document()->registerFormElementWithState(this);
-                document()->unregisterForDidRestoreFromCacheCallback(this);
+                ownerDocument()->registerFormElementWithState(this);
+                ownerDocument()->unregisterForDidRestoreFromCacheCallback(this);
             } else if (!wasPasswordField && isPasswordField) {
-                document()->unregisterFormElementWithState(this);
-                document()->registerForDidRestoreFromCacheCallback(this);
+                ownerDocument()->unregisterFormElementWithState(this);
+                ownerDocument()->registerForDidRestoreFromCacheCallback(this);
             }
 
@@ -762 +762 @@
         }
     }
-
-    if (inputType() == PASSWORD)
-        document()->registerForDidRestoreFromCacheCallback(this);
 }
 
@@ -1479 +1476 @@
     reset();
 }
+
+void HTMLInputElement::willMoveToNewOwnerDocument()
+{
+    if (inputType() == PASSWORD)
+        ownerDocument()->unregisterForDidRestoreFromCacheCallback(this);
+    else
+        ownerDocument()->unregisterFormElementWithState(this);
+        
+    HTMLGenericFormElement::willMoveToNewOwnerDocument();
+}
+
+void HTMLInputElement::didMoveToNewOwnerDocument()
+{
+    if (inputType() == PASSWORD)
+        document()->registerForDidRestoreFromCacheCallback(this);
+    else
+        document()->registerFormElementWithState(this);
+        
+    HTMLGenericFormElement::didMoveToNewOwnerDocument();
+}
     
 } // namespace

trunk/WebCore/html/HTMLInputElement.h

@@ -187 +187 @@
     
 protected:
+    virtual void willMoveToNewOwnerDocument();
+    virtual void didMoveToNewOwnerDocument();
+    
     AtomicString m_name;