Take UnboundedNetworking assertion when a file upload is in progress
Created attachment 364448 [details] Patch
Comment on attachment 364448 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364448&action=review r=me, some comments below, also please update to trunk > Source/WebKit/ChangeLog:8 > + This patch implememts whole bunch of bookkeeping in both the Networking and UI processes. implememts => implements a > Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:808 > +void NetworkConnectionToWebProcess::setProcessIdentifier(ProcessIdentifier processIdentifier) setProcessIdentifier => setWebProcessIdentifier processIdentifier => webProcessIdentifier > Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:813 > +void NetworkConnectionToWebProcess::connectionHasUploads() connectionHasUploads => setConnectionHasUploads > Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:820 > +void NetworkConnectionToWebProcess::connectionNoLongerHasUploads() connectionNoLongerHasUploads => clearConnectionHasUploads > Source/WebKit/NetworkProcess/NetworkResourceLoadMap.cpp:39 > + if (m_loadersWithUploads.size() == 1) > + m_connectionToWebProcess.connectionHasUploads(); Nit: I think it's slightly clearer to check for zero before inserting than it is to check for 1 (or more) after inserting. One reason is that it matches the check in remove. Another reason is that zero is always zero, but non-zero is not always 1. > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:1200 > +void NetworkProcessProxy::releaseUploadAssertion() releaseUploadAssertion => clearUploadAssertion > Source/WebKit/UIProcess/WebProcessPool.cpp:2502 > + if (m_processesWithUploads.size() == 1) { Same comment about 1.
(In reply to Geoffrey Garen from comment #2) > Comment on attachment 364448 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=364448&action=review > > r=me, some comments below, also please update to trunk I was planning on *NOT* updating to trunk, though! :)
Created attachment 364541 [details] Patch
Comment on attachment 364541 [details] Patch Not sure if I should review, but r=me :P
Comment on attachment 364541 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364541&action=review > Source/WebKit/NetworkProcess/NetworkResourceLoadMap.h:64 > + std::unique_ptr<ProcessAssertion> m_networkAssertion; Where is this initialized?
Comment on attachment 364541 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364541&action=review > Source/WebKit/UIProcess/Network/NetworkProcessProxy.h:268 > + std::unique_ptr<ProcessAssertion> m_uploadAssertion; If you take a ProcessAssertion and not a ProcessAndUIAssertion, then what prevents the UIProcess from getting suspended? If the UIProcess gets suspended, how can this unbounded assertion get released?
Comment on attachment 364541 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364541&action=review > Source/WebKit/UIProcess/WebProcessPool.h:776 > + std::unique_ptr<ProcessAssertion> m_uiProcessUploadAssertion; Currently seems like a misnomer no? AFAICT, it does not prevent UIProcess suspension at the moment.
(In reply to Chris Dumez from comment #7) > Comment on attachment 364541 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=364541&action=review > > > Source/WebKit/UIProcess/Network/NetworkProcessProxy.h:268 > > + std::unique_ptr<ProcessAssertion> m_uploadAssertion; > > If you take a ProcessAssertion and not a ProcessAndUIAssertion, then what > prevents the UIProcess from getting suspended? > If the UIProcess gets suspended, how can this unbounded assertion get > released? Let's talk in person, but basically the names you think are important are not.
(In reply to Brady Eidson from comment #9) > (In reply to Chris Dumez from comment #7) > > Comment on attachment 364541 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=364541&action=review > > > > > Source/WebKit/UIProcess/Network/NetworkProcessProxy.h:268 > > > + std::unique_ptr<ProcessAssertion> m_uploadAssertion; > > > > If you take a ProcessAssertion and not a ProcessAndUIAssertion, then what > > prevents the UIProcess from getting suspended? > > If the UIProcess gets suspended, how can this unbounded assertion get > > released? > > Let's talk in person, but basically the names you think are important are > not. Ok, looks good to me now besides the unused data member.
Committed r242891: <https://trac.webkit.org/changeset/242891>
<rdar://problem/48854017>