Changeset 132695 in webkit

Timestamp:

Oct 26, 2012, 2:30:22 PM (13 years ago)

Author:

beidson@apple.com

Message:

Crash in WebProces at WebCore::ResourceLoadScheduler::crossOriginRedirectReceived + 78
<rdar://problem/12575514> and ​https://bugs.webkit.org/show_bug.cgi?id=100554

Reviewed by Alexey Proskuryakov.

This was fallout from ​http://trac.webkit.org/changeset/132501 where I missed some of the
spots that call resourceLoadScheduler().

As a result we were creating more than one ResourceLoadScheduler, allowing the host records
to get out of sync.

The fix that also results in less #ifdefs scattered throughout the code is to use a single
choke point for all ResourceLoadScheduler access.

No new tests
(No change of behavior for the default config, not testable at this time in the repro config)

Add a single choke point for accessing the correct ResourceLoadScheduler:

• loader/ResourceLoadScheduler.cpp:

(WebCore::defaultResourceLoadScheduler): New private function that keeps the singleton default ResourceLoadScheduler.
(WebCore::resourceLoadScheduler): Refactor this function to either ask the LoaderStrategy or call through to

Revert back to using that single choke point everywhere:

• dom/ContainerNode.cpp:

(WebCore::ContainerNode::suspendPostAttachCallbacks):
(WebCore::ContainerNode::resumePostAttachCallbacks):

• loader/MainResourceLoader.cpp:

(WebCore::MainResourceLoader::loadNow):

• loader/ResourceLoader.cpp:

(WebCore::ResourceLoader::releaseResources):
(WebCore::ResourceLoader::willSendRequest):

• loader/cache/CachedResource.cpp:

(WebCore::CachedResource::load):

• loader/cache/CachedResourceLoader.cpp:

(WebCore::CachedResourceLoader::performPostLoadActions):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/ContainerNode.cpp (modified) (3 diffs)
• loader/MainResourceLoader.cpp (modified) (2 diffs)
• loader/ResourceLoadScheduler.cpp (modified) (2 diffs)
• loader/ResourceLoadScheduler.h (modified) (1 diff)
• loader/ResourceLoader.cpp (modified) (3 diffs)
• loader/cache/CachedResource.cpp (modified) (2 diffs)
• loader/cache/CachedResourceLoader.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-10-26  Brady Eidson  <beidson@apple.com>
+
+        Crash in WebProces at WebCore::ResourceLoadScheduler::crossOriginRedirectReceived + 78
+        <rdar://problem/12575514> and https://bugs.webkit.org/show_bug.cgi?id=100554
+
+        Reviewed by Alexey Proskuryakov.
+
+        This was fallout from http://trac.webkit.org/changeset/132501 where I missed some of the 
+        spots that call resourceLoadScheduler().
+
+        As a result we were creating more than one ResourceLoadScheduler, allowing the host records 
+        to get out of sync.
+
+        The fix that also results in less #ifdefs scattered throughout the code is to use a single 
+        choke point for all ResourceLoadScheduler access.
+
+        No new tests 
+        (No change of behavior for the default config, not testable at this time in the repro config)
+
+        Add a single choke point for accessing the correct ResourceLoadScheduler:
+        * loader/ResourceLoadScheduler.cpp:
+        (WebCore::defaultResourceLoadScheduler): New private function that keeps the singleton default ResourceLoadScheduler.
+        (WebCore::resourceLoadScheduler): Refactor this function to either ask the LoaderStrategy or call through to
+
+        Revert back to using that single choke point everywhere:
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::suspendPostAttachCallbacks):
+        (WebCore::ContainerNode::resumePostAttachCallbacks):
+
+        * loader/MainResourceLoader.cpp:
+        (WebCore::MainResourceLoader::loadNow):
+
+        * loader/ResourceLoader.cpp:
+        (WebCore::ResourceLoader::releaseResources):
+        (WebCore::ResourceLoader::willSendRequest):
+
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::load):
+
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::performPostLoadActions):
+
 2012-10-26  Elliott Sprehn  <esprehn@chromium.org>
 

trunk/Source/WebCore/dom/ContainerNode.cpp

@@ -36 +36 @@
 #include "InsertionPoint.h"
 #include "InspectorInstrumentation.h"
-#include "LoaderStrategy.h"
 #include "MemoryCache.h"
 #include "MutationEvent.h"
-#include "ResourceLoadScheduler.h"
 #include "Page.h"
-#include "PlatformStrategies.h"
 #include "RenderBox.h"
 #include "RenderTheme.h"
 #include "RenderWidget.h"
+#include "ResourceLoadScheduler.h"
 #include "RootInlineBox.h"
 #include "UndoManager.h"
@@ -644 +642 @@
             }
         }
-#if USE(PLATFORM_STRATEGIES)
-        platformStrategies()->loaderStrategy()->resourceLoadScheduler()->suspendPendingRequests();
-#else
         resourceLoadScheduler()->suspendPendingRequests();
-#endif
     }
     ++s_attachDepth;
@@ -665 +659 @@
                 page->setMemoryCacheClientCallsEnabled(true);
         }
-#if USE(PLATFORM_STRATEGIES)
-        platformStrategies()->loaderStrategy()->resourceLoadScheduler()->resumePendingRequests();
-#else
         resourceLoadScheduler()->resumePendingRequests();
-#endif
     }
     --s_attachDepth;

trunk/Source/WebCore/loader/MainResourceLoader.cpp

@@ -45 +45 @@
 #include "HistoryItem.h"
 #include "InspectorInstrumentation.h"
-#include "LoaderStrategy.h"
 #include "Page.h"
-#include "PlatformStrategies.h"
 #include "ResourceError.h"
 #include "ResourceHandle.h"
@@ -633 +631 @@
         return true;
 
-#if USE(PLATFORM_STRATEGIES)
-    platformStrategies()->loaderStrategy()->resourceLoadScheduler()->addMainResourceLoad(this);
-#else
     resourceLoadScheduler()->addMainResourceLoad(this);
-#endif
 
     if (m_substituteData.isValid())

trunk/Source/WebCore/loader/ResourceLoadScheduler.cpp

@@ -32 +32 @@
 #include "InspectorInstrumentation.h"
 #include "KURL.h"
+#include "LoaderStrategy.h"
 #include "Logging.h"
 #include "NetscapePlugInStreamLoader.h"
+#include "PlatformStrategies.h"
 #include "ResourceLoader.h"
 #include "ResourceRequest.h"
@@ -72 +74 @@
 }
 
-ResourceLoadScheduler* resourceLoadScheduler()
+ResourceLoadScheduler* ResourceLoadScheduler::defaultResourceLoadScheduler()
 {
     ASSERT(isMainThread());
     DEFINE_STATIC_LOCAL(ResourceLoadScheduler, resourceLoadScheduler, ());
     return &resourceLoadScheduler;
+}
+
+ResourceLoadScheduler* resourceLoadScheduler()
+{
+#if USE(PLATFORM_STRATEGIES)
+    return platformStrategies()->loaderStrategy()->resourceLoadScheduler();
+#else
+    return ResourceLoadScheduler::defaultResourceLoadScheduler();
+#endif
 }
 

trunk/Source/WebCore/loader/ResourceLoadScheduler.h

@@ -106 +106 @@
     void servePendingRequests(HostInformation*, ResourceLoadPriority);
 
+    static ResourceLoadScheduler* defaultResourceLoadScheduler();
+
     typedef HashMap<String, HostInformation*, StringHash> HostMap;
     HostMap m_hosts;

trunk/Source/WebCore/loader/ResourceLoader.cpp

@@ -38 +38 @@
 #include "FrameLoaderClient.h"
 #include "InspectorInstrumentation.h"
-#include "LoaderStrategy.h"
 #include "Page.h"
-#include "PlatformStrategies.h"
 #include "ProgressTracker.h"
 #include "ResourceBuffer.h"
@@ -92 +90 @@
     m_reachedTerminalState = true;
 
-#if USE(PLATFORM_STRATEGIES)
-    platformStrategies()->loaderStrategy()->resourceLoadScheduler()->remove(this);
-#endif
+    resourceLoadScheduler()->remove(this);
+
     m_identifier = 0;
-#if !USE(PLATFORM_STRATEGIES)
-    resourceLoadScheduler()->remove(this);
-#endif
 
     if (m_handle) {
@@ -244 +238 @@
     }
 
-    if (!redirectResponse.isNull()) {
-#if USE(PLATFORM_STRATEGIES)
-        platformStrategies()->loaderStrategy()->resourceLoadScheduler()->crossOriginRedirectReceived(this, request.url());
-#else
+    if (!redirectResponse.isNull())
         resourceLoadScheduler()->crossOriginRedirectReceived(this, request.url());
-#endif
-    }
+
     m_request = request;
 }

trunk/Source/WebCore/loader/cache/CachedResource.cpp

@@ -37 +37 @@
 #include "InspectorInstrumentation.h"
 #include "KURL.h"
-#include "LoaderStrategy.h"
 #include "Logging.h"
-#include "PlatformStrategies.h"
 #include "PurgeableBuffer.h"
 #include "ResourceBuffer.h"
@@ -286 +284 @@
         addAdditionalRequestHeaders(cachedResourceLoader);
 
-#if USE(PLATFORM_STRATEGIES)
-    m_loader = platformStrategies()->loaderStrategy()->resourceLoadScheduler()->scheduleSubresourceLoad(cachedResourceLoader->frame(), this, m_resourceRequest, m_resourceRequest.priority(), options);
-#else
     m_loader = resourceLoadScheduler()->scheduleSubresourceLoad(cachedResourceLoader->frame(), this, m_resourceRequest, m_resourceRequest.priority(), options);
-#endif
 
     if (!m_loader) {

trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

@@ -45 +45 @@
 #include "FrameLoaderClient.h"
 #include "HTMLElement.h"
-#include "LoaderStrategy.h"
 #include "Logging.h"
 #include "MemoryCache.h"
 #include "PingLoader.h"
-#include "PlatformStrategies.h"
 #include "ResourceLoadScheduler.h"
 #include "SecurityOrigin.h"
@@ -731 +729 @@
     checkForPendingPreloads();
 
-#if USE(PLATFORM_STRATEGIES)
-    platformStrategies()->loaderStrategy()->resourceLoadScheduler()->servePendingRequests();
-#else
     resourceLoadScheduler()->servePendingRequests();
-#endif
 }