[webkit-reviews] review granted: [Bug 214896] ProcessThrottler should register itself as client to its new assertion if already registered to the old assertion : [Attachment 405445] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 214896: ProcessThrottler should register itself as client to its new
assertion if already registered to the old assertion
https://bugs.webkit.org/show_bug.cgi?id=214896

Attachment 405445: Patch

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




--- Comment #8 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 405445
  --> https://bugs.webkit.org/attachment.cgi?id=405445
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=405445&action=review

>> Source/WebKit/UIProcess/ProcessAssertion.h:69
>> +	ProcessAssertion(ProcessID, const String& reason, ProcessAssertionType,
Client* = nullptr);
> 
> It is not really less error-prone if the parameter is optional.. Also, why
can it be null?

Ok, I see that ProcessAssertion is used by other things than ProcessThrottler
(like DownloadMap) and those don't set themselves as client. So using a raw
pointer seems OK. That said, I don't think the parameter should be optional. If
it is optional, then it is not any less error-prone than setting the client
later using a setter.