Commit 7497990e authored by yoav's avatar yoav Committed by Commit bot

Fix memory corruption related to load blocking resource move

This fixes an issue where a resource loader belonging to one
ResourceFetcher was accidentally added as a blocking loader to another
ResourceFetcher, by checking the loader is part of the already
non-blocking loaders belonging to current ResourceFetcher.

This also adds DCHECKs on a couple of methods removing loaders from
hashmaps, to make sure we're not trying to remove a nullptr.

BUG=666563

Review-Url: https://codereview.chromium.org/2537303003
Cr-Commit-Position: refs/heads/master@{#435573}
parent 62e0cea5
......@@ -451,6 +451,7 @@ void ResourceFetcher::moveCachedNonBlockingResourceToBlocking(
// to not-block even after being preloaded and discovered.
if (resource && resource->loader() &&
resource->isLoadEventBlockingResourceType() &&
m_nonBlockingLoaders.contains(resource->loader()) &&
resource->isLinkPreload() && !request.forPreload()) {
m_nonBlockingLoaders.remove(resource->loader());
m_loaders.add(resource->loader());
......@@ -1280,6 +1281,9 @@ void ResourceFetcher::acceptDataFromThreadedReceiver(unsigned long identifier,
}
void ResourceFetcher::moveResourceLoaderToNonBlocking(ResourceLoader* loader) {
DCHECK(loader);
// TODO(yoav): Convert CHECK to DCHECK if no crash reports come in.
CHECK(m_loaders.contains(loader));
m_nonBlockingLoaders.add(loader);
m_loaders.remove(loader);
}
......@@ -1324,6 +1328,7 @@ bool ResourceFetcher::startLoad(Resource* resource) {
}
void ResourceFetcher::removeResourceLoader(ResourceLoader* loader) {
DCHECK(loader);
if (m_loaders.contains(loader))
m_loaders.remove(loader);
else if (m_nonBlockingLoaders.contains(loader))
......
......@@ -615,4 +615,30 @@ TEST_F(ResourceFetcherTest, Revalidate304) {
EXPECT_NE(resource, newResource);
}
TEST_F(ResourceFetcherTest, LinkPreloadImageMultipleFetchersAndMove) {
ResourceFetcher* fetcher = ResourceFetcher::create(
MockFetchContext::create(MockFetchContext::kShouldLoadNewResource));
ResourceFetcher* fetcher2 = ResourceFetcher::create(
MockFetchContext::create(MockFetchContext::kShouldLoadNewResource));
KURL url(ParsedURLString, "http://127.0.0.1:8000/foo.png");
URLTestHelpers::registerMockedURLLoad(url, testImageFilename, "image/png");
FetchRequest fetchRequestOriginal = FetchRequest(url, FetchInitiatorInfo());
fetchRequestOriginal.setLinkPreload(true);
Resource* resource = ImageResource::fetch(fetchRequestOriginal, fetcher);
ASSERT_TRUE(resource);
EXPECT_TRUE(resource->isLinkPreload());
fetcher->preloadStarted(resource);
// Image created by parser on the second fetcher
FetchRequest fetchRequest2 = FetchRequest(url, FetchInitiatorInfo());
Resource* newResource2 = ImageResource::fetch(fetchRequest2, fetcher2);
Persistent<MockResourceClient> client2 = new MockResourceClient(newResource2);
EXPECT_EQ(resource, newResource2);
EXPECT_FALSE(fetcher2->isFetching());
Platform::current()->getURLLoaderMockFactory()->serveAsynchronousRequests();
Platform::current()->getURLLoaderMockFactory()->unregisterURL(url);
}
} // namespace blink
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment