2009-07-29 Nikolas Zimmermann <nikolas.zimmermann@torchmobile.com>
authorzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Jul 2009 13:18:30 +0000 (13:18 +0000)
committerzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Jul 2009 13:18:30 +0000 (13:18 +0000)
        Reviewed by Adam Treat.

        [WML] Running WML tests in random order multiple times exposes subtle bugs
        https://bugs.webkit.org/show_bug.cgi?id=27801

        Some changes to fix random order WML tests, simplilfy WMLTestCase.js and reset testDocument
        properly in enter-first-card-with-events.js. fast/wml/err-multi-access.wml still creates a layout
        test difference on consecutive runs, though that's related to bug 27721, which has to be fixed first.

        * wml/resources/WMLTestCase.js:
        * wml/resources/enter-first-card-with-events.js:
        (setupTestDocument):
        (prepareTest):
        (executeTest):

2009-07-29  Nikolas Zimmermann  <nikolas.zimmermann@torchmobile.com>

        Reviewed by Adam Treat.

        [WML] Running WML tests in random order multiple times exposes subtle bugs
        https://bugs.webkit.org/show_bug.cgi?id=27801

        Remove superflous assertions regarding the parent node. Under certain circumstances
        these can even fire (related to garbage collection while destructing). Fixes random order
        WML tests (run-webkit-tests fast/wml wml http/tests/wml fast/wml ... --random)

        The wml/enter-first-card-with-events.html test relied on a bug in our implementation of
        WMLPageState::reset() - the history stack should still contain the current card afterwards.
        Fix that bug by preserving the first item in BackForwardList::clearWMLPageHistory().

        * history/BackForwardList.cpp: Preserve first item in history stack, as demanded by the spec.
        (WebCore::BackForwardList::clearWMLPageHistory):
        * wml/WMLDoElement.cpp:
        (WebCore::WMLDoElement::insertedIntoDocument):
        (WebCore::WMLDoElement::removedFromDocument):
        * wml/WMLNoopElement.cpp:
        (WebCore::WMLNoopElement::insertedIntoDocument):
        * wml/WMLOnEventElement.cpp:
        (WebCore::eventHandlingParent):
        * wml/WMLPostfieldElement.cpp:
        (WebCore::WMLPostfieldElement::insertedIntoDocument):
        (WebCore::WMLPostfieldElement::removedFromDocument):
        * wml/WMLSetvarElement.cpp:
        (WebCore::WMLSetvarElement::insertedIntoDocument):
        (WebCore::WMLSetvarElement::removedFromDocument):
        * wml/WMLTaskElement.cpp:
        (WebCore::WMLTaskElement::insertedIntoDocument):
        (WebCore::WMLTaskElement::removedFromDocument):
        * wml/WMLTimerElement.cpp:
        (WebCore::WMLTimerElement::insertedIntoDocument):
        (WebCore::WMLTimerElement::removedFromDocument):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@46539 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/wml/resources/WMLTestCase.js
LayoutTests/wml/resources/enter-first-card-with-events.js
WebCore/ChangeLog
WebCore/history/BackForwardList.cpp
WebCore/wml/WMLDoElement.cpp
WebCore/wml/WMLNoopElement.cpp
WebCore/wml/WMLOnEventElement.cpp
WebCore/wml/WMLPostfieldElement.cpp
WebCore/wml/WMLSetvarElement.cpp
WebCore/wml/WMLTaskElement.cpp
WebCore/wml/WMLTimerElement.cpp

index 1230d55..8a0afc4 100644 (file)
@@ -1,3 +1,20 @@
+2009-07-29  Nikolas Zimmermann  <nikolas.zimmermann@torchmobile.com>
+
+        Reviewed by Adam Treat.
+
+        [WML] Running WML tests in random order multiple times exposes subtle bugs
+        https://bugs.webkit.org/show_bug.cgi?id=27801
+
+        Some changes to fix random order WML tests, simplilfy WMLTestCase.js and reset testDocument
+        properly in enter-first-card-with-events.js. fast/wml/err-multi-access.wml still creates a layout
+        test difference on consecutive runs, though that's related to bug 27721, which has to be fixed first.
+
+        * wml/resources/WMLTestCase.js:
+        * wml/resources/enter-first-card-with-events.js:
+        (setupTestDocument):
+        (prepareTest):
+        (executeTest):
+
 2009-07-29  Jan Michael Alonzo  <jmalonzo@webkit.org>
 
         Reviewed by Eric Seidel and Xan Lopez.
index 2f9ceb4..aa0a8ab 100644 (file)
@@ -9,32 +9,24 @@ function createWMLElement(name) {
     return testDocument.createElementNS(wmlNS, "wml:" + name);
 }
 
-function createWMLTestCase(testDescription, substitutesVariables, testName, executeImmediately, needsReset) {
+function createWMLTestCase(testDescription, substitutesVariables, testName, executeImmediately) {
     // Setup default test options
-    if (substitutesVariables == null) {
+    if (substitutesVariables == null)
         substitutesVariables = true;
-    }
 
     // Setup default test name
     if (testName == null) {
         executeImmediately = false; // Only honored, when testName != null
         testName = relativePathToLayoutTests + "/wml/resources/test-document.wml";
-    } else if (executeImmediately == null) {
+    } else if (executeImmediately == null)
         executeImmediately = true;
-    }
-
-    // Some tests may want to handle resetting the page state themselves
-    if (needsReset == null) {
-        needsReset = true;
-    }
 
     // Initialize JS test
     description(testDescription);
     bodyElement = document.getElementsByTagName("body")[0];
 
     // Clear variable state & history
-    if (needsReset)
-        document.resetWMLPageState();
+    document.resetWMLPageState();
 
     // Setup DRT specific settings
     if (window.layoutTestController) {
index 0d2b91e..260e659 100644 (file)
@@ -1,23 +1,22 @@
 /// [Name] enter-first-card-with-events.js
 
-createWMLTestCase("Tests entering first card backwards that has intrinsic events set", false, "resources/enter-first-card-with-events.wml", false, false);
-
-var ranOnce = false;
-var resultIndicatorElement;
+createWMLTestCase("Tests entering first card backwards that has intrinsic events set", false, "resources/enter-first-card-with-events.wml", false);
 
 function setupTestDocument() {
-    resultIndicatorElement = testDocument.getElementById("resultIndicator");
+    var resultIndicatorElement = testDocument.getElementById("resultIndicator");
+    if (resultIndicatorElement.textContent == "DONE")
+        completeTest();
 }
 
 function prepareTest() {
-    if (resultIndicatorElement.textContent == "DONE")
-        completeTest();
-    else
-        executeTest();
+    startTest(25, 15);
 }
 
 function executeTest() {
     startTest(25, 15);
+
+    // Force re-setup of the test document, as we check for completion upon setupTestDocument()
+    testDocument = undefined;
 }
 
 var successfullyParsed = true;
index 99d32c5..c2288d2 100644 (file)
@@ -1,3 +1,40 @@
+2009-07-29  Nikolas Zimmermann  <nikolas.zimmermann@torchmobile.com>
+
+        Reviewed by Adam Treat.
+
+        [WML] Running WML tests in random order multiple times exposes subtle bugs
+        https://bugs.webkit.org/show_bug.cgi?id=27801
+
+        Remove superflous assertions regarding the parent node. Under certain circumstances
+        these can even fire (related to garbage collection while destructing). Fixes random order
+        WML tests (run-webkit-tests fast/wml wml http/tests/wml fast/wml ... --random)
+
+        The wml/enter-first-card-with-events.html test relied on a bug in our implementation of
+        WMLPageState::reset() - the history stack should still contain the current card afterwards.
+        Fix that bug by preserving the first item in BackForwardList::clearWMLPageHistory().
+
+        * history/BackForwardList.cpp: Preserve first item in history stack, as demanded by the spec.
+        (WebCore::BackForwardList::clearWMLPageHistory):
+        * wml/WMLDoElement.cpp:
+        (WebCore::WMLDoElement::insertedIntoDocument):
+        (WebCore::WMLDoElement::removedFromDocument):
+        * wml/WMLNoopElement.cpp:
+        (WebCore::WMLNoopElement::insertedIntoDocument):
+        * wml/WMLOnEventElement.cpp:
+        (WebCore::eventHandlingParent):
+        * wml/WMLPostfieldElement.cpp:
+        (WebCore::WMLPostfieldElement::insertedIntoDocument):
+        (WebCore::WMLPostfieldElement::removedFromDocument):
+        * wml/WMLSetvarElement.cpp:
+        (WebCore::WMLSetvarElement::insertedIntoDocument):
+        (WebCore::WMLSetvarElement::removedFromDocument):
+        * wml/WMLTaskElement.cpp:
+        (WebCore::WMLTaskElement::insertedIntoDocument):
+        (WebCore::WMLTaskElement::removedFromDocument):
+        * wml/WMLTimerElement.cpp:
+        (WebCore::WMLTimerElement::insertedIntoDocument):
+        (WebCore::WMLTimerElement::removedFromDocument):
+
 2009-07-29  Yongjun Zhang  <yongjun.zhang@nokia.com>
 
         Reviewed by Simon Hausmann.
index a0636b5..d10e61c 100644 (file)
@@ -268,6 +268,8 @@ bool BackForwardList::containsItem(HistoryItem* entry)
 #if ENABLE(WML)
 void BackForwardList::clearWMLPageHistory()
 {
+    RefPtr<HistoryItem> currentItem = this->currentItem();
+
     int size = m_entries.size();
     for (int i = 0; i < size; ++i)
         pageCache()->remove(m_entries[i].get());
@@ -275,6 +277,9 @@ void BackForwardList::clearWMLPageHistory()
     m_entries.clear();
     m_entryHash.clear();
     m_current = NoCurrentItemIndex;
+
+    // Spec: The history stack may be reset to a state where it only contains the current card.
+    addItem(currentItem);
 }
 #endif
 
index 8b20e56..34be6df 100644 (file)
@@ -113,8 +113,6 @@ void WMLDoElement::insertedIntoDocument()
         m_name = m_type;
 
     Node* parent = parentNode();
-    ASSERT(parent);
-
     if (!parent || !parent->isWMLElement())
         return;
 
@@ -125,7 +123,6 @@ void WMLDoElement::insertedIntoDocument()
 void WMLDoElement::removedFromDocument()
 {
     Node* parent = parentNode();
-    ASSERT(parent);
 
     if (parent  && parent->isWMLElement()) {
         if (WMLEventHandlingElement* eventHandlingElement = toWMLEventHandlingElement(static_cast<WMLElement*>(parent)))
index 7c69ddc..b2ce506 100644 (file)
@@ -45,8 +45,6 @@ void WMLNoopElement::insertedIntoDocument()
     WMLElement::insertedIntoDocument();
 
     Node* parent = parentNode();
-    ASSERT(parent);
-
     if (!parent || !parent->isWMLElement())
         return;
 
index 6774527..6fc4e8b 100644 (file)
@@ -62,7 +62,6 @@ void WMLOnEventElement::parseMappedAttribute(MappedAttribute* attr)
 
 static inline WMLEventHandlingElement* eventHandlingParent(Node* parent)
 {
-    ASSERT(parent);
     if (!parent || !parent->isWMLElement())
         return 0;
 
index 1bee6f0..21f4c5b 100644 (file)
@@ -44,20 +44,14 @@ void WMLPostfieldElement::insertedIntoDocument()
     WMLElement::insertedIntoDocument();
 
     Node* parent = parentNode();
-    ASSERT(parent);
-
-    if (!parent->hasTagName(goTag))
-        return;
-
-    static_cast<WMLGoElement*>(parent)->registerPostfieldElement(this);
+    if (parent && parent->hasTagName(goTag))
+        static_cast<WMLGoElement*>(parent)->registerPostfieldElement(this);
 }
 
 void WMLPostfieldElement::removedFromDocument()
 {
     Node* parent = parentNode();
-    ASSERT(parent);
-
-    if (parent->hasTagName(goTag))
+    if (parent && parent->hasTagName(goTag))
         static_cast<WMLGoElement*>(parent)->deregisterPostfieldElement(this);
 
     WMLElement::removedFromDocument();
index 16d655b..5e10ca8 100644 (file)
@@ -56,8 +56,6 @@ void WMLSetvarElement::insertedIntoDocument()
     WMLElement::insertedIntoDocument();
  
     Node* parent = parentNode();
-    ASSERT(parent);
-
     if (!parent || !parent->isWMLElement())
         return;
 
@@ -68,8 +66,6 @@ void WMLSetvarElement::insertedIntoDocument()
 void WMLSetvarElement::removedFromDocument()
 {
     Node* parent = parentNode();
-    ASSERT(parent);
-
     if (parent && parent->isWMLElement()) {
         if (static_cast<WMLElement*>(parent)->isWMLTaskElement())
             static_cast<WMLTaskElement*>(parent)->deregisterVariableSetter(this);
index 55de663..d49a03e 100644 (file)
@@ -48,8 +48,6 @@ void WMLTaskElement::insertedIntoDocument()
     WMLElement::insertedIntoDocument();
 
     Node* parent = parentNode();
-    ASSERT(parent);
-
     if (!parent || !parent->isWMLElement())
         return;
 
@@ -64,8 +62,6 @@ void WMLTaskElement::insertedIntoDocument()
 void WMLTaskElement::removedFromDocument()
 {
     Node* parent = parentNode();
-    ASSERT(parent);
-
     if (parent && parent->isWMLElement()) {
         if (parent->hasTagName(anchorTag))
             static_cast<WMLAnchorElement*>(parent)->deregisterTask(this);
index c165b08..c6476f7 100644 (file)
@@ -59,8 +59,6 @@ void WMLTimerElement::insertedIntoDocument()
         return;
 
     Node* parent = parentNode();
-    ASSERT(parent);
-
     if (!parent || !parent->isWMLElement())
         return;
 
@@ -73,13 +71,9 @@ void WMLTimerElement::insertedIntoDocument()
 void WMLTimerElement::removedFromDocument()
 {
     Node* parent = parentNode();
-    ASSERT(parent);
-
-    if (parent && parent->isWMLElement()) {
-        if (parent->hasTagName(cardTag)) {
-            m_card->setIntrinsicEventTimer(0);
-            m_card = 0;
-        }
+    if (parent && parent->isWMLElement() && parent->hasTagName(cardTag)) {
+        m_card->setIntrinsicEventTimer(0);
+        m_card = 0;
     }
 
     WMLElement::removedFromDocument();