Attachment 341741 Details for Bug 185284 – Fixes the bug

[patch] Fixes the bug

bug-185284-20180601004055.patch (text/plain), 26.13 KB, created by Ryosuke Niwa on 2018-06-01 00:40:55 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-06-01 00:40:55 PDT

Size: 26.13 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 232388)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,76 @@
>+2018-05-31  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        ResourceLoader::cancel() shouldn't synchronously fire load event on document
>+        https://bugs.webkit.org/show_bug.cgi?id=185284
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Because a resource loading can be canceled as a node is removed a document or CachedResource is destructed,
>+        it's not safe to synchronously fire load event on document upon cancelation. This patch makes the cancellation
>+        of a resource load schedule m_checkTimer in FrameLoader to fire a load event asynchronously instead.
>+
>+        Specifically, this patch makes FrameLoader::loadDone call FrameLoader::scheduleCheckCompleted when the load
>+        had failed or cancled instead of calling FrameLoader::checkCompleted which can synchronously fire load event.
>+        To differentiate the two cases, new enum LoadCompletionType has been added to FrameLoader::loadDone and related
>+        functions. To avoid calling the navigation delegate too early, the same abstraction for checkLoadComplete()
>+        has been added in the form of FrameLoader::subresourceLoadDone.
>+
>+        Unfortunately, delaying calls to checkCompleted() and checkLoadComplete() by a timer can result in client
>+        callbacks such as didFinishLoadForFrame and didFailLoadWithError to never get called when the frame gets
>+        detached from the parent after the last resource had stopped loading but before the timer fires. To preserve
>+        these deleagte callbacks, this patch expedites the timer in FrameLoader::frameDetached and Page::goToItem by
>+        by invoking newly added stopAllLoadersAndCheckcompleteness, which stops all loading and then immediately invokes
>+        checkCompleted() and checkLoadComplete() synchronously if m_checkTimer had been started.
>+
>+        Tests: http/tests/preload/dynamic_removing_preload.html
>+
>+        * css/CSSFontSelector.cpp:
>+        (WebCore::CSSFontSelector::beginLoadTimerFired): Removed superfluous call to checkLoadComplete since
>+        cachedResourceLoader's loadDone would call checkLoadComplete anyway.
>+        * html/HTMLFrameOwnerElement.cpp:
>+        (WebCore::HTMLFrameOwnerElement::disconnectContentFrame): Removed the misleading comment added in r140090.
>+        Firefox DOES indeed fire unload event in the content document of a removed frame. While this comment made
>+        it sound like this function isn't called when a frame is removed from the tree when in reality we simply
>+        remove a frame prior to removing the node via disconnectSubframesIfNeeded.
>+        * loader/DocumentLoader.cpp:
>+        (WebCore::DocumentLoader::removeSubresourceLoader):
>+        * loader/DocumentLoader.h:
>+        * loader/FrameLoader.cpp:
>+        (WebCore::FrameLoader::FrameLoader):
>+        (WebCore::FrameLoader::loadDone):
>+        (WebCore::FrameLoader::subresourceLoadDone):
>+        (WebCore::FrameLoader::checkCompleted): Added a release assert that this function is only called when it's safe
>+        to execute scripts.
>+        (WebCore::FrameLoader::checkCompletenessNow): Renamed from checkTimerFired.
>+        (WebCore::FrameLoader::stopAllLoaders): Removed the code to stop m_checkTimer introduced in r53655.
>+        Stopping the timer here would prevent FrameLoader::frameDetached to detect the case when stopping the loader
>+        scheduled a load completion check. Also stopping this timer without clearing the corresponding booleans:
>+        m_checkingLoadCompleteForDetachment and m_checkingLoadCompleteForDetachment is problematic. The assertion
>+        r53655 addressed is now addressed by explicitly checking & clearing the timer in frameDetached.
>+        (WebCore::FrameLoader::stopAllLoadersAndCheckcompleteness): Added.
>+        (WebCore::FrameLoader::checkLoadCompleteForThisFrame): Avoid an early exit when the newly added boolean
>+        m_checkingLoadCompleteForDetachment is set since m_isStopping is no longer set in frameDetached in order
>+        to invoke didFailLoadWithError when detaching a frame.
>+        (WebCore::FrameLoader::frameDetached): Call checkCompletenessNow in the case the frame had already been
>+        completed loading. Also call stopAllLoadersAndCheckcompleteness in the case stopping loading would complete
>+        the loading before stopping active DOM objects.
>+        * loader/FrameLoader.h:
>+        (WebCore::FrameLoader::m_checkingLoadCompleteForDetachment): Added.
>+        * loader/FrameLoaderTypes.h:
>+        (WebCore::LoadCompletionType): Added.
>+        * loader/SubresourceLoader.cpp:
>+        (WebCore::SubresourceLoader::didFinishLoading):
>+        (WebCore::SubresourceLoader::didFail):
>+        (WebCore::SubresourceLoader::didCancel):
>+        (WebCore::SubresourceLoader::notifyDone):
>+        * loader/SubresourceLoader.h:
>+        * loader/cache/CachedResourceLoader.cpp:
>+        (WebCore::CachedResourceLoader::loadDone):
>+        * loader/cache/CachedResourceLoader.h:
>+        * page/Page.cpp:
>+        (WebCore::Page::goToItem): Call stopAllLoadersAndCheckcompleteness instead of stopAllLoaders since stopping
>+        loading here may complete loading.
>+
> 2018-05-31  Per Arne Vollan  <pvollan@apple.com>
> 
>         Add OpenGL display mask to WebPage creation parameters.
>Index: Source/WebCore/css/CSSFontSelector.cpp
>===================================================================
>--- Source/WebCore/css/CSSFontSelector.cpp	(revision 232166)
>+++ Source/WebCore/css/CSSFontSelector.cpp	(working copy)
>@@ -365,11 +365,7 @@ void CSSFontSelector::beginLoadTimerFire
>         cachedResourceLoader.decrementRequestCount(*fontHandle);
>     }
>     // Ensure that if the request count reaches zero, the frame loader will know about it.
>-    cachedResourceLoader.loadDone();
>-    // New font loads may be triggered by layout after the document load is complete but before we have dispatched
>-    // didFinishLoading for the frame. Make sure the delegate is always dispatched by checking explicitly.
>-    if (m_document && m_document->frame())
>-        m_document->frame()->loader().checkLoadComplete();
>+    cachedResourceLoader.loadDone(LoadCompletionType::Finish);
> }
> 
> 
>Index: Source/WebCore/html/HTMLFrameOwnerElement.cpp
>===================================================================
>--- Source/WebCore/html/HTMLFrameOwnerElement.cpp	(revision 232166)
>+++ Source/WebCore/html/HTMLFrameOwnerElement.cpp	(working copy)
>@@ -77,10 +77,6 @@ void HTMLFrameOwnerElement::clearContent
> 
> void HTMLFrameOwnerElement::disconnectContentFrame()
> {
>-    // FIXME: Currently we don't do this in removedFrom because this causes an
>-    // unload event in the subframe which could execute script that could then
>-    // reach up into this document and then attempt to look back down. We should
>-    // see if this behavior is really needed as Gecko does not allow this.
>     if (RefPtr<Frame> frame = contentFrame()) {
>         Ref<Frame> protect(*frame);
>         frame->loader().frameDetached();
>Index: Source/WebCore/loader/DocumentLoader.cpp
>===================================================================
>--- Source/WebCore/loader/DocumentLoader.cpp	(revision 232166)
>+++ Source/WebCore/loader/DocumentLoader.cpp	(working copy)
>@@ -1618,7 +1618,7 @@ void DocumentLoader::addSubresourceLoade
>     m_subresourceLoaders.add(loader->identifier(), loader);
> }
> 
>-void DocumentLoader::removeSubresourceLoader(ResourceLoader* loader)
>+void DocumentLoader::removeSubresourceLoader(LoadCompletionType type, ResourceLoader* loader)
> {
>     ASSERT(loader->identifier());
> 
>@@ -1626,7 +1626,7 @@ void DocumentLoader::removeSubresourceLo
>         return;
>     checkLoadComplete();
>     if (Frame* frame = m_frame)
>-        frame->loader().checkLoadComplete();
>+        frame->loader().subresourceLoadDone(type);
> }
> 
> void DocumentLoader::addPlugInStreamLoader(ResourceLoader& loader)
>Index: Source/WebCore/loader/DocumentLoader.h
>===================================================================
>--- Source/WebCore/loader/DocumentLoader.h	(revision 232166)
>+++ Source/WebCore/loader/DocumentLoader.h	(working copy)
>@@ -269,7 +269,7 @@ public:
>     void setPopUpPolicy(PopUpPolicy popUpPolicy) { m_popUpPolicy = popUpPolicy; }
> 
>     void addSubresourceLoader(ResourceLoader*);
>-    void removeSubresourceLoader(ResourceLoader*);
>+    void removeSubresourceLoader(LoadCompletionType, ResourceLoader*);
>     void addPlugInStreamLoader(ResourceLoader&);
>     void removePlugInStreamLoader(ResourceLoader&);
> 
>Index: Source/WebCore/loader/FrameLoader.cpp
>===================================================================
>--- Source/WebCore/loader/FrameLoader.cpp	(revision 232166)
>+++ Source/WebCore/loader/FrameLoader.cpp	(working copy)
>@@ -280,7 +280,7 @@ FrameLoader::FrameLoader(Frame& frame, F
>     , m_wasUnloadEventEmitted(false)
>     , m_isComplete(false)
>     , m_needsClear(false)
>-    , m_checkTimer(*this, &FrameLoader::checkTimerFired)
>+    , m_checkTimer(*this, &FrameLoader::checkCompletenessNow)
>     , m_shouldCallCheckCompleted(false)
>     , m_shouldCallCheckLoadComplete(false)
>     , m_opener(nullptr)
>@@ -783,9 +783,20 @@ void FrameLoader::finishedParsing()
>     m_frame.view()->restoreScrollbar();
> }
> 
>-void FrameLoader::loadDone()
>+void FrameLoader::loadDone(LoadCompletionType type)
> {
>-    checkCompleted();
>+    if (type == LoadCompletionType::Finish)
>+        checkCompleted();
>+    else
>+        scheduleCheckCompleted();
>+}
>+
>+void FrameLoader::subresourceLoadDone(LoadCompletionType type)
>+{
>+    if (type == LoadCompletionType::Finish)
>+        checkLoadComplete();
>+    else
>+        scheduleCheckLoadComplete();
> }
> 
> bool FrameLoader::allChildrenAreComplete() const
>@@ -808,6 +819,7 @@ bool FrameLoader::allAncestorsAreComplet
> 
> void FrameLoader::checkCompleted()
> {
>+    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(ScriptDisallowedScope::InMainThread::isScriptAllowed());
>     m_shouldCallCheckCompleted = false;
> 
>     // Have we completed before?
>@@ -881,7 +893,7 @@ void FrameLoader::checkCompleted()
>         checkLoadComplete();
> }
> 
>-void FrameLoader::checkTimerFired()
>+void FrameLoader::checkCompletenessNow()
> {
>     Ref<Frame> protect(m_frame);
> 
>@@ -1761,11 +1773,20 @@ void FrameLoader::stopAllLoaders(ClearPr
> 
>     setProvisionalDocumentLoader(nullptr);
> 
>-    m_checkTimer.stop();
>-
>     m_inStopAllLoaders = false;    
> }
> 
>+void FrameLoader::stopAllLoadersAndCheckcompleteness()
>+{
>+    stopAllLoaders();
>+    if (m_checkTimer.isActive()) {
>+        m_checkTimer.stop();
>+        m_checkingLoadCompleteForDetachment = true;
>+        checkCompletenessNow();
>+        m_checkingLoadCompleteForDetachment = false;
>+    }
>+}
>+
> void FrameLoader::stopForUserCancel(bool deferCheckLoadComplete)
> {
>     // Calling stopAllLoaders can cause the frame to be deallocated, including the frame loader.
>@@ -2391,8 +2412,8 @@ void FrameLoader::checkLoadCompleteForTh
>         }
>         
>         case FrameStateCommittedPage: {
>-            DocumentLoader* dl = m_documentLoader.get();            
>-            if (!dl || (dl->isLoadingInAPISense() && !dl->isStopping()))
>+            DocumentLoader* dl = m_documentLoader.get();
>+            if (!dl || (dl->isLoadingInAPISense() && !dl->isStopping() && !m_checkingLoadCompleteForDetachment))
>                 return;
> 
>             setState(FrameStateComplete);
>@@ -2615,11 +2636,16 @@ void FrameLoader::dispatchOnloadEvents()
> 
> void FrameLoader::frameDetached()
> {
>-    // Calling stopAllLoaders() can cause the frame to be deallocated, including the frame loader.
>+    // Calling stopAllLoadersAndCheckcompleteness() can cause the frame to be deallocated, including the frame loader.
>     Ref<Frame> protectedFrame(m_frame);
> 
>+    if (m_checkTimer.isActive()) {
>+        m_checkTimer.stop();
>+        checkCompletenessNow();
>+    }
>+
>     if (m_frame.document()->pageCacheState() != Document::InPageCache) {
>-        stopAllLoaders();
>+        stopAllLoadersAndCheckcompleteness();
>         m_frame.document()->stopActiveDOMObjects();
>     }
> 
>Index: Source/WebCore/loader/FrameLoader.h
>===================================================================
>--- Source/WebCore/loader/FrameLoader.h	(revision 232166)
>+++ Source/WebCore/loader/FrameLoader.h	(working copy)
>@@ -138,6 +138,7 @@ public:
>     static void reportAuthenticationChallengeBlocked(Frame*, const URL&, const String& reason);
> 
>     // FIXME: These are all functions which stop loads. We have too many.
>+    void stopAllLoadersAndCheckcompleteness();
>     WEBCORE_EXPORT void stopAllLoaders(ClearProvisionalItemPolicy = ShouldClearProvisionalItem);
>     WEBCORE_EXPORT void stopForUserCancel(bool deferCheckLoadComplete = false);
>     void stop();
>@@ -253,7 +254,8 @@ public:
> 
>     void setOutgoingReferrer(const URL&);
> 
>-    void loadDone();
>+    void loadDone(LoadCompletionType);
>+    void subresourceLoadDone(LoadCompletionType);
>     void finishedParsing();
>     void checkCompleted();
> 
>@@ -318,7 +320,7 @@ private:
> 
>     bool allChildrenAreComplete() const; // immediate children, not all descendants
> 
>-    void checkTimerFired();
>+    void checkCompletenessNow();
> 
>     void loadSameDocumentItem(HistoryItem&);
>     void loadDifferentDocumentItem(HistoryItem&, FrameLoadType, FormSubmissionCacheLoadPolicy, NavigationPolicyCheck);
>@@ -467,6 +469,8 @@ private:
>     bool m_isStrictRawResourceValidationPolicyDisabledForTesting { false };
>     bool m_currentLoadShouldCheckNavigationPolicy { true };
> 
>+    bool m_checkingLoadCompleteForDetachment { false };
>+
>     URL m_previousURL;
>     RefPtr<HistoryItem> m_requestedHistoryItem;
> };
>Index: Source/WebCore/loader/FrameLoaderTypes.h
>===================================================================
>--- Source/WebCore/loader/FrameLoaderTypes.h	(revision 232166)
>+++ Source/WebCore/loader/FrameLoaderTypes.h	(working copy)
>@@ -155,6 +155,11 @@ struct SystemPreviewInfo {
>     bool isSystemPreview { false };
> };
> 
>+enum class LoadCompletionType {
>+    Finish,
>+    Cancel
>+};
>+
> } // namespace WebCore
> 
> namespace WTF {
>Index: Source/WebCore/loader/SubresourceLoader.cpp
>===================================================================
>--- Source/WebCore/loader/SubresourceLoader.cpp	(revision 232166)
>+++ Source/WebCore/loader/SubresourceLoader.cpp	(working copy)
>@@ -647,7 +647,7 @@ void SubresourceLoader::didFinishLoading
>     m_resource->finish();
>     ASSERT(!reachedTerminalState());
>     didFinishLoadingOnePart(networkLoadMetrics);
>-    notifyDone();
>+    notifyDone(LoadCompletionType::Finish);
> 
>     if (reachedTerminalState())
>         return;
>@@ -683,7 +683,7 @@ void SubresourceLoader::didFail(const Re
>         MemoryCache::singleton().remove(*m_resource);
>     m_resource->error(CachedResource::LoadError);
>     cleanupForError(error);
>-    notifyDone();
>+    notifyDone(LoadCompletionType::Cancel);
>     if (reachedTerminalState())
>         return;
>     releaseResources();
>@@ -725,7 +725,7 @@ void SubresourceLoader::didCancel(const 
>         tracePoint(SubresourceLoadDidEnd);
> 
>     m_resource->cancelLoad();
>-    notifyDone();
>+    notifyDone(LoadCompletionType::Cancel);
> }
> 
> void SubresourceLoader::didRetrieveDerivedDataFromCache(const String& type, SharedBuffer& buffer)
>@@ -735,20 +735,21 @@ void SubresourceLoader::didRetrieveDeriv
>     m_resource->didRetrieveDerivedDataFromCache(type, buffer);
> }
> 
>-void SubresourceLoader::notifyDone()
>+void SubresourceLoader::notifyDone(LoadCompletionType type)
> {
>     if (reachedTerminalState())
>         return;
> 
>     m_requestCountTracker = std::nullopt;
>+    bool shouldPerformPostLoadActions = true;
> #if PLATFORM(IOS)
>-    m_documentLoader->cachedResourceLoader().loadDone(m_state != CancelledWhileInitializing);
>-#else
>-    m_documentLoader->cachedResourceLoader().loadDone();
>+    if (m_state == CancelledWhileInitializing)
>+        shouldPerformPostLoadActions = false;
> #endif
>+    m_documentLoader->cachedResourceLoader().loadDone(type, shouldPerformPostLoadActions);
>     if (reachedTerminalState())
>         return;
>-    m_documentLoader->removeSubresourceLoader(this);
>+    m_documentLoader->removeSubresourceLoader(type, this);
> }
> 
> void SubresourceLoader::releaseResources()
>Index: Source/WebCore/loader/SubresourceLoader.h
>===================================================================
>--- Source/WebCore/loader/SubresourceLoader.h	(revision 232166)
>+++ Source/WebCore/loader/SubresourceLoader.h	(working copy)
>@@ -95,7 +95,7 @@ private:
> 
>     void didReceiveDataOrBuffer(const char*, int, RefPtr<SharedBuffer>&&, long long encodedDataLength, DataPayloadType);
> 
>-    void notifyDone();
>+    void notifyDone(LoadCompletionType);
> 
>     void reportResourceTiming(const NetworkLoadMetrics&);
> 
>Index: Source/WebCore/loader/cache/CachedResourceLoader.cpp
>===================================================================
>--- Source/WebCore/loader/cache/CachedResourceLoader.cpp	(revision 232166)
>+++ Source/WebCore/loader/cache/CachedResourceLoader.cpp	(working copy)
>@@ -1299,13 +1299,13 @@ void CachedResourceLoader::removeCachedR
>     m_documentResources.remove(resource.url());
> }
> 
>-void CachedResourceLoader::loadDone(bool shouldPerformPostLoadActions)
>+void CachedResourceLoader::loadDone(LoadCompletionType type, bool shouldPerformPostLoadActions)
> {
>     RefPtr<DocumentLoader> protectDocumentLoader(m_documentLoader);
>     RefPtr<Document> protectDocument(m_document);
> 
>     if (frame())
>-        frame()->loader().loadDone();
>+        frame()->loader().loadDone(type);
> 
>     if (shouldPerformPostLoadActions)
>         performPostLoadActions();
>Index: Source/WebCore/loader/cache/CachedResourceLoader.h
>===================================================================
>--- Source/WebCore/loader/cache/CachedResourceLoader.h	(revision 232166)
>+++ Source/WebCore/loader/cache/CachedResourceLoader.h	(working copy)
>@@ -132,7 +132,7 @@ public:
> 
>     void removeCachedResource(CachedResource&);
> 
>-    void loadDone(bool shouldPerformPostLoadActions = true);
>+    void loadDone(LoadCompletionType, bool shouldPerformPostLoadActions = true);
> 
>     WEBCORE_EXPORT void garbageCollectDocumentResources();
> 
>Index: Source/WebCore/page/Page.cpp
>===================================================================
>--- Source/WebCore/page/Page.cpp	(revision 232166)
>+++ Source/WebCore/page/Page.cpp	(working copy)
>@@ -482,8 +482,9 @@ void Page::goToItem(HistoryItem& item, F
>     // being deref()-ed. Make sure we can still use it with HistoryController::goToItem later.
>     Ref<HistoryItem> protector(item);
> 
>-    if (m_mainFrame->loader().history().shouldStopLoadingForHistoryItem(item))
>-        m_mainFrame->loader().stopAllLoaders();
>+    auto& frameLoader = m_mainFrame->loader();
>+    if (frameLoader.history().shouldStopLoadingForHistoryItem(item))
>+        m_mainFrame->loader().stopAllLoadersAndCheckcompleteness();
> 
>     m_mainFrame->loader().history().goToItem(item, type, navigationPolicyCheck);
> }
>Index: WebKit.xcworkspace/xcshareddata/xcschemes/All Source.xcscheme
>===================================================================
>--- WebKit.xcworkspace/xcshareddata/xcschemes/All Source.xcscheme	(revision 232166)
>+++ WebKit.xcworkspace/xcshareddata/xcschemes/All Source.xcscheme	(working copy)
>@@ -1,6 +1,6 @@
> <?xml version="1.0" encoding="UTF-8"?>
> <Scheme
>-   version = "1.7">
>+   version = "1.3">
>    <BuildAction
>       parallelizeBuildables = "NO"
>       buildImplicitDependencies = "YES">
>@@ -137,17 +137,16 @@
>       buildConfiguration = "Debug"
>       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
>       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
>-      language = ""
>       shouldUseLaunchSchemeArgsEnv = "YES">
>       <Testables>
>       </Testables>
>       <MacroExpansion>
>          <BuildableReference
>             BuildableIdentifier = "primary"
>-            BlueprintIdentifier = "8D1107260486CEB800E47090"
>-            BuildableName = "MiniBrowser.app"
>-            BlueprintName = "MiniBrowser"
>-            ReferencedContainer = "container:Tools/MiniBrowser/MiniBrowser.xcodeproj">
>+            BlueprintIdentifier = "8DD76F960486AA7600D96B5E"
>+            BuildableName = "WebKitTestRunner"
>+            BlueprintName = "WebKitTestRunner"
>+            ReferencedContainer = "container:Tools/WebKitTestRunner/WebKitTestRunner.xcodeproj">
>          </BuildableReference>
>       </MacroExpansion>
>       <AdditionalOptions>
>@@ -157,7 +156,6 @@
>       buildConfiguration = "Debug"
>       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
>       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
>-      language = ""
>       launchStyle = "0"
>       useCustomWorkingDirectory = "NO"
>       ignoresPersistentStateOnLaunch = "YES"
>@@ -168,12 +166,29 @@
>          runnableDebuggingMode = "0">
>          <BuildableReference
>             BuildableIdentifier = "primary"
>-            BlueprintIdentifier = "8D1107260486CEB800E47090"
>-            BuildableName = "MiniBrowser.app"
>-            BlueprintName = "MiniBrowser"
>-            ReferencedContainer = "container:Tools/MiniBrowser/MiniBrowser.xcodeproj">
>+            BlueprintIdentifier = "8DD76F960486AA7600D96B5E"
>+            BuildableName = "WebKitTestRunner"
>+            BlueprintName = "WebKitTestRunner"
>+            ReferencedContainer = "container:Tools/WebKitTestRunner/WebKitTestRunner.xcodeproj">
>          </BuildableReference>
>       </BuildableProductRunnable>
>+      <CommandLineArguments>
>+         <CommandLineArgument
>+            argument = "/Volumes/Data/webkit4/LayoutTests/jquery/offset.html"
>+            isEnabled = "NO">
>+         </CommandLineArgument>
>+         <CommandLineArgument
>+            argument = "http://localhost:8800/WebKit/service-workers/clone-opaque-being-loaded-response.html"
>+            isEnabled = "YES">
>+         </CommandLineArgument>
>+      </CommandLineArguments>
>+      <EnvironmentVariables>
>+         <EnvironmentVariable
>+            key = "OS_ACTIVITY_MODE"
>+            value = "disable"
>+            isEnabled = "YES">
>+         </EnvironmentVariable>
>+      </EnvironmentVariables>
>       <AdditionalOptions>
>       </AdditionalOptions>
>    </LaunchAction>
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 232342)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,16 @@
>+2018-05-31  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        ResourceLoader::cancel() shouldn't synchronously fire load event on document
>+        https://bugs.webkit.org/show_bug.cgi?id=185284
>+        <rdar://problem/39994507>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * http/tests/xmlhttprequest/reentrant-cancel-expected.txt:
>+        * http/tests/xmlhttprequest/reentrant-cancel.html: Canceling XHR inside addElement is no longer
>+        firing load event synchronously as expected. Added a code to end the test after the load event.
>+        * http/wpt/service-workers/clone-opaque-being-loaded-response.html:
>+
> 2018-05-30  Nan Wang  <n_wang@apple.com>
> 
>         AX: VoiceOver on macOS does not announce fieldset description from aria-describedby when focussing inputs
>Index: LayoutTests/editing/pasteboard/drag-image-to-contenteditable-in-iframe.html
>===================================================================
>--- LayoutTests/editing/pasteboard/drag-image-to-contenteditable-in-iframe.html	(revision 232166)
>+++ LayoutTests/editing/pasteboard/drag-image-to-contenteditable-in-iframe.html	(working copy)
>@@ -10,14 +10,15 @@ function log(message) {
>     li.appendChild(text);
> }
> 
>+if (window.testRunner)
>+    testRunner.waitUntilDone();
>+
> function runTest() {
>     if (!window.testRunner) {
>         log("To run this test manually attempt to drag and drop Abe after the broken image in the editable div");
>         return;
>     }
>-
>-    testRunner.waitUntilDone();
>-
>+    
>     //find abe
>     var dragme = document.getElementById("dragme");
>     x1 = dragme.offsetLeft + 20;
>@@ -34,8 +35,14 @@ function runTest() {
>     eventSender.leapForward(500);
>     eventSender.mouseMoveTo(x2, y2);
>     eventSender.mouseUp();
>-    
>-    testRunner.notifyDone();
>+
>+    if (window.testRunner) {
>+        var pasted_image = drag_target.contentDocument.querySelector('.target');
>+        if (pasted_image.complete)
>+            testRunner.notifyDone();
>+        else
>+            pasted_image.onload = function () { testRunner.notifyDone(); }
>+    }
> }
> 
> </script>
>@@ -44,7 +51,7 @@ function runTest() {
> <body onload="runTest()">
>     <p>This tests that we can drag an image into the last position of a content editable div in an iframe that already contains an image, without crashing.</p>
>     
>-    <img id="dragme" src="../resources/abe.png"/>
>+    <img id="dragme" class="target" src="../resources/abe.png"/>
>     <iframe id="drag_target" src="resources/drag-image-to-contenteditable-iframe.html"></iframe>
>     <ul id="console"></ul>
> </body>
>Index: LayoutTests/http/tests/xmlhttprequest/reentrant-cancel-expected.txt
>===================================================================
>--- LayoutTests/http/tests/xmlhttprequest/reentrant-cancel-expected.txt	(revision 232166)
>+++ LayoutTests/http/tests/xmlhttprequest/reentrant-cancel-expected.txt	(working copy)
>@@ -1 +1,4 @@
>+CONSOLE MESSAGE: line 16: Sending XHR
>+CONSOLE MESSAGE: line 16: Sending XHR
>+CONSOLE MESSAGE: line 16: Sending XHR
> XThis tests that when we re-entrantly create and cancel XHRs, we don't try to disconnect the same CachedResourceClient multiple times from its CachedResource. We pass if we don't crash. XX
>Index: LayoutTests/http/tests/xmlhttprequest/reentrant-cancel.html
>===================================================================
>--- LayoutTests/http/tests/xmlhttprequest/reentrant-cancel.html	(revision 232166)
>+++ LayoutTests/http/tests/xmlhttprequest/reentrant-cancel.html	(working copy)
>@@ -13,6 +13,7 @@ window.onload = addElement;
> var xhr = new XMLHttpRequest;
> function sendXHR()
> {
>+    console.log('Sending XHR');
>     xhr.open("GET", "", true);
>     try {
>         xhr.send();
>@@ -28,6 +29,10 @@ function sendXHR()
> }
> window.addEventListener("DOMSubtreeModified", sendXHR);
> addElement();
>+window.addEventListener('load', () => setTimeout(() => {
>+    if (window.testRunner)
>+        testRunner.notifyDone();
>+}, 0));
> </script>
> This tests that when we re-entrantly create and cancel XHRs, we don't try to disconnect the same CachedResourceClient
> multiple times from its CachedResource. We pass if we don't crash.

Actions: View | Formatted Diff | Diff

Attachments on bug 185284: 341647 | 341652 | 341653 | 341656 | 341678 | 341737 | 341741 | 341743 | 341745 | 341786