Post-review comments on r188146
Created attachment 258526 [details] Patch
Comment on attachment 258526 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258526&action=review > Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:55 > + return originalFont; Take originalFont to be non-null and features to be null. Then this will cause a memory leak because on return from this function the originalFont will have a retain count of 2. Assuming you like the design of having a non void return type for this function, one way to prevent this is to change the data type of originalFont from CTFontRef to RetainPtr<CTFontRef>. > Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:60 > + appendTrueTypeFeature(featureArray.get(), (*features)[i]); > + appendOpenTypeFeature(featureArray.get(), (*features)[i]); Is there a better way to write (*features)[i]? One idea is to rename the argument features to featuresPtr and then define a local variable, features, that is constant lvalue reference to FontFeatureSettings after line 55. Then write the remaining code for this function in terms of features.
(In reply to comment #2) > Comment on attachment 258526 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=258526&action=review > > > Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:55 > > + return originalFont; > > [...] Then this will > cause a memory leak because on return from this function the originalFont > will have a retain count of 2. Disregard this remark. It will not leak. Though it will ref-count churn :/
Committed r188168: <http://trac.webkit.org/changeset/188168>