[webkit-reviews] review denied: [Bug 68434] [GTK] Implement cache model for WebKit2 : [Attachment 108325] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 68434: [GTK] Implement cache model for WebKit2
https://bugs.webkit.org/show_bug.cgi?id=68434

Attachment 108325: Patch
https://bugs.webkit.org/attachment.cgi?id=108325&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=108325&action=review


> Source/WebKit2/WebProcess/gtk/WebProcessGtk.cpp:54
> +    g_object_get(G_OBJECT(cache), "cache-dir", &cacheDir.outPtr(), NULL);
> +    if (!cacheDir)
> +	   return 0;

Would it make sense to attach a listener to this property to update WebCore
when it changes?

>>> Source/WebKit2/WebProcess/gtk/WebProcessGtk.cpp:71
>>> +	 guint64 totalMem = g_ascii_strtoull(ptr + strlen("MemTotal:"),
&endPtr, 10);
>> 
>> Could we have strlen("MemTotal:") precalculated in a static variable? Maybe
we could even use sysconf with _SC_PHYS_PAGES and _SC_PAGE_SIZE, it could be
cleaner.
> 
> This code will usually be executed only once to set the cache model, so I
don't think we need micro-optimizations here, but if you think it's worh it I
don't mind to do it.

Sysconf seems like it would be a lot cleaner and not Linux only. I think it'd
be better to ues that here if it makes sense to you.