[webkit-reviews] review granted: [Bug 195497] Take UnboundedNetworking assertion when a file upload is in progress : [Attachment 364448] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Brady Eidson
<beidson at apple.com>'s request for review:
Bug 195497: Take UnboundedNetworking assertion when a file upload is in
progress
https://bugs.webkit.org/show_bug.cgi?id=195497

Attachment 364448: Patch

https://bugs.webkit.org/attachment.cgi?id=364448&action=review




--- Comment #2 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 364448
  --> https://bugs.webkit.org/attachment.cgi?id=364448
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.