Summary: | Need to enable font cache purging in MemoryPressureHandler | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||||
Component: | WebCore Misc. | Assignee: | Michael Saboff <msaboff> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ggaren | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Michael Saboff
2011-06-03 15:29:17 PDT
Created attachment 95976 [details]
Patch for review
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. ...another option is to have install() initialize the memoryPressureHandler singleton, and have the singleton's constructor do all the dispatch registration. Created attachment 96145 [details]
Updated patch with changes in response to reviewer's comments
Comment on attachment 96145 [details]
Updated patch with changes in response to reviewer's comments
r=me
Committed r88261: <http://trac.webkit.org/changeset/88261> |