Changeset 228469 in webkit

Timestamp:

Feb 14, 2018 10:27:37 AM (6 years ago)

Author:

jmarcell@apple.com

Message:

Cherry-pick r228435. rdar://problem/37538686

Location:

tags/Safari-605.1.27.2

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/inspector/debugger/reload-paused-expected.txt (added)
• LayoutTests/inspector/debugger/reload-paused.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/cache/CachedRawResource.cpp (modified) (4 diffs)
• Source/WebCore/loader/cache/CachedRawResource.h (modified) (2 diffs)

tags/Safari-605.1.27.2/LayoutTests/ChangeLog

@@ +1 @@
+2018-02-14  Jason Marcell  <jmarcell@apple.com>
+
+        Cherry-pick r228435. rdar://problem/37538686
+
+    2018-02-13  Antti Koivisto  <antti@apple.com>
+
+            Crash when breakpoint hit in unload handler
+            https://bugs.webkit.org/show_bug.cgi?id=169855
+            <rdar://problem/28683567>
+
+            Reviewed by Daniel Bates and Joseph Pecoraro.
+
+            * inspector/debugger/reload-paused-expected.txt: Added.
+            * inspector/debugger/reload-paused.html: Added.
+
 2018-02-08  Ryan Haddad  <ryanhaddad@apple.com>
 

tags/Safari-605.1.27.2/Source/WebCore/ChangeLog

@@ +1 @@
+2018-02-14  Jason Marcell  <jmarcell@apple.com>
+
+        Cherry-pick r228435. rdar://problem/37538686
+
+    2018-02-13  Antti Koivisto  <antti@apple.com>
+
+            Crash when breakpoint hit in unload handler
+            https://bugs.webkit.org/show_bug.cgi?id=169855
+            <rdar://problem/28683567>
+
+            Reviewed by Daniel Bates.
+
+            Test: inspector/debugger/reload-paused.html
+
+            CachedRawResource::updateBuffer may generate unload event in client notify callback. If Inspector was
+            paused, this even would spawn a nested runloop. CachedRawResource::finishLoading would get called in
+            the nested loop, confusing the DocumentLoader state machine and resulting in crashes later.
+
+            * loader/cache/CachedRawResource.cpp:
+            (WebCore::CachedRawResource::updateBuffer):
+
+            - Set a bit when entering the client callback.
+            - Ensure we don't re-enter updateBuffer.
+            - If finishLoading got delayed during client callback, do it at the end.
+
+            (WebCore::CachedRawResource::finishLoading):
+
+            If we are in updateBuffer client callback, save the buffer and bail out.
+
+            * loader/cache/CachedRawResource.h:
+
 2018-02-12  Jason Marcell  <jmarcell@apple.com>
 

tags/Safari-605.1.27.2/Source/WebCore/loader/cache/CachedRawResource.cpp

@@ -34 +34 @@
 #include "SubresourceLoader.h"
 #include <wtf/CompletionHandler.h>
+#include <wtf/SetForScope.h>
 #include <wtf/text/StringView.h>
 
@@ -56 +57 @@
 void CachedRawResource::updateBuffer(SharedBuffer& data)
 {
+    // Skip any updateBuffers triggered from nested runloops. We'll have the complete buffer in finishLoading.
+    if (m_inIncrementalDataNotify)
+        return;
+
     CachedResourceHandle<CachedRawResource> protectedThis(this);
     ASSERT(dataBufferingPolicy() == BufferData);
@@ -62 +67 @@
     auto incrementalData = calculateIncrementalDataChunk(&data);
     setEncodedSize(data.size());
-    if (incrementalData)
+    if (incrementalData) {
+        SetForScope<bool> notifyScope(m_inIncrementalDataNotify, true);
         notifyClientsDataWasReceived(incrementalData->data(), incrementalData->size());
+    }
+
     if (dataBufferingPolicy() == DoNotBufferData) {
         if (m_loader)
             m_loader->setDataBufferingPolicy(DoNotBufferData);
         clear();
-        return;
-    }
-
-    CachedResource::updateBuffer(data);
+    } else
+        CachedResource::updateBuffer(data);
+
+    if (m_delayedFinishLoading) {
+        auto delayedFinishLoading = std::exchange(m_delayedFinishLoading, std::nullopt);
+        finishLoading(delayedFinishLoading->buffer.get());
+    }
 }
 
@@ -83 +94 @@
 void CachedRawResource::finishLoading(SharedBuffer* data)
 {
+    if (m_inIncrementalDataNotify) {
+        // We may get here synchronously from updateBuffer() if the callback there ends up spinning a runloop.
+        // In that case delay the call.
+        m_delayedFinishLoading = std::make_optional(DelayedFinishLoading { data });
+        return;
+    };
     CachedResourceHandle<CachedRawResource> protectedThis(this);
     DataBufferingPolicy dataBufferingPolicy = this->dataBufferingPolicy();

tags/Safari-605.1.27.2/Source/WebCore/loader/cache/CachedRawResource.h

@@ -76 +76 @@
     unsigned long m_identifier;
     bool m_allowEncodedDataReplacement;
+    bool m_inIncrementalDataNotify { false };
 
     struct RedirectPair {
@@ -90 +91 @@
 
     Vector<RedirectPair> m_redirectChain;
+
+    struct DelayedFinishLoading {
+        RefPtr<SharedBuffer> buffer;
+    };
+    std::optional<DelayedFinishLoading> m_delayedFinishLoading;
 };