[webkit-reviews] review granted: [Bug 209464] Several refactorings done in the MemoryPressureMonitor.cpp : [Attachment 394601] patch

[bugzilla-daemon at webkit.org]

Adrian Perez <aperez at igalia.com> has granted Pablo Saavedra
<psaavedra at igalia.com>'s request for review:
Bug 209464: Several refactorings done in the MemoryPressureMonitor.cpp
https://bugs.webkit.org/show_bug.cgi?id=209464

Attachment 394601: patch

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




--- Comment #8 from Adrian Perez <aperez at igalia.com> ---
Comment on attachment 394601
  --> https://bugs.webkit.org/attachment.cgi?id=394601
patch

Thanks for the patch, having the checks for parsing numeric values in
places and some of the duplicated code factored out makes me more
confident in this being robust ��️. Please check the comments before
landing.

It would be still nice to get rid of “strtok()” because often it is a
source of footguns; but I do not have any good suggestion that would
avoid writing a more formal parser… so let's just leave things as-is.

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

> Source/WebKit/ChangeLog:34
> +	   getMemoryUsageWithCgroup() sighly. The name of the controller

Typo: sighly → slightly

> Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp:232
> +    CString memoryControllerPath = CString();

You can remove the “= CString()” assignment. The default constructor
will initialize the value to an empty string anyway.