Skip to content
Permalink
Browse files
LayoutTests/fast/encoding/parser-tests-*.html timeout with threaded H…
…TML parser

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:


Canonical link: https://commits.webkit.org/128806@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@143664 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
eseidel committed Feb 22, 2013
1 parent e57ccba commit 3b76668ad93a2d5f41b4cb15b2ffabe833410da6
@@ -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
@@ -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
@@ -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
@@ -5773,6 +5773,7 @@ void Document::decrementActiveParserCount()
--m_activeParserCount;
if (!frame())
return;
loader()->checkLoadComplete();
frame()->loader()->checkLoadComplete();
}

@@ -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();
@@ -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();

0 comments on commit 3b76668

Please sign in to comment.