FileReader should not prevent entering the back/forward cache
https://bugs.webkit.org/show_bug.cgi?id=203106
Reviewed by Geoffrey Garen.
Source/WebCore:
FileReader should not prevent entering the back/forward cache. To support this,
its implementation now uses a SuspendableTaskQueue to dispatch events.
Test: fast/files/file-reader-back-forward-cache.html
* dom/ActiveDOMObject.cpp:
(WebCore::ActiveDOMObject::isAllowedToRunScript const):
* dom/ActiveDOMObject.h:
* dom/ScriptExecutionContext.cpp:
(WebCore::ScriptExecutionContext::resumeActiveDOMObjects):
* fileapi/FileReader.cpp:
(WebCore::FileReader::FileReader):
(WebCore::FileReader::stop):
(WebCore::FileReader::hasPendingActivity const):
(WebCore::FileReader::readInternal):
(WebCore::FileReader::abort):
(WebCore::FileReader::didStartLoading):
(WebCore::FileReader::didReceiveData):
(WebCore::FileReader::didFinishLoading):
(WebCore::FileReader::didFail):
(WebCore::FileReader::fireEvent):
* fileapi/FileReader.h:
LayoutTests:
Add layout test coverage.
* TestExpectations:
* fast/files/file-reader-back-forward-cache-expected.txt: Added.
* fast/files/file-reader-back-forward-cache.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@251327 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 9f3365b..0107579 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2019-10-19 Chris Dumez <cdumez@apple.com>
+
+ FileReader should not prevent entering the back/forward cache
+ https://bugs.webkit.org/show_bug.cgi?id=203106
+
+ Reviewed by Geoffrey Garen.
+
+ Add layout test coverage.
+
+ * TestExpectations:
+ * fast/files/file-reader-back-forward-cache-expected.txt: Added.
+ * fast/files/file-reader-back-forward-cache.html: Added.
+
2019-10-19 Ryosuke Niwa <rniwa@webkit.org>
Integrate media query evaluation into HTML5 event loop
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index d453a9a..33bba44 100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -266,6 +266,7 @@
imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html [ DumpJSConsoleLogInStdErr Failure Pass ]
imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird.html [ DumpJSConsoleLogInStdErr ]
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-cors-xhr.https.html [ DumpJSConsoleLogInStdErr ]
+fast/files/file-reader-back-forward-cache.html [ DumpJSConsoleLogInStdErr ]
fast/history/page-cache-createImageBitmap.html [ DumpJSConsoleLogInStdErr ]
webkit.org/b/202495 imported/w3c/web-platform-tests/shadow-dom/directionality-002.tentative.html [ ImageOnlyFailure ]
diff --git a/LayoutTests/fast/files/file-reader-back-forward-cache-expected.txt b/LayoutTests/fast/files/file-reader-back-forward-cache-expected.txt
new file mode 100644
index 0000000..006b31f
--- /dev/null
+++ b/LayoutTests/fast/files/file-reader-back-forward-cache-expected.txt
@@ -0,0 +1,15 @@
+Tests that an active FileReader that not prevent entering the back/forward cache.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+pageshow - not from cache
+pagehide - entering cache
+pageshow - from cache
+PASS Page did enter and was restored from the page cache
+PASS Received load event after restoring from the cache
+PASS Received loadend event
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/files/file-reader-back-forward-cache.html b/LayoutTests/fast/files/file-reader-back-forward-cache.html
new file mode 100644
index 0000000..66682ab
--- /dev/null
+++ b/LayoutTests/fast/files/file-reader-back-forward-cache.html
@@ -0,0 +1,72 @@
+<!-- webkit-test-runner [ enableBackForwardCache=true ] -->
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+
+<script>
+let fileReader;
+const testString = 'x'.repeat(10 * 1024);
+let didRestoreFromBackForwardCache = false;
+let successfulyLoaded = false;
+
+window.addEventListener("pageshow", function(event) {
+ debug("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
+
+ if (event.persisted) {
+ testPassed("Page did enter and was restored from the page cache");
+ didRestoreFromBackForwardCache = true;
+ }
+}, false);
+
+window.addEventListener("pagehide", function(event) {
+ debug("pagehide - " + (event.persisted ? "" : "not ") + "entering cache");
+ if (!event.persisted) {
+ testFailed("Page did not enter the page cache.");
+ finishJSTest();
+ }
+}, false);
+
+function onLoad()
+{
+ if (!didRestoreFromBackForwardCache)
+ return;
+
+ testPassed("Received load event after restoring from the cache");
+ successfulyLoaded = true;
+}
+
+function onLoadEnd()
+{
+ if (didRestoreFromBackForwardCache && successfulyLoaded) {
+ testPassed("Received loadend event");
+ finishJSTest();
+ return;
+ }
+
+ setTimeout(() => {
+ fileReader.readAsText(new Blob([testString]));
+ }, 0);
+}
+
+function runTest() {
+ description("Tests that an active FileReader that not prevent entering the back/forward cache.");
+ window.jsTestIsAsync = true;
+
+ setTimeout(() => {
+ fileReader = new FileReader();
+ fileReader.addEventListener("load", onLoad, false);
+ fileReader.addEventListener("loadend", onLoadEnd, false);
+
+ fileReader.readAsText(new Blob([testString]));
+
+ // Force a back navigation back to this page.
+ window.location.href = "../history/resources/page-cache-helper.html";
+ }, 0);
+}
+
+onload = runTest;
+
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 8730ea3..0ad2152 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,33 @@
+2019-10-19 Chris Dumez <cdumez@apple.com>
+
+ FileReader should not prevent entering the back/forward cache
+ https://bugs.webkit.org/show_bug.cgi?id=203106
+
+ Reviewed by Geoffrey Garen.
+
+ FileReader should not prevent entering the back/forward cache. To support this,
+ its implementation now uses a SuspendableTaskQueue to dispatch events.
+
+ Test: fast/files/file-reader-back-forward-cache.html
+
+ * dom/ActiveDOMObject.cpp:
+ (WebCore::ActiveDOMObject::isAllowedToRunScript const):
+ * dom/ActiveDOMObject.h:
+ * dom/ScriptExecutionContext.cpp:
+ (WebCore::ScriptExecutionContext::resumeActiveDOMObjects):
+ * fileapi/FileReader.cpp:
+ (WebCore::FileReader::FileReader):
+ (WebCore::FileReader::stop):
+ (WebCore::FileReader::hasPendingActivity const):
+ (WebCore::FileReader::readInternal):
+ (WebCore::FileReader::abort):
+ (WebCore::FileReader::didStartLoading):
+ (WebCore::FileReader::didReceiveData):
+ (WebCore::FileReader::didFinishLoading):
+ (WebCore::FileReader::didFail):
+ (WebCore::FileReader::fireEvent):
+ * fileapi/FileReader.h:
+
2019-10-19 Adrian Perez de Castro <aperez@igalia.com>
[GTK][WPE] Fix non-unified builds after r250857
diff --git a/Source/WebCore/dom/ActiveDOMObject.cpp b/Source/WebCore/dom/ActiveDOMObject.cpp
index 249cdbf..6347ea2 100644
--- a/Source/WebCore/dom/ActiveDOMObject.cpp
+++ b/Source/WebCore/dom/ActiveDOMObject.cpp
@@ -125,4 +125,9 @@
return !scriptExecutionContext() || scriptExecutionContext()->activeDOMObjectsAreStopped();
}
+bool ActiveDOMObject::isAllowedToRunScript() const
+{
+ return scriptExecutionContext() && !scriptExecutionContext()->activeDOMObjectsAreStopped() && !scriptExecutionContext()->activeDOMObjectsAreSuspended();
+}
+
} // namespace WebCore
diff --git a/Source/WebCore/dom/ActiveDOMObject.h b/Source/WebCore/dom/ActiveDOMObject.h
index 5aa5954..76b5de6 100644
--- a/Source/WebCore/dom/ActiveDOMObject.h
+++ b/Source/WebCore/dom/ActiveDOMObject.h
@@ -109,6 +109,7 @@
}
bool isContextStopped() const;
+ bool isAllowedToRunScript() const;
protected:
explicit ActiveDOMObject(ScriptExecutionContext*);
diff --git a/Source/WebCore/dom/ScriptExecutionContext.cpp b/Source/WebCore/dom/ScriptExecutionContext.cpp
index ca5c5a2..b6c5de1 100644
--- a/Source/WebCore/dom/ScriptExecutionContext.cpp
+++ b/Source/WebCore/dom/ScriptExecutionContext.cpp
@@ -301,13 +301,14 @@
if (m_reasonForSuspendingActiveDOMObjects != why)
return;
- m_activeDOMObjectsAreSuspended = false;
forEachActiveDOMObject([](auto& activeDOMObject) {
activeDOMObject.resume();
return ShouldContinue::Yes;
});
+ m_activeDOMObjectsAreSuspended = false;
+
// In case there were pending messages at the time the script execution context entered the BackForwardCache,
// make sure those get dispatched shortly after restoring from the BackForwardCache.
processMessageWithMessagePortsSoon();
diff --git a/Source/WebCore/fileapi/FileReader.cpp b/Source/WebCore/fileapi/FileReader.cpp
index 62438cd..8355949 100644
--- a/Source/WebCore/fileapi/FileReader.cpp
+++ b/Source/WebCore/fileapi/FileReader.cpp
@@ -36,6 +36,7 @@
#include "Logging.h"
#include "ProgressEvent.h"
#include "ScriptExecutionContext.h"
+#include "SuspendableTaskQueue.h"
#include <JavaScriptCore/ArrayBuffer.h>
#include <wtf/IsoMallocInlines.h>
#include <wtf/text/CString.h>
@@ -56,6 +57,7 @@
FileReader::FileReader(ScriptExecutionContext& context)
: ActiveDOMObject(&context)
+ , m_taskQueue(SuspendableTaskQueue::create(&context))
{
}
@@ -65,12 +67,6 @@
m_loader->cancel();
}
-// FIXME: This should never prevent entering the back/forward cache.
-bool FileReader::shouldPreventEnteringBackForwardCache_DEPRECATED() const
-{
- return hasPendingActivity();
-}
-
const char* FileReader::activeDOMObjectName() const
{
return "FileReader";
@@ -83,7 +79,11 @@
m_loader = nullptr;
}
m_state = DONE;
- m_loadingActivity = nullptr;
+}
+
+bool FileReader::hasPendingActivity() const
+{
+ return m_taskQueue->hasPendingTasks() || m_state == LOADING || ActiveDOMObject::hasPendingActivity();
}
ExceptionOr<void> FileReader::readAsArrayBuffer(Blob* blob)
@@ -133,8 +133,6 @@
if (m_state == LOADING)
return Exception { InvalidStateError };
- m_loadingActivity = makePendingActivity(*this);
-
m_blob = &blob;
m_readType = type;
m_state = LOADING;
@@ -157,10 +155,8 @@
m_aborting = true;
// Schedule to have the abort done later since abort() might be called from the event handler and we do not want the resource loading code to be in the stack.
- scriptExecutionContext()->postTask([this, protectedThis = makeRef(*this)] (ScriptExecutionContext&) {
- if (isContextStopped())
- return;
-
+ m_taskQueue->cancelAllTasks();
+ m_taskQueue->enqueueTask([this] {
ASSERT(m_state != DONE);
stop();
@@ -176,20 +172,24 @@
void FileReader::didStartLoading()
{
- fireEvent(eventNames().loadstartEvent);
+ m_taskQueue->enqueueTask([this] {
+ fireEvent(eventNames().loadstartEvent);
+ });
}
void FileReader::didReceiveData()
{
- auto now = MonotonicTime::now();
- if (std::isnan(m_lastProgressNotificationTime)) {
- m_lastProgressNotificationTime = now;
- return;
- }
- if (now - m_lastProgressNotificationTime > progressNotificationInterval) {
- fireEvent(eventNames().progressEvent);
- m_lastProgressNotificationTime = now;
- }
+ m_taskQueue->enqueueTask([this] {
+ auto now = MonotonicTime::now();
+ if (std::isnan(m_lastProgressNotificationTime)) {
+ m_lastProgressNotificationTime = now;
+ return;
+ }
+ if (now - m_lastProgressNotificationTime > progressNotificationInterval) {
+ fireEvent(eventNames().progressEvent);
+ m_lastProgressNotificationTime = now;
+ }
+ });
}
void FileReader::didFinishLoading()
@@ -197,14 +197,14 @@
if (m_aborting)
return;
- ASSERT(m_state != DONE);
- m_state = DONE;
+ m_taskQueue->enqueueTask([this] {
+ ASSERT(m_state != DONE);
+ m_state = DONE;
- fireEvent(eventNames().progressEvent);
- fireEvent(eventNames().loadEvent);
- fireEvent(eventNames().loadendEvent);
-
- m_loadingActivity = nullptr;
+ fireEvent(eventNames().progressEvent);
+ fireEvent(eventNames().loadEvent);
+ fireEvent(eventNames().loadendEvent);
+ });
}
void FileReader::didFail(int errorCode)
@@ -213,18 +213,19 @@
if (m_aborting)
return;
- ASSERT(m_state != DONE);
- m_state = DONE;
+ m_taskQueue->enqueueTask([this, errorCode] {
+ ASSERT(m_state != DONE);
+ m_state = DONE;
- m_error = FileError::create(static_cast<FileError::ErrorCode>(errorCode));
- fireEvent(eventNames().errorEvent);
- fireEvent(eventNames().loadendEvent);
-
- m_loadingActivity = nullptr;
+ m_error = FileError::create(static_cast<FileError::ErrorCode>(errorCode));
+ fireEvent(eventNames().errorEvent);
+ fireEvent(eventNames().loadendEvent);
+ });
}
void FileReader::fireEvent(const AtomString& type)
{
+ RELEASE_ASSERT(isAllowedToRunScript());
dispatchEvent(ProgressEvent::create(type, true, m_loader ? m_loader->bytesLoaded() : 0, m_loader ? m_loader->totalBytes() : 0));
}
diff --git a/Source/WebCore/fileapi/FileReader.h b/Source/WebCore/fileapi/FileReader.h
index c0b7df7a..ebb8f5b 100644
--- a/Source/WebCore/fileapi/FileReader.h
+++ b/Source/WebCore/fileapi/FileReader.h
@@ -36,6 +36,7 @@
#include "FileError.h"
#include "FileReaderLoader.h"
#include "FileReaderLoaderClient.h"
+#include <wtf/UniqueRef.h>
namespace JSC {
class ArrayBuffer;
@@ -44,6 +45,7 @@
namespace WebCore {
class Blob;
+class SuspendableTaskQueue;
class FileReader final : public RefCounted<FileReader>, public ActiveDOMObject, public EventTargetWithInlineData, private FileReaderLoaderClient {
WTF_MAKE_ISO_ALLOCATED(FileReader);
@@ -74,11 +76,14 @@
using RefCounted::ref;
using RefCounted::deref;
+ // ActiveDOMObject.
+ bool hasPendingActivity() const final;
+
private:
explicit FileReader(ScriptExecutionContext&);
+ // ActiveDOMObject.
const char* activeDOMObjectName() const final;
- bool shouldPreventEnteringBackForwardCache_DEPRECATED() const final;
void stop() final;
EventTargetInterface eventTargetInterface() const final { return FileReaderEventTargetInterfaceType; }
@@ -103,7 +108,7 @@
std::unique_ptr<FileReaderLoader> m_loader;
RefPtr<FileError> m_error;
MonotonicTime m_lastProgressNotificationTime { MonotonicTime::nan() };
- RefPtr<PendingActivity<FileReader>> m_loadingActivity;
+ UniqueRef<SuspendableTaskQueue> m_taskQueue;
};
} // namespace WebCore
diff --git a/Source/WebCore/history/BackForwardCache.cpp b/Source/WebCore/history/BackForwardCache.cpp
index 02acd95..310668a 100644
--- a/Source/WebCore/history/BackForwardCache.cpp
+++ b/Source/WebCore/history/BackForwardCache.cpp
@@ -57,7 +57,7 @@
namespace WebCore {
-#define PCLOG(...) LOG(BackForwardCache, "%*s%s", indentLevel*4, "", makeString(__VA_ARGS__).utf8().data())
+#define PCLOG(...) WTFLogAlways("PageCache: %*s%s", indentLevel*4, "", makeString(__VA_ARGS__).utf8().data())
static inline void logBackForwardCacheFailureDiagnosticMessage(DiagnosticLoggingClient& client, const String& reason)
{
