Bug 62060 – Need to enable font cache purging in MemoryPressureHandler

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Michael Saboff]

This was split out from https://bugs.webkit.org/show_bug.cgi?id=61875.

Need to add call to fontCache()->purgeInactiveFontData(); in MemoryPressureHandler::respondToMemoryPressure().

There is another change discovered to add as well, prevent the ::install() method from multiple invocation.

[Michael Saboff]

Created attachment 95976 [details]
Patch for review

[Geoffrey Garen]

Comment on attachment 95976 [details]
Patch for review

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

r- because I don't think it's good to have two tests for the same thing.

> Source/WebCore/platform/mac/MemoryPressureHandlerMac.mm:67
> +    if (m_installed)
> +        return;
> +    
>      if (!_cache_event_source) {
>          dispatch_async(dispatch_get_main_queue(), ^{
>              _cache_event_source = dispatch_source_create(DISPATCH_SOURCE_TYPE_VM, 0, DISPATCH_VM_PRESSURE, dispatch_get_main_queue());

There is an existing "I've been installed" test: "if (!_cache_event_source)". You're adding a second: "if (m_installed)". I don't think it's good to have two tests for the same thing.

Why isn't the current test sufficient? Is it because initialization is asynchronous, so multiple calls to install() can occur before initialization? If so, you can avoid two tests for the same thing either by removing the initialization delay, or by keeping the delay and adding a null check for _cache_event_source inside the delayed code.

BTW, an early return after the _cache_event_source test would reduce the indentation level of this code, making it more readable.

[Geoffrey Garen]

...another option is to have install() initialize the memoryPressureHandler singleton, and have the singleton's constructor do all the dispatch registration.

[Michael Saboff]

Created attachment 96145 [details]
Updated patch with changes in response to reviewer's comments

[Geoffrey Garen]

Comment on attachment 96145 [details]
Updated patch with changes in response to reviewer's comments

r=me

[Michael Saboff]

Committed r88261: <http://trac.webkit.org/changeset/88261>