NetworkResourceLoader::setDefersLoading() may cause start() to be called multiple...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 3 Aug 2017 05:57:45 +0000 (05:57 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 3 Aug 2017 05:57:45 +0000 (05:57 +0000)
commit59c874339d3496dc6d301cb258b1d8b5f9248242
tree951a79a3d17a3ff40f60a24907a3e7cd6fa86108
parent094e990f69d79bf54635f8614f6b0de5994e6059
NetworkResourceLoader::setDefersLoading() may cause start() to be called multiple times
https://bugs.webkit.org/show_bug.cgi?id=175109
<rdar://problem/33363169>

Reviewed by Brady Eidson.

Source/WebKit:

If NetworkResourceLoader::setDefersLoading(false) is called by the client while m_networkLoad is null
then we call NetworkResourceLoader::start() to start the load. This is needed in the case where
a NetworkResourceLoader is constructed in deferred mode so that the load can later be started via
setDefersLoading(). However, it is possible for setDefersLoading(false) to be called when start()
has already been called, which causes start() to be called multiple times and leads to an assertion
hit in debug.

Normally, setDefersLoading(false) returns without calling start() if m_networkLoad is not null. It
relies on m_networkLoad being non-null to determine if start() should be called. This is bad because
start() checks the disk cache asynchronously *before* calling startNetworkLoad() and initializing
m_networkLoad. Therefore, if setDefersLoading(false) is called *while* we are checking the cache,
then we will call incorrectly call start() again.

In the case of the radar, this happens when we:
1. Call start() and check the disk cache
2. Retrieve a cached redirect from the cache, which causes a WillSendRequest IPC to be sent to the
   WebProcess
3. The WebProcess calls setDefersLoading(true), sends the ContinueWillSendRequest IPC back and
   then calls setDefersLoading(false)

The call to continueWillSendRequest() causes us to retrieve the redirected entry from the cache,
asynchronously, which will return no entry and start a load.
The later call to setDefersLoading(false) causes us to call start() again and we end up back to step 1.

* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::start):
(WebKit::NetworkResourceLoader::setDefersLoading):
* NetworkProcess/NetworkResourceLoader.h:

LayoutTests:

Extend test coverage to cover cacheable redirects to a resource that needs
revalidation, similarly to the case in the radar.

* http/tests/cache/disk-cache/disk-cache-redirect-expected.txt:
* http/tests/cache/disk-cache/disk-cache-redirect.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@220185 268f45cc-cd09-0410-ab3c-d52691b4dbfc
LayoutTests/ChangeLog
LayoutTests/http/tests/cache/disk-cache/disk-cache-redirect-expected.txt
LayoutTests/http/tests/cache/disk-cache/disk-cache-redirect.html
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
Source/WebKit/NetworkProcess/NetworkResourceLoader.h