http/tests/w3c/dom/nodes/Element-matches.html is flaky

https://bugs.webkit.org/show_bug.cgi?id=148615

Reviewed by Ryosuke Niwa.

Source/WebCore:

Several newly-imported w3c tests were flaky due to the :target
pseudo-class selectors sometimes giving different results. The
issue seems to be that this type of selector relies on the
Document::cssTarget() element to do the matching. We update
this cssTarget Element in FrameView's scrollToFragment() /
scrollToAnchor(). This is called from
scrollToFragmentWithParentBoundary() which is called by
FrameLoader's finishedParsing() and loadInSameDocument().

In the first one, it is called *after* calling checkComplete()
which fires the onload event. However, in the second method,
it is called *before*. This patch updates finishedParsing()
so that scrollToFragmentWithParentBoundary() is called *before*
firing the onload event, consistently with loadInSameDocument().
This makes sure that JavaScript executed in an onload event
handler will get accurate results for :target pseudo-class
selectors.

No new tests, covered by:
http/tests/w3c/dom/nodes/Element-matches.html
http/tests/w3c/dom/nodes/ParentNode-querySelector-All-xhtml.xhtml

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::finishedParsing):

LayoutTests:

Unskip the tests and rebaseline them now that the target pseudo selector
checks are consistently passing.

* TestExpectations:
* http/tests/w3c/dom/nodes/Element-matches-expected.txt:
* http/tests/w3c/dom/nodes/ParentNode-querySelector-All-expected.txt:
* http/tests/w3c/dom/nodes/ParentNode-querySelector-All-xhtml-expected.txt:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 1e33f69..efa49eb 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,18 @@
+2015-09-02  Chris Dumez  <cdumez@apple.com>
+
+        http/tests/w3c/dom/nodes/Element-matches.html is flaky
+        https://bugs.webkit.org/show_bug.cgi?id=148615
+
+        Reviewed by Ryosuke Niwa.
+
+        Unskip the tests and rebaseline them now that the target pseudo selector
+        checks are consistently passing.
+
+        * TestExpectations:
+        * http/tests/w3c/dom/nodes/Element-matches-expected.txt:
+        * http/tests/w3c/dom/nodes/ParentNode-querySelector-All-expected.txt:
+        * http/tests/w3c/dom/nodes/ParentNode-querySelector-All-xhtml-expected.txt:
+
 2015-09-02  Daniel Bates  <dabates@apple.com>
 
         Update iOS TestExpectations files

diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index 2b2fb1a..7c86745 100644 (file)
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -222,11 +222,6 @@ webkit.org/b/148546 [ Debug ] http/tests/w3c/dom/ranges/Range-isPointInRange.htm
 webkit.org/b/148546 [ Debug ] http/tests/w3c/dom/ranges/Range-mutations.html [ Skip ]
 webkit.org/b/148546 [ Debug ] http/tests/w3c/dom/ranges/Range-set.html [ Skip ]
 
-# Newly imported W3C DOM tests that are flaky.
-webkit.org/b/148615 http/tests/w3c/dom/nodes/Element-matches.html [ Failure Pass ]
-webkit.org/b/148615 http/tests/w3c/dom/nodes/ParentNode-querySelector-All.html [ Failure Pass ]
-webkit.org/b/148615 http/tests/w3c/dom/nodes/ParentNode-querySelector-All-xhtml.xhtml [ Failure Pass ]
-
 # @supports W3C Failures
 webkit.org/b/137566 css3/conditional/w3c/at-supports-010.html [ ImageOnlyFailure ]
 webkit.org/b/137568 css3/conditional/w3c/at-supports-014.html [ ImageOnlyFailure ]

diff --git a/LayoutTests/http/tests/w3c/dom/nodes/ParentNode-querySelector-All-expected.txt b/LayoutTests/http/tests/w3c/dom/nodes/ParentNode-querySelector-All-expected.txt
index 2c0da22..7f45644 100644 (file)
--- a/LayoutTests/http/tests/w3c/dom/nodes/ParentNode-querySelector-All-expected.txt
+++ b/LayoutTests/http/tests/w3c/dom/nodes/ParentNode-querySelector-All-expected.txt
@@ -553,8 +553,8 @@ PASS Document.querySelectorAll: :link and :visited pseudo-class selectors, match
 PASS Document.querySelector: :link and :visited pseudo-class selectors, matching a and area elements with href attributes: #pseudo-link :link, #pseudo-link :visited 
 PASS Document.querySelectorAll: :link and :visited pseudo-class selectors, matching link elements with href attributes: #head :link, #head :visited 
 PASS Document.querySelector: :link and :visited pseudo-class selectors, matching link elements with href attributes: #head :link, #head :visited 
-FAIL Document.querySelectorAll: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target assert_equals: The method should return the expected number of matches. expected 1 but got 0
-FAIL Document.querySelector: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target assert_not_equals: The method should return a match. got disallowed value null
+PASS Document.querySelectorAll: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target 
+PASS Document.querySelector: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target 
 PASS Document.querySelectorAll: :lang pseudo-class selector, matching inherited language: #pseudo-lang-div1:lang(en) 
 PASS Document.querySelector: :lang pseudo-class selector, matching inherited language: #pseudo-lang-div1:lang(en) 
 PASS Document.querySelectorAll: :lang pseudo-class selector, matching specified language with exact value: #pseudo-lang-div2:lang(fr) 
@@ -1803,8 +1803,8 @@ PASS In-document Element.querySelectorAll: :link and :visited pseudo-class selec
 PASS In-document Element.querySelector: :link and :visited pseudo-class selectors, not matching link elements with href attributes: #head :link, #head :visited 
 PASS In-document Element.querySelectorAll: :link and :visited pseudo-class selectors, chained, mutually exclusive pseudo-classes match nothing: :link:visited 
 PASS In-document Element.querySelector: :link and :visited pseudo-class selectors, chained, mutually exclusive pseudo-classes match nothing: :link:visited 
-FAIL In-document Element.querySelectorAll: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target assert_equals: The method should return the expected number of matches. expected 1 but got 0
-FAIL In-document Element.querySelector: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target assert_not_equals: The method should return a match. got disallowed value null
+PASS In-document Element.querySelectorAll: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target 
+PASS In-document Element.querySelector: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target 
 PASS In-document Element.querySelectorAll: :lang pseudo-class selector, matching inherited language: #pseudo-lang-div1:lang(en) 
 PASS In-document Element.querySelector: :lang pseudo-class selector, matching inherited language: #pseudo-lang-div1:lang(en) 
 PASS In-document Element.querySelectorAll: :lang pseudo-class selector, matching specified language with exact value: #pseudo-lang-div2:lang(fr) 

diff --git a/LayoutTests/http/tests/w3c/dom/nodes/ParentNode-querySelector-All-xhtml-expected.txt b/LayoutTests/http/tests/w3c/dom/nodes/ParentNode-querySelector-All-xhtml-expected.txt
index 12aff4f..98f6122 100644 (file)
--- a/LayoutTests/http/tests/w3c/dom/nodes/ParentNode-querySelector-All-xhtml-expected.txt
+++ b/LayoutTests/http/tests/w3c/dom/nodes/ParentNode-querySelector-All-xhtml-expected.txt
@@ -553,8 +553,8 @@ PASS Document.querySelectorAll: :link and :visited pseudo-class selectors, match
 PASS Document.querySelector: :link and :visited pseudo-class selectors, matching a and area elements with href attributes: #pseudo-link :link, #pseudo-link :visited 
 PASS Document.querySelectorAll: :link and :visited pseudo-class selectors, matching link elements with href attributes: #head :link, #head :visited 
 PASS Document.querySelector: :link and :visited pseudo-class selectors, matching link elements with href attributes: #head :link, #head :visited 
-FAIL Document.querySelectorAll: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target assert_equals: The method should return the expected number of matches. expected 1 but got 0
-FAIL Document.querySelector: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target assert_not_equals: The method should return a match. got disallowed value null
+PASS Document.querySelectorAll: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target 
+PASS Document.querySelector: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target 
 PASS Document.querySelectorAll: :lang pseudo-class selector, matching inherited language: #pseudo-lang-div1:lang(en) 
 PASS Document.querySelector: :lang pseudo-class selector, matching inherited language: #pseudo-lang-div1:lang(en) 
 PASS Document.querySelectorAll: :lang pseudo-class selector, matching specified language with exact value: #pseudo-lang-div2:lang(fr) 
@@ -1803,8 +1803,8 @@ PASS In-document Element.querySelectorAll: :link and :visited pseudo-class selec
 PASS In-document Element.querySelector: :link and :visited pseudo-class selectors, not matching link elements with href attributes: #head :link, #head :visited 
 PASS In-document Element.querySelectorAll: :link and :visited pseudo-class selectors, chained, mutually exclusive pseudo-classes match nothing: :link:visited 
 PASS In-document Element.querySelector: :link and :visited pseudo-class selectors, chained, mutually exclusive pseudo-classes match nothing: :link:visited 
-FAIL In-document Element.querySelectorAll: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target assert_equals: The method should return the expected number of matches. expected 1 but got 0
-FAIL In-document Element.querySelector: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target assert_not_equals: The method should return a match. got disallowed value null
+PASS In-document Element.querySelectorAll: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target 
+PASS In-document Element.querySelector: :target pseudo-class selector, matching the element referenced by the URL fragment identifier: :target 
 PASS In-document Element.querySelectorAll: :lang pseudo-class selector, matching inherited language: #pseudo-lang-div1:lang(en) 
 PASS In-document Element.querySelector: :lang pseudo-class selector, matching inherited language: #pseudo-lang-div1:lang(en) 
 PASS In-document Element.querySelectorAll: :lang pseudo-class selector, matching specified language with exact value: #pseudo-lang-div2:lang(fr) 

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 1a98a8f..0fafdb8 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,35 @@
+2015-09-02  Chris Dumez  <cdumez@apple.com>
+
+        http/tests/w3c/dom/nodes/Element-matches.html is flaky
+        https://bugs.webkit.org/show_bug.cgi?id=148615
+
+        Reviewed by Ryosuke Niwa.
+
+        Several newly-imported w3c tests were flaky due to the :target
+        pseudo-class selectors sometimes giving different results. The
+        issue seems to be that this type of selector relies on the
+        Document::cssTarget() element to do the matching. We update
+        this cssTarget Element in FrameView's scrollToFragment() /
+        scrollToAnchor(). This is called from
+        scrollToFragmentWithParentBoundary() which is called by
+        FrameLoader's finishedParsing() and loadInSameDocument().
+
+        In the first one, it is called *after* calling checkComplete()
+        which fires the onload event. However, in the second method,
+        it is called *before*. This patch updates finishedParsing()
+        so that scrollToFragmentWithParentBoundary() is called *before*
+        firing the onload event, consistently with loadInSameDocument().
+        This makes sure that JavaScript executed in an onload event
+        handler will get accurate results for :target pseudo-class
+        selectors.
+
+        No new tests, covered by:
+        http/tests/w3c/dom/nodes/Element-matches.html
+        http/tests/w3c/dom/nodes/ParentNode-querySelector-All-xhtml.xhtml
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::finishedParsing):
+
 2015-09-02  Zan Dobersek  <zdobersek@igalia.com>
 
         Construct default winding string arguments in CanvasRenderingContext2D from ASCIILiteral objects

diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
index cc9a035..8deacf4 100644 (file)
--- a/Source/WebCore/loader/FrameLoader.cpp
+++ b/Source/WebCore/loader/FrameLoader.cpp
@@ -753,6 +753,8 @@ void FrameLoader::finishedParsing()
 
     m_client.dispatchDidFinishDocumentLoad();
 
+    scrollToFragmentWithParentBoundary(m_frame.document()->url());
+
     checkCompleted();
 
     if (!m_frame.view())
@@ -761,7 +763,6 @@ void FrameLoader::finishedParsing()
     // Check if the scrollbars are really needed for the content.
     // If not, remove them, relayout, and repaint.
     m_frame.view()->restoreScrollbar();
-    scrollToFragmentWithParentBoundary(m_frame.document()->url());
 }
 
 void FrameLoader::loadDone()