Title: [228450] branches/safari-605-branch

Diff

Modified: branches/safari-605-branch/LayoutTests/ChangeLog (228449 => 228450)


--- branches/safari-605-branch/LayoutTests/ChangeLog	2018-02-14 04:15:50 UTC (rev 228449)
+++ branches/safari-605-branch/LayoutTests/ChangeLog	2018-02-14 04:15:53 UTC (rev 228450)
@@ -1,5 +1,20 @@
 2018-02-13  Jason Marcell  <jmarc...@apple.com>
 
+        Cherry-pick r228435. rdar://problem/37518843
+
+    2018-02-13  Antti Koivisto  <an...@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-13  Jason Marcell  <jmarc...@apple.com>
+
         Cherry-pick r228299. rdar://problem/37518837
 
     2018-02-08  Chris Dumez  <cdu...@apple.com>

Added: branches/safari-605-branch/LayoutTests/inspector/debugger/reload-paused-expected.txt (0 => 228450)


--- branches/safari-605-branch/LayoutTests/inspector/debugger/reload-paused-expected.txt	                        (rev 0)
+++ branches/safari-605-branch/LayoutTests/inspector/debugger/reload-paused-expected.txt	2018-02-14 04:15:53 UTC (rev 228450)
@@ -0,0 +1,8 @@
+main frame - has 1 onunload handler(s)
+main frame - has 1 onunload handler(s)
+Test that reloading a paused page doesn't crash.
+
+
+== Running test suite: ReloadPaused
+-- Running test case: ReloadPausedNoCrash
+

Added: branches/safari-605-branch/LayoutTests/inspector/debugger/reload-paused.html (0 => 228450)


--- branches/safari-605-branch/LayoutTests/inspector/debugger/reload-paused.html	                        (rev 0)
+++ branches/safari-605-branch/LayoutTests/inspector/debugger/reload-paused.html	2018-02-14 04:15:53 UTC (rev 228450)
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script src=""
+<script>
+function unloadHandler()
+{
+    debugger;
+}
+
+function test()
+{
+    WI.debuggerManager.allExceptionsBreakpoint.disabled = false;
+
+    let suite = InspectorTest.createAsyncSuite("ReloadPaused");
+
+    suite.addTestCase({
+       name: "ReloadPausedNoCrash",
+       async test() {
+           InspectorTest.reloadPage();
+           await WI.debuggerManager.awaitEvent(WI.DebuggerManager.Event.Paused);
+           await WI.debuggerManager.resume();
+       }
+   });
+
+   suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body _onload_="runTest()" _onunload_="unloadHandler()">
+<p>Test that reloading a paused page doesn't crash.</p>
+</body>
+</html>

Modified: branches/safari-605-branch/Source/WebCore/ChangeLog (228449 => 228450)


--- branches/safari-605-branch/Source/WebCore/ChangeLog	2018-02-14 04:15:50 UTC (rev 228449)
+++ branches/safari-605-branch/Source/WebCore/ChangeLog	2018-02-14 04:15:53 UTC (rev 228450)
@@ -1,5 +1,36 @@
 2018-02-13  Jason Marcell  <jmarc...@apple.com>
 
+        Cherry-pick r228435. rdar://problem/37518843
+
+    2018-02-13  Antti Koivisto  <an...@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-13  Jason Marcell  <jmarc...@apple.com>
+
         Cherry-pick r228325. rdar://problem/37518694
 
     2018-02-09  Youenn Fablet  <you...@apple.com>

Modified: branches/safari-605-branch/Source/WebCore/loader/cache/CachedRawResource.cpp (228449 => 228450)


--- branches/safari-605-branch/Source/WebCore/loader/cache/CachedRawResource.cpp	2018-02-14 04:15:50 UTC (rev 228449)
+++ branches/safari-605-branch/Source/WebCore/loader/cache/CachedRawResource.cpp	2018-02-14 04:15:53 UTC (rev 228450)
@@ -33,6 +33,7 @@
 #include "SharedBuffer.h"
 #include "SubresourceLoader.h"
 #include <wtf/CompletionHandler.h>
+#include <wtf/SetForScope.h>
 #include <wtf/text/StringView.h>
 
 namespace WebCore {
@@ -55,6 +56,10 @@
 
 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);
     m_data = &data;
@@ -61,16 +66,22 @@
 
     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;
+    } else
+        CachedResource::updateBuffer(data);
+
+    if (m_delayedFinishLoading) {
+        auto delayedFinishLoading = std::exchange(m_delayedFinishLoading, std::nullopt);
+        finishLoading(delayedFinishLoading->buffer.get());
     }
-
-    CachedResource::updateBuffer(data);
 }
 
 void CachedRawResource::updateData(const char* data, unsigned length)
@@ -82,6 +93,12 @@
 
 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();
     if (dataBufferingPolicy == BufferData) {

Modified: branches/safari-605-branch/Source/WebCore/loader/cache/CachedRawResource.h (228449 => 228450)


--- branches/safari-605-branch/Source/WebCore/loader/cache/CachedRawResource.h	2018-02-14 04:15:50 UTC (rev 228449)
+++ branches/safari-605-branch/Source/WebCore/loader/cache/CachedRawResource.h	2018-02-14 04:15:53 UTC (rev 228450)
@@ -75,6 +75,7 @@
 
     unsigned long m_identifier;
     bool m_allowEncodedDataReplacement;
+    bool m_inIncrementalDataNotify { false };
 
     struct RedirectPair {
     public:
@@ -89,6 +90,11 @@
     };
 
     Vector<RedirectPair> m_redirectChain;
+
+    struct DelayedFinishLoading {
+        RefPtr<SharedBuffer> buffer;
+    };
+    std::optional<DelayedFinishLoading> m_delayedFinishLoading;
 };
 
 } // namespace WebCore
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to