LayoutTests/fast/encoding/parser-tests-*.html timeout with threaded HTML parser
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Feb 2013 00:39:36 +0000 (00:39 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Feb 2013 00:39:36 +0000 (00:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=109995

Reviewed by Adam Barth.

Source/WebCore:

In the case where during main document onload, we
load a new iframe, and then from within that iframe
we run script to remove the iframe and call testRunner.notifyDone()
the notifyDone() will not correctly dump because
the testRunner does not yet realize that the main resource
has completed loading.

In the main-thread parser, the testRunner does correctly know
that the main thread has completed, because removing the iframe
causes a didFailLoad callback to the embedder, because when
the iframe is being removed, the DocumentLoader for that iframe
is still on the stack and believe's its loading
(because it has a MainResourceLoader which is also on the stack
delivering us the bytes which contain this inline script).

In the threaded-parser case, the DocumentLoader and MainResourceLoader
are no longer on the stack, as we are parsing the iframe asynchronously
after all the bytes have been delivered, and the MainResourceLoader destroyed.
Thus when DocumentLoader::stopLoading() is called, loading() returns
false, and it returns early.  One might argue that we should remove that
early return entirely, but it seemed safer to extend the idea of
when we're loading to include the time when the parser is active.

This patch solves this by teaching the DocumentLoader that it is still
"loading" so long as the parser is still active.

Also added a call to DocumentLoader::checkLoadComplete from
Document::decrementActiveParserCount which seemed to cause
http/tests/multipart/policy-ignore-crash.php to pass.

This causes http/tests/security/feed-urls-from-remote.html to timeout
on chromium (but no other platforms that I'm aware of).  I believe this
is due to a bug in our DRT implementation in the policyDelegate case
(which AFAIK is not a codepath which Chromium actually uses in the wild).
The test already times out on TOT if you remove the setCustomPolicyDelegate calls!

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::isLoading):
(WebCore):
* loader/DocumentLoader.h:
(DocumentLoader):

LayoutTests:

Mark http/tests/security/feed-urls-from-remote.html as timeout on chromium.
I believe this is due to a bug in our DRT implementation in the policyDelegate case
(which AFAIK is not a codepath which Chromium actually uses in the wild).
The test already times out on TOT if you remove the setCustomPolicyDelegate calls.

* platform/chromium/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/platform/chromium/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/DocumentLoader.h

index d5c6878..6f67392 100644 (file)
@@ -1,3 +1,17 @@
+2013-02-21  Eric Seidel  <eric@webkit.org>
+
+        LayoutTests/fast/encoding/parser-tests-*.html timeout with threaded HTML parser
+        https://bugs.webkit.org/show_bug.cgi?id=109995
+
+        Reviewed by Adam Barth.
+
+        Mark http/tests/security/feed-urls-from-remote.html as timeout on chromium.
+        I believe this is due to a bug in our DRT implementation in the policyDelegate case
+        (which AFAIK is not a codepath which Chromium actually uses in the wild).
+        The test already times out on TOT if you remove the setCustomPolicyDelegate calls.
+
+        * platform/chromium/TestExpectations:
+
 2013-02-21  Erik Arvidsson  <arv@chromium.org>
 
         Nodes should not have attributes property
index 23be792..5776eae 100644 (file)
@@ -586,6 +586,8 @@ http/tests/security/storage-blocking-loosened-shared-worker.html [ WontFix ]
 http/tests/security/storage-blocking-strengthened-shared-worker.html [ WontFix ]
 storage/indexeddb/basics-shared-workers.html [ WontFix ]
 
+webkit.org/b/110401 http/tests/security/feed-urls-from-remote.html [ Timeout ]
+
 # test_shell does not support message ports
 webkit.org/b/74459 fast/workers/termination-with-port-messages.html
 webkit.org/b/74459 fast/workers/worker-cloneport.html
index 51ba2ac..449b74b 100644 (file)
@@ -1,3 +1,52 @@
+2013-02-21  Eric Seidel  <eric@webkit.org>
+
+        LayoutTests/fast/encoding/parser-tests-*.html timeout with threaded HTML parser
+        https://bugs.webkit.org/show_bug.cgi?id=109995
+
+        Reviewed by Adam Barth.
+
+        In the case where during main document onload, we
+        load a new iframe, and then from within that iframe
+        we run script to remove the iframe and call testRunner.notifyDone()
+        the notifyDone() will not correctly dump because
+        the testRunner does not yet realize that the main resource
+        has completed loading.
+
+        In the main-thread parser, the testRunner does correctly know
+        that the main thread has completed, because removing the iframe
+        causes a didFailLoad callback to the embedder, because when
+        the iframe is being removed, the DocumentLoader for that iframe
+        is still on the stack and believe's its loading
+        (because it has a MainResourceLoader which is also on the stack
+        delivering us the bytes which contain this inline script).
+
+        In the threaded-parser case, the DocumentLoader and MainResourceLoader
+        are no longer on the stack, as we are parsing the iframe asynchronously
+        after all the bytes have been delivered, and the MainResourceLoader destroyed.
+        Thus when DocumentLoader::stopLoading() is called, loading() returns
+        false, and it returns early.  One might argue that we should remove that
+        early return entirely, but it seemed safer to extend the idea of
+        when we're loading to include the time when the parser is active.
+
+        This patch solves this by teaching the DocumentLoader that it is still
+        "loading" so long as the parser is still active.
+
+        Also added a call to DocumentLoader::checkLoadComplete from
+        Document::decrementActiveParserCount which seemed to cause
+        http/tests/multipart/policy-ignore-crash.php to pass.
+
+        This causes http/tests/security/feed-urls-from-remote.html to timeout
+        on chromium (but no other platforms that I'm aware of).  I believe this
+        is due to a bug in our DRT implementation in the policyDelegate case
+        (which AFAIK is not a codepath which Chromium actually uses in the wild).
+        The test already times out on TOT if you remove the setCustomPolicyDelegate calls!
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::isLoading):
+        (WebCore):
+        * loader/DocumentLoader.h:
+        (DocumentLoader):
+
 2013-02-21  Erik Arvidsson  <arv@chromium.org>
 
         Nodes should not have attributes property
index 02c92a1..807b5a8 100644 (file)
@@ -5773,6 +5773,7 @@ void Document::decrementActiveParserCount()
     --m_activeParserCount;
     if (!frame())
         return;
+    loader()->checkLoadComplete();
     frame()->loader()->checkLoadComplete();
 }
 
index 80537fb..36a214b 100644 (file)
@@ -278,6 +278,13 @@ void DocumentLoader::commitIfReady()
     }
 }
 
+bool DocumentLoader::isLoading() const
+{
+    if (m_frame->document() && m_frame->document()->hasActiveParser())
+        return true;
+    return isLoadingMainResource() || !m_subresourceLoaders.isEmpty() || !m_plugInStreamLoaders.isEmpty();
+}
+
 void DocumentLoader::finishedLoading()
 {
     commitIfReady();
index 8b843e4..0e5cea1 100644 (file)
@@ -114,7 +114,7 @@ namespace WebCore {
         void stopLoading();
         void setCommitted(bool committed) { m_committed = committed; }
         bool isCommitted() const { return m_committed; }
-        bool isLoading() const { return isLoadingMainResource() || !m_subresourceLoaders.isEmpty() || !m_plugInStreamLoaders.isEmpty(); }
+        bool isLoading() const;
         void receivedData(const char*, int);
         void setupForReplace();
         void finishedLoading();
@@ -247,6 +247,7 @@ namespace WebCore {
         ApplicationCacheHost* applicationCacheHost() const { return m_applicationCacheHost.get(); }
 
         virtual void reportMemoryUsage(MemoryObjectInfo*) const;
+        void checkLoadComplete();
 
     protected:
         DocumentLoader(const ResourceRequest&, const SubstituteData&);
@@ -257,7 +258,6 @@ namespace WebCore {
         void commitIfReady();
         void setMainDocumentError(const ResourceError&);
         void commitLoad(const char*, int);
-        void checkLoadComplete();
         void clearMainResourceLoader();
         
         bool maybeCreateArchive();