Title: [223373] releases/WebKitGTK/webkit-2.18
Revision
223373
Author
carlo...@webkit.org
Date
2017-10-16 03:42:09 -0700 (Mon, 16 Oct 2017)

Log Message

Merge r222097 - Make DocumentLoader a FrameDestructionObserver
https://bugs.webkit.org/show_bug.cgi?id=176364
<rdar://problem/34254780>

Reviewed by Alex Christensen.

The DocumentLoader needs to know when its Frame is destroyed so that it can
perform properly cleanup.

Test: fast/events/beforeunload-dom-manipulation-crash.html

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::DocumentLoader): Call FrameDestructionObserver constructor.
(WebCore::DocumentLoader::responseReceived): Drive-by fix. Make sure the current
object is valid during the callback.
(WebCore::DocumentLoader::attachToFrame): Use FrameDestructionObserver::observerFrame rather
than setting the m_frame variable directly.
(WebCore::DocumentLoader::detachFromFrame): Ditto.
* loader/DocumentLoader.h:
(WebCore::DocumentLoader::frame const): Deleted, as this is provided by the FrameDestructionObserver.

Tools:
Provide mechanism to immediately end tests
https://bugs.webkit.org/show_bug.cgi?id=176364
<rdar://problem/34254780>

Reviewed by Alex Christensen.

WebKitTestRunner does not output state if the top loading frame has not been removed. This prevents some
tests that attempt to exercise failed load state from working properly.

This change adds a new 'forceImmediateCompletion' handler for DumpRenderTree and WebKitTestRunner so
that we can properly test these conditions.

* DumpRenderTree/TestRunner.h:
* DumpRenderTree/mac/TestRunnerMac.mm:
(TestRunner::forceImmediateCompletion): Added.
* DumpRenderTree/win/TestRunnerWin.cpp:
(TestRunner::forceImmediateCompletion): Ditto.
* WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
* WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::forceImmediateCompletion): Ditto.
* WebKitTestRunner/InjectedBundle/TestRunner.h:

LayoutTests:
Make DocumentLoader a FrameDestructionObserver
https://bugs.webkit.org/show_bug.cgi?id=176364
<rdar://problem/34254780>

Reviewed by Alex Christensen.

* fast/events/beforeunload-dom-manipulation-crash-expected.txt: Added.
* fast/events/beforeunload-dom-manipulation-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.18/LayoutTests/ChangeLog (223372 => 223373)


--- releases/WebKitGTK/webkit-2.18/LayoutTests/ChangeLog	2017-10-16 10:37:35 UTC (rev 223372)
+++ releases/WebKitGTK/webkit-2.18/LayoutTests/ChangeLog	2017-10-16 10:42:09 UTC (rev 223373)
@@ -1,3 +1,14 @@
+2017-09-15  Brent Fulgham  <bfulg...@apple.com>
+
+        Make DocumentLoader a FrameDestructionObserver
+        https://bugs.webkit.org/show_bug.cgi?id=176364
+        <rdar://problem/34254780>
+
+        Reviewed by Alex Christensen.
+
+        * fast/events/beforeunload-dom-manipulation-crash-expected.txt: Added.
+        * fast/events/beforeunload-dom-manipulation-crash.html: Added.
+
 2017-09-14  Wenson Hsieh  <wenson_hs...@apple.com>
 
         fast/forms/append-children-during-form-submission.html fails due to a text diff on iOS

Added: releases/WebKitGTK/webkit-2.18/LayoutTests/fast/events/beforeunload-dom-manipulation-crash-expected.txt (0 => 223373)


--- releases/WebKitGTK/webkit-2.18/LayoutTests/fast/events/beforeunload-dom-manipulation-crash-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.18/LayoutTests/fast/events/beforeunload-dom-manipulation-crash-expected.txt	2017-10-16 10:42:09 UTC (rev 223373)
@@ -0,0 +1 @@
+This test passes if it does not crash.

Added: releases/WebKitGTK/webkit-2.18/LayoutTests/fast/events/beforeunload-dom-manipulation-crash.html (0 => 223373)


--- releases/WebKitGTK/webkit-2.18/LayoutTests/fast/events/beforeunload-dom-manipulation-crash.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.18/LayoutTests/fast/events/beforeunload-dom-manipulation-crash.html	2017-10-16 10:42:09 UTC (rev 223373)
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<head>
+<script src=""
+<script>
+jsTestIsAsync = true;
+
+function runTest() {
+    window._onbeforeunload_ = nextStep;
+
+    iframe.name = "foo";
+    iframe.src = ""
+
+    location.href = ""
+}
+
+function nextStep() {
+    document.head.appendChild(del);
+    if (window.testRunner)
+        testRunner.forceImmediateCompletion();
+}
+</script>
+</head>
+<body _onload_="runTest()">
+    <p>This test passes if it does not crash.</p>
+    <del id="del">
+    <iframe id="iframe"></iframe>
+</body>

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog (223372 => 223373)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog	2017-10-16 10:37:35 UTC (rev 223372)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog	2017-10-16 10:42:09 UTC (rev 223373)
@@ -1,3 +1,26 @@
+2017-09-15  Brent Fulgham  <bfulg...@apple.com>
+
+        Make DocumentLoader a FrameDestructionObserver
+        https://bugs.webkit.org/show_bug.cgi?id=176364
+        <rdar://problem/34254780>
+
+        Reviewed by Alex Christensen.
+
+        The DocumentLoader needs to know when its Frame is destroyed so that it can
+        perform properly cleanup.
+
+        Test: fast/events/beforeunload-dom-manipulation-crash.html
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::DocumentLoader): Call FrameDestructionObserver constructor.
+        (WebCore::DocumentLoader::responseReceived): Drive-by fix. Make sure the current
+        object is valid during the callback.
+        (WebCore::DocumentLoader::attachToFrame): Use FrameDestructionObserver::observerFrame rather
+        than setting the m_frame variable directly.
+        (WebCore::DocumentLoader::detachFromFrame): Ditto.
+        * loader/DocumentLoader.h:
+        (WebCore::DocumentLoader::frame const): Deleted, as this is provided by the FrameDestructionObserver.
+
 2017-09-15  Carlos Garcia Campos  <cgar...@igalia.com>
 
         [Harfbuzz] Material icons not rendered correctly when using the web font

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/loader/DocumentLoader.cpp (223372 => 223373)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/loader/DocumentLoader.cpp	2017-10-16 10:37:35 UTC (rev 223372)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/loader/DocumentLoader.cpp	2017-10-16 10:42:09 UTC (rev 223373)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2017 Apple Inc. All rights reserved.
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -133,7 +133,8 @@
 }
 
 DocumentLoader::DocumentLoader(const ResourceRequest& request, const SubstituteData& substituteData)
-    : m_cachedResourceLoader(CachedResourceLoader::create(this))
+    : FrameDestructionObserver(nullptr)
+    , m_cachedResourceLoader(CachedResourceLoader::create(this))
     , m_writer(m_frame)
     , m_originalRequest(request)
     , m_substituteData(substituteData)
@@ -704,7 +705,7 @@
     }
 #endif
 
-    frameLoader()->checkContentPolicy(m_response, [this](PolicyAction policy) {
+    frameLoader()->checkContentPolicy(m_response, [this, protectedThis = makeRef(*this)](PolicyAction policy) {
         continueAfterContentPolicy(policy);
     });
 }
@@ -982,7 +983,7 @@
         return;
 
     ASSERT(!m_frame);
-    m_frame = &frame;
+    observeFrame(&frame);
     m_writer.setFrame(&frame);
     attachToFrame();
 
@@ -1023,7 +1024,7 @@
 
     InspectorInstrumentation::loaderDetachedFromFrame(*m_frame, *this);
 
-    m_frame = nullptr;
+    observeFrame(nullptr);
 }
 
 void DocumentLoader::clearMainResourceLoader()

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/loader/DocumentLoader.h (223372 => 223373)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/loader/DocumentLoader.h	2017-10-16 10:37:35 UTC (rev 223372)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/loader/DocumentLoader.h	2017-10-16 10:42:09 UTC (rev 223373)
@@ -32,6 +32,7 @@
 #include "CachedRawResourceClient.h"
 #include "CachedResourceHandle.h"
 #include "DocumentWriter.h"
+#include "FrameDestructionObserver.h"
 #include "LinkIcon.h"
 #include "LoadTiming.h"
 #include "NavigationAction.h"
@@ -90,7 +91,7 @@
     InheritedUserGestures = 1 << 1,
 };
 
-class DocumentLoader : public RefCounted<DocumentLoader>, private CachedRawResourceClient {
+class DocumentLoader : public RefCounted<DocumentLoader>, public FrameDestructionObserver, private CachedRawResourceClient {
     WTF_MAKE_FAST_ALLOCATED;
     friend class ContentFilter;
 public:
@@ -101,7 +102,6 @@
     WEBCORE_EXPORT virtual ~DocumentLoader();
 
     void attachToFrame(Frame&);
-    Frame* frame() const { return m_frame; }
 
     WEBCORE_EXPORT virtual void detachFromFrame();
 
@@ -363,7 +363,6 @@
 
     void notifyFinishedLoadingIcon(uint64_t callbackIdentifier, SharedBuffer*);
 
-    Frame* m_frame { nullptr };
     Ref<CachedResourceLoader> m_cachedResourceLoader;
 
     CachedResourceHandle<CachedRawResource> m_mainResource;

Modified: releases/WebKitGTK/webkit-2.18/Tools/ChangeLog (223372 => 223373)


--- releases/WebKitGTK/webkit-2.18/Tools/ChangeLog	2017-10-16 10:37:35 UTC (rev 223372)
+++ releases/WebKitGTK/webkit-2.18/Tools/ChangeLog	2017-10-16 10:42:09 UTC (rev 223373)
@@ -1,3 +1,27 @@
+2017-09-15  Brent Fulgham  <bfulg...@apple.com>
+
+        Provide mechanism to immediately end tests
+        https://bugs.webkit.org/show_bug.cgi?id=176364
+        <rdar://problem/34254780>
+
+        Reviewed by Alex Christensen.
+
+        WebKitTestRunner does not output state if the top loading frame has not been removed. This prevents some
+        tests that attempt to exercise failed load state from working properly.
+        
+        This change adds a new 'forceImmediateCompletion' handler for DumpRenderTree and WebKitTestRunner so
+        that we can properly test these conditions.
+
+        * DumpRenderTree/TestRunner.h:
+        * DumpRenderTree/mac/TestRunnerMac.mm:
+        (TestRunner::forceImmediateCompletion): Added.
+        * DumpRenderTree/win/TestRunnerWin.cpp:
+        (TestRunner::forceImmediateCompletion): Ditto.
+        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
+        (WTR::TestRunner::forceImmediateCompletion): Ditto.
+        * WebKitTestRunner/InjectedBundle/TestRunner.h:
+
 2017-09-14  Carlos Alberto Lopez Perez  <clo...@igalia.com>
 
         [GTK] Add a switch to start the mini-browser in full-screen mode

Modified: releases/WebKitGTK/webkit-2.18/Tools/DumpRenderTree/TestRunner.h (223372 => 223373)


--- releases/WebKitGTK/webkit-2.18/Tools/DumpRenderTree/TestRunner.h	2017-10-16 10:37:35 UTC (rev 223372)
+++ releases/WebKitGTK/webkit-2.18/Tools/DumpRenderTree/TestRunner.h	2017-10-16 10:42:09 UTC (rev 223373)
@@ -73,6 +73,7 @@
     void displayAndTrackRepaints();
     void execCommand(JSStringRef name, JSStringRef value);
     bool findString(JSContextRef, JSStringRef, JSObjectRef optionsArray);
+    void forceImmediateCompletion();
     void goBack();
     JSValueRef originsWithApplicationCache(JSContextRef);
     long long applicationCacheDiskUsageForOrigin(JSStringRef name);

Modified: releases/WebKitGTK/webkit-2.18/Tools/DumpRenderTree/mac/TestRunnerMac.mm (223372 => 223373)


--- releases/WebKitGTK/webkit-2.18/Tools/DumpRenderTree/mac/TestRunnerMac.mm	2017-10-16 10:37:35 UTC (rev 223372)
+++ releases/WebKitGTK/webkit-2.18/Tools/DumpRenderTree/mac/TestRunnerMac.mm	2017-10-16 10:42:09 UTC (rev 223373)
@@ -290,6 +290,13 @@
     m_waitToDump = false;
 }
 
+void TestRunner::forceImmediateCompletion()
+{
+    if (m_waitToDump && !WorkQueue::singleton().count())
+        dump();
+    m_waitToDump = false;
+}
+
 static inline std::string stringFromJSString(JSStringRef jsString)
 {
     size_t maxBufferSize = JSStringGetMaximumUTF8CStringSize(jsString);

Modified: releases/WebKitGTK/webkit-2.18/Tools/DumpRenderTree/win/TestRunnerWin.cpp (223372 => 223373)


--- releases/WebKitGTK/webkit-2.18/Tools/DumpRenderTree/win/TestRunnerWin.cpp	2017-10-16 10:37:35 UTC (rev 223372)
+++ releases/WebKitGTK/webkit-2.18/Tools/DumpRenderTree/win/TestRunnerWin.cpp	2017-10-16 10:42:09 UTC (rev 223373)
@@ -293,6 +293,14 @@
     m_waitToDump = false;
 }
 
+void TestRunner::forceImmediateCompletion()
+{
+    // Same as on mac. This can be shared.
+    if (m_waitToDump && !WorkQueue::singleton().count())
+        dump();
+    m_waitToDump = false;
+}
+
 static wstring jsStringRefToWString(JSStringRef jsStr)
 {
     size_t length = JSStringGetLength(jsStr);

Modified: releases/WebKitGTK/webkit-2.18/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl (223372 => 223373)


--- releases/WebKitGTK/webkit-2.18/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl	2017-10-16 10:37:35 UTC (rev 223372)
+++ releases/WebKitGTK/webkit-2.18/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl	2017-10-16 10:42:09 UTC (rev 223373)
@@ -101,6 +101,9 @@
     void display();
     void displayAndTrackRepaints();
 
+    // Failed load condition testing
+    void forceImmediateCompletion();
+
     // Printing
     boolean isPageBoxVisible(long pageIndex);
 

Modified: releases/WebKitGTK/webkit-2.18/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp (223372 => 223373)


--- releases/WebKitGTK/webkit-2.18/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp	2017-10-16 10:37:35 UTC (rev 223372)
+++ releases/WebKitGTK/webkit-2.18/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp	2017-10-16 10:42:09 UTC (rev 223373)
@@ -191,6 +191,18 @@
     m_waitToDump = false;
 }
 
+void TestRunner::forceImmediateCompletion()
+{
+    auto& injectedBundle = InjectedBundle::singleton();
+    if (!injectedBundle.isTestRunning())
+        return;
+
+    if (m_waitToDump && injectedBundle.page())
+        injectedBundle.page()->dump();
+
+    m_waitToDump = false;
+}
+
 unsigned TestRunner::imageCountInGeneralPasteboard() const
 {
     return InjectedBundle::singleton().imageCountInGeneralPasteboard();

Modified: releases/WebKitGTK/webkit-2.18/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h (223372 => 223373)


--- releases/WebKitGTK/webkit-2.18/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h	2017-10-16 10:37:35 UTC (rev 223372)
+++ releases/WebKitGTK/webkit-2.18/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h	2017-10-16 10:42:09 UTC (rev 223373)
@@ -164,6 +164,9 @@
     bool shouldDisallowIncreaseForApplicationCacheQuota() { return m_disallowIncreaseForApplicationCacheQuota; }
     JSValueRef originsWithApplicationCache();
 
+    // Failed load condition testing
+    void forceImmediateCompletion();
+
     // Printing
     bool isPageBoxVisible(int pageIndex);
     bool isPrinting() { return m_isPrinting; }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to