Changeset 234045 in webkit

Timestamp:

Jul 20, 2018 9:58:54 AM (6 years ago)

Author:

youenn@apple.com

Message:

FetchResponse should close its stream when loading finishes
​https://bugs.webkit.org/show_bug.cgi?id=187790

Reviewed by Chris Dumez.

It simplifies for a FetchResponse to push all its data into its stream if already created at end of load time.
Did some refactoring in FetchBodyOwner to have a cleaner relationship with the stream source.
Did a minor refactoring to expose the error description when loading fails as part of the rejected promise.
This is consistent to errors sent back through callbacks.

Covered by existing tests.

• Modules/fetch/FetchBodyOwner.cpp:

(WebCore::FetchBodyOwner::~FetchBodyOwner):

• Modules/fetch/FetchBodyOwner.h:
• Modules/fetch/FetchBodySource.cpp:

(WebCore::FetchBodySource::FetchBodySource):
(WebCore::FetchBodySource::setActive):
(WebCore::FetchBodySource::setInactive):
(WebCore::FetchBodySource::doStart):
(WebCore::FetchBodySource::doPull):
(WebCore::FetchBodySource::doCancel):
(WebCore::FetchBodySource::cleanBodyOwner):

• Modules/fetch/FetchBodySource.h:
• Modules/fetch/FetchResponse.cpp:

(WebCore::FetchResponse::BodyLoader::didSucceed):
(WebCore::FetchResponse::BodyLoader::didFail):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/fetch/FetchBodyOwner.cpp (modified) (1 diff)
• Modules/fetch/FetchBodyOwner.h (modified) (1 diff)
• Modules/fetch/FetchBodySource.cpp (modified) (4 diffs)
• Modules/fetch/FetchBodySource.h (modified) (2 diffs)
• Modules/fetch/FetchResponse.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-07-20  Youenn Fablet  <youenn@apple.com>
+
+        FetchResponse should close its stream when loading finishes
+        https://bugs.webkit.org/show_bug.cgi?id=187790
+
+        Reviewed by Chris Dumez.
+
+        It simplifies for a FetchResponse to push all its data into its stream if already created at end of load time.
+        Did some refactoring in FetchBodyOwner to have a cleaner relationship with the stream source.
+        Did a minor refactoring to expose the error description when loading fails as part of the rejected promise.
+        This is consistent to errors sent back through callbacks.
+
+        Covered by existing tests.
+
+        * Modules/fetch/FetchBodyOwner.cpp:
+        (WebCore::FetchBodyOwner::~FetchBodyOwner):
+        * Modules/fetch/FetchBodyOwner.h:
+        * Modules/fetch/FetchBodySource.cpp:
+        (WebCore::FetchBodySource::FetchBodySource):
+        (WebCore::FetchBodySource::setActive):
+        (WebCore::FetchBodySource::setInactive):
+        (WebCore::FetchBodySource::doStart):
+        (WebCore::FetchBodySource::doPull):
+        (WebCore::FetchBodySource::doCancel):
+        (WebCore::FetchBodySource::cleanBodyOwner):
+        * Modules/fetch/FetchBodySource.h:
+        * Modules/fetch/FetchResponse.cpp:
+        (WebCore::FetchResponse::BodyLoader::didSucceed):
+        (WebCore::FetchResponse::BodyLoader::didFail):
+
 2018-07-20  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebCore/Modules/fetch/FetchBodyOwner.cpp

@@ -46 +46 @@
 }
 
+FetchBodyOwner::~FetchBodyOwner()
+{
+    if (m_readableStreamSource)
+        m_readableStreamSource->detach();
+}
+
 void FetchBodyOwner::stop()
 {

trunk/Source/WebCore/Modules/fetch/FetchBodyOwner.h

@@ -42 +42 @@
 public:
     FetchBodyOwner(ScriptExecutionContext&, std::optional<FetchBody>&&, Ref<FetchHeaders>&&);
+    ~FetchBodyOwner();
 
     bool bodyUsed() const { return isDisturbed(); }

trunk/Source/WebCore/Modules/fetch/FetchBodySource.cpp

@@ -35 +35 @@
 
 FetchBodySource::FetchBodySource(FetchBodyOwner& bodyOwner)
-    : m_bodyOwner(bodyOwner)
+    : m_bodyOwner(&bodyOwner)
 {
 }
@@ -41 +41 @@
 void FetchBodySource::setActive()
 {
-    m_bodyOwner.setPendingActivity(&m_bodyOwner);
+    ASSERT(m_bodyOwner);
+    if (m_bodyOwner)
+        m_bodyOwner->setPendingActivity(m_bodyOwner);
 }
 
 void FetchBodySource::setInactive()
 {
-    m_bodyOwner.unsetPendingActivity(&m_bodyOwner);
+    ASSERT(m_bodyOwner);
+    if (m_bodyOwner)
+        m_bodyOwner->unsetPendingActivity(m_bodyOwner);
 }
 
 void FetchBodySource::doStart()
 {
-    m_bodyOwner.consumeBodyAsStream();
+    ASSERT(m_bodyOwner);
+    if (m_bodyOwner)
+        m_bodyOwner->consumeBodyAsStream();
 }
 
 void FetchBodySource::doPull()
 {
-    m_bodyOwner.feedStream();
+    ASSERT(m_bodyOwner);
+    if (m_bodyOwner)
+        m_bodyOwner->feedStream();
 }
 
@@ -62 +70 @@
 {
     m_isCancelling = true;
-    m_bodyOwner.cancel();
+    ASSERT(m_bodyOwner || m_isClosed);
+    if (!m_bodyOwner)
+        return;
+
+    m_bodyOwner->cancel();
+    m_bodyOwner = nullptr;
 }
 
 void FetchBodySource::close()
 {
+#ifndef NDEBUG
+    m_isClosed = true;
+#endif
+
     controller().close();
     clean();
+    m_bodyOwner = nullptr;
 }
 
@@ -75 +93 @@
     controller().error(value);
     clean();
+    m_bodyOwner = nullptr;
 }
 

trunk/Source/WebCore/Modules/fetch/FetchBodySource.h

@@ -50 +50 @@
 
     void resolvePullPromise() { pullFinished(); }
+    void detach() { m_bodyOwner = nullptr; }
 
 private:
@@ -58 +59 @@
     void setInactive() final;
 
-    FetchBodyOwner& m_bodyOwner;
+    FetchBodyOwner* m_bodyOwner;
     bool m_isCancelling { false };
+#ifndef NDEBUG
+    bool m_isClosed { false };
+#endif
 };
 

trunk/Source/WebCore/Modules/fetch/FetchResponse.cpp

@@ -226 +226 @@
 
 #if ENABLE(STREAMS_API)
-    if (m_response.m_readableStreamSource && !m_response.body().consumer().hasData())
+    if (m_response.m_readableStreamSource) {
+        if (m_response.body().consumer().hasData())
+            m_response.m_readableStreamSource->enqueue(m_response.body().consumer().takeAsArrayBuffer());
+
         m_response.closeStream();
+    }
 #endif
     if (auto consumeDataCallback = WTFMove(m_consumeDataCallback))
@@ -253 +257 @@
     if (m_response.m_readableStreamSource) {
         if (!m_response.m_readableStreamSource->isCancelling())
-            m_response.m_readableStreamSource->error("Loading failed"_s);
+            m_response.m_readableStreamSource->error(makeString("Loading failed: "_s, error.localizedDescription()));
         m_response.m_readableStreamSource = nullptr;
     }