Changeset 245480 in webkit
- Timestamp:
- May 17, 2019 2:37:07 PM (5 years ago)
- Location:
- trunk/Source/WebKit
- Files:
-
- 5 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebKit/ChangeLog
r245474 r245480 1 2019-05-17 Brady Eidson <beidson@apple.com> 2 3 Avoid races in taking networking assertions for downloads by having both Networking and UIProcess do it. 4 <rdar://problem/50564630> and https://bugs.webkit.org/show_bug.cgi?id=197995 5 6 Reviewed by Chris Dumez. 7 8 There's a fairly indeterminant time gap between when the UIProcess decides a load becomes a download 9 and when the NetworkProcess Download object is created, and therefore the download assertion to be taken. 10 11 The time gap can be long enough for the Networking process to suspend before the download actually starts. 12 13 There's the reverse race when the UIProcess tells a download to stop, as well. 14 15 By having both the UIProcess and NetworkProcess take an assertion on behalf of the NetworkProcess we 16 avoid the race. 17 18 * NetworkProcess/Downloads/DownloadMap.cpp: 19 (WebKit::DownloadMap::add): 20 (WebKit::DownloadMap::remove): 21 22 * NetworkProcess/NetworkResourceLoader.cpp: 23 (WebKit::NetworkResourceLoader::convertToDownload): 24 25 * UIProcess/Downloads/DownloadProxyMap.cpp: 26 (WebKit::DownloadProxyMap::createDownloadProxy): 27 (WebKit::DownloadProxyMap::downloadFinished): 28 (WebKit::DownloadProxyMap::invalidate): 29 * UIProcess/Downloads/DownloadProxyMap.h: 30 1 31 2019-05-17 Keith Rollin <krollin@apple.com> 2 32 -
trunk/Source/WebKit/NetworkProcess/Downloads/DownloadMap.cpp
r243110 r245480 56 56 DownloadMap::DownloadMapType::AddResult DownloadMap::add(DownloadID downloadID, std::unique_ptr<Download>&& download) 57 57 { 58 RELEASE_LOG(Loading, "Adding download %" PRIu64 " to NetworkProcess DownloadMap", downloadID.downloadID()); 59 58 60 auto result = m_downloads.add(downloadID, WTFMove(download)); 59 61 if (m_downloads.size() == 1) { 60 62 ASSERT(!m_downloadAssertion); 61 63 m_downloadAssertion = std::make_unique<ProcessAssertion>(getpid(), "WebKit downloads"_s, AssertionState::UnboundedNetworking); 64 RELEASE_LOG(ProcessSuspension, "Took 'WebKit downloads' assertion in NetworkProcess"); 62 65 } 63 66 … … 67 70 bool DownloadMap::remove(DownloadID downloadID) 68 71 { 72 RELEASE_LOG(Loading, "Removing download %" PRIu64 " from NetworkProcess DownloadMap", downloadID.downloadID()); 73 69 74 auto result = m_downloads.remove(downloadID); 70 75 if (m_downloads.isEmpty()) { 71 76 ASSERT(m_downloadAssertion); 72 77 m_downloadAssertion = nullptr; 78 RELEASE_LOG(ProcessSuspension, "Dropped 'WebKit downloads' assertion in NetworkProcess"); 73 79 } 74 80 -
trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
r245053 r245480 323 323 void NetworkResourceLoader::convertToDownload(DownloadID downloadID, const ResourceRequest& request, const ResourceResponse& response) 324 324 { 325 RELEASE_LOG(Loading, "Converting NetworkResourceLoader %p to download %" PRIu64 " (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ")", this, downloadID.downloadID(), m_parameters.webPageID, m_parameters.webFrameID, m_parameters.identifier); 326 325 327 // This can happen if the resource came from the disk cache. 326 328 if (!m_networkLoad) { -
trunk/Source/WebKit/UIProcess/Downloads/DownloadProxyMap.cpp
r244225 r245480 85 85 m_downloads.set(downloadProxy->downloadID(), downloadProxy.copyRef()); 86 86 87 RELEASE_LOG(Loading, "Adding download %" PRIu64 " to UIProcess DownloadProxyMap", downloadProxy->downloadID().downloadID()); 88 87 89 if (m_downloads.size() == 1 && m_shouldTakeAssertion) { 88 ASSERT(!m_downloadAssertion); 89 m_downloadAssertion = std::make_unique<ProcessAssertion>(getCurrentProcessID(), "WebKit downloads"_s, AssertionState::UnboundedNetworking); 90 ASSERT(!m_downloadUIAssertion); 91 m_downloadUIAssertion = std::make_unique<ProcessAssertion>(getCurrentProcessID(), "WebKit downloads"_s, AssertionState::UnboundedNetworking); 92 93 ASSERT(!m_downloadNetworkingAssertion); 94 RELEASE_ASSERT(m_process); 95 m_downloadNetworkingAssertion = std::make_unique<ProcessAssertion>(m_process->processIdentifier(), "WebKit downloads"_s, AssertionState::UnboundedNetworking); 96 97 RELEASE_LOG(ProcessSuspension, "UIProcess took 'WebKit downloads' assertions for UIProcess and NetworkProcess"); 90 98 } 91 99 … … 99 107 auto downloadID = downloadProxy.downloadID(); 100 108 109 RELEASE_LOG(Loading, "Removing download %" PRIu64 " from UIProcess DownloadProxyMap", downloadID.downloadID()); 110 101 111 // The DownloadProxy may be holding the last reference to the process pool. 102 112 auto protectedProcessPool = makeRefPtr(m_process->processPool()); … … 109 119 110 120 if (m_downloads.isEmpty() && m_shouldTakeAssertion) { 111 ASSERT(m_downloadAssertion); 112 m_downloadAssertion = nullptr; 121 ASSERT(m_downloadUIAssertion); 122 ASSERT(m_downloadNetworkingAssertion); 123 m_downloadUIAssertion = nullptr; 124 m_downloadNetworkingAssertion = nullptr; 125 RELEASE_LOG(ProcessSuspension, "UIProcess released 'WebKit downloads' assertions for UIProcess and NetworkProcess"); 113 126 } 114 127 } … … 124 137 125 138 m_downloads.clear(); 126 m_downloadAssertion = nullptr; 139 m_downloadUIAssertion = nullptr; 140 m_downloadNetworkingAssertion = nullptr; 141 RELEASE_LOG(ProcessSuspension, "UIProcess DownloadProxyMap invalidated - Released 'WebKit downloads' assertions for UIProcess and NetworkProcess"); 142 127 143 m_process = nullptr; 128 144 } -
trunk/Source/WebKit/UIProcess/Downloads/DownloadProxyMap.h
r244217 r245480 73 73 74 74 bool m_shouldTakeAssertion { false }; 75 std::unique_ptr<ProcessAssertion> m_downloadAssertion; 75 std::unique_ptr<ProcessAssertion> m_downloadUIAssertion; 76 std::unique_ptr<ProcessAssertion> m_downloadNetworkingAssertion; 77 76 78 #if PLATFORM(IOS_FAMILY) 77 79 RetainPtr<id> m_backgroundObserver;
Note: See TracChangeset
for help on using the changeset viewer.